[
https://issues.apache.org/jira/browse/SLING-3278?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13851862#comment-13851862
]
Bertrand Delacretaz commented on SLING-3278:
--------------------------------------------
Competition is good! That's how we get excellent code ;-)
Thanks Georg for your revised patch, I see your point about parallel execution
of the HC, agree that it's a good thing and that's the cause for some
additional complexity w.r.t my variant. but it's worth it.
Here are my comments in no particular order.
I agree with the reformatting issue that Carsten mentions.
IMO the Result timing information should be:
* Result creation time, set automatically in constructor
* Optional time to live, can be set with a method but not changed after that
* Optional HC execution elapsed time, can be set with a method but not changed
after that
* isExpired() method that uses creation time + time to live
We do not use @author tags, as is customary in Apache projects. You will get
credit in the commit message but we don't want code to appear like it belongs
to specific people, especially as over time this changes.
The it tests fail, I'll attach a patch that fixes them.
I'd still like to remove the async execution from this patch, and rediscuss on
list. My use case would be to execute some HC at regular intervals based on
their tags, instead of based on individual HC configurations. This can then be
implemented in the support bundle.
We need to keep the CompositeHealthCheck as we already released it, I agree
that it can lead to executing some health checks several times if you don't
choose tags wisely. That's not really a problem, and even less with this
improved execution mechanism as results are cached. It shouldn't be hard to
adapt CompositeHealthCheck to use the new executor.
The maven-sling-plugin shouldn't be added to the pom.xml in this patch - the
Sling parent pom as an autoInstallBundle profile which does that already.
The SlowHealthCheck demo HC from my patch should be included once we apply your
patch, with its config as that's a useful demo for caching results and
execution timeout.
> 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)
