Make LowResourceMonitor extendable

jetty / jetty.project

Eclipse Jetty® - Web Container & Clients - supports HTTP/2, HTTP/1.1, HTTP/1.0, websocket, servlets, and more

https://eclipse.dev/jetty

Other

3.79k stars 1.91k forks source link

Make LowResourceMonitor extendable #2775

Open blazespinnaker opened 5 years ago

blazespinnaker commented 5 years ago

LowResourceMonitor is a useful foundation for managing low resources in a jetty instance. In particular, I'd like to be able to extend it to monitor DB connections and certain other resources. Adding a dependency injection function to override to be called by monitor and the ability to access _cause in a subclass would be useful.

joakime commented 5 years ago

@olamy are you going to make that commit (on your branch) a PR?

olamy commented 5 years ago

@joakime still need to look at the test failure

olamy commented 5 years ago

pr merged

sbordet commented 5 years ago

@olamy there is some work to do for this issue.

Interface LowResourceCheck was not public, see #3193.

Also, that interface makes little sense. It has 2 methods, but getReason() is never called. Furthermore, having 2 methods makes the information on low resources not atomic, so the wrong information may be returned.

This whole issue needs to be rethought and redesigned more carefully.

olamy commented 5 years ago

@sbordet the idea was too keep maximum of backward compat. What about something such

    public interface LowResourceCheck
    {
        LowResourceStatus getLowResourceStatus();
    }

    public class LowResourceStatus
    {
        public boolean isLowOnResources() {
        }

        public String getReason(){
        }
    }

Something more simple could be

    public interface LowResourceCheck
    {
        boolean isLowOnResources();
    }

And the reason is printed in the log. WDYT?