[
https://issues.apache.org/jira/browse/SLING-3278?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13851218#comment-13851218
]
Georg Henzler commented on SLING-3278:
--------------------------------------
Hi Bertrand,
now we have two competing implementations :( I was hoping you would base your
work on my patch or just give feedback.
So I have an improved version of the patch that makes as little changes as
possible (but still is more capable as the version from your patch):
* HealthCheckExecutor.runAllForTags() acts as a facade to the user interface,
implementation details (timeouts, hc lookup etc.) are handled in the executor,
but the user of the interface does not need to know them. Also the design is
cleaner as in the web console, you don't have to read service reference
properties (as the results have the property HealthCheckDescriptor)
* The tests are truely run in parallel and the latest results are returned (as
HealthCheckExecutorImpl.runAllForTags(String...) waits for the futures)
* Caching is in place (configurable, 2sec default)
* Async execution can easily be achieved by specifying a property (this could
easily be taken out if necessary)
In contrary your patch has the following disadvantages:
* The checks are somewhat triggered for parallel execution, but usually you
receive the result from the last call (if the last call to the check is 2hours
in the past, then the result will be 2h old). I really think the
HealthCheckExecutor needs to be the "broker" for futures in order to be able to
achieve the goals as stated in the issue description (e.g. look at
HealthCheckExecutorImpl.waitForFuturesRespectingTimeout() for what you cannot
achieve with your design)
* The HealthCheckExecutor is only capable of running one check at at time - I
believe the main use case for the HC in general is to run all tests and get a
current system health as quickly as possible. No matter how the actual
implementation ends up looking like, I would really like to see the signature
Set<Result> runAllForTags(String... tags) in the interface HealthCheckExecutor
My latest patch is attached, and I think it's better to use that version as a
base for further work.
> Provide a HealthCheckExecutor service
> -------------------------------------
>
> Key: SLING-3278
> URL: https://issues.apache.org/jira/browse/SLING-3278
> Project: Sling
> Issue Type: New Feature
> Components: Health Check
> Reporter: Georg Henzler
> Assignee: Georg Henzler
> Attachments: SLING-3278-bertrand.patch,
> SLING-3278-hc.core-HealthCheckExecutorService-2013-12-18.patch,
> SLING-3278-hc.core-HealthCheckExecutorService-v0.5.patch,
> SLING-3278-hc.webconsole-2013-12-18.patch, SLING-3278-hc.webconsole-v0.5.patch
>
>
> Goals:
> * Be able to get an overall (aggregated) result as quickly as possible
> (ideally <2sec)
> * Whenever possible, return most current results (e.g. for a memory check)
> * Provide a declarative way for async checks (async checks should be the
> exception though)
> Approach
> * Run checks in parallel
> * Make sure long running (or even stuck) checks are timed out
> * If a health check must run asynchronously (because its execution time
> cannot be optimized), it should be enough to just specify a service property
> (e.g. "hc.async").
> See also
> http://apache-sling.73963.n3.nabble.com/Health-Check-Improvements-td4029330.html#a4029402
> http://apache-sling.73963.n3.nabble.com/Health-checks-execution-service-td4028477.html
--
This message was sent by Atlassian JIRA
(v6.1.4#6159)
