You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by Georg Henzler <sl...@cq-eclipse-plugin.net> on 2014/01/01 16:57:46 UTC

Re: HealthCheckExecutor.execute(ServiceReference)??

I updated the wiki page at 
https://cwiki.apache.org/confluence/display/SLING/Health+Checks+Executor+Design 
with API Variant B: It is mostly in line with the current version in 
SVN with the two changes I already suggested. Carsten, WDYT?

>
> That might work...OTOH I want to have the execution timeout specified
> in the execute call, as being able to change it is very useful when
> troubleshooting things from the webconsole.

I don't think it is that useful to be able to specify a timeout in the 
web console HC plugin for the following reasons:
* Health check futures are never canceled, they always keep running 
until they are finished even if the executor returns a timeout => it is 
possible to set the log level of the HC to debug and check the log file 
even after the web console returned a timeout
* If there is trouble, it is usually necessary to have a look at the 
proper log files anyway (for instance, a HC result is currently not 
capable of carrying an exception)
* If really a longer timeout is needed, it is always possible to 
configure a higher timeout in the OSGi config of the HealthCheckExecutor 
(this is always possible in DEV/TEST envs)

-Georg


Re: HealthCheckExecutor.execute(ServiceReference) ??

Posted by Bertrand Delacretaz <bd...@apache.org>.
On Tue, Jan 14, 2014 at 3:49 PM, Carsten Ziegeler <cz...@apache.org> wrote:
> Adding a value to the enum would be a change in the api, which I would like
> to avoid.

ok, let's stay with boolean hasTimedOut() as is now.

-Bertrand

Re: HealthCheckExecutor.execute(ServiceReference) ??

Posted by Carsten Ziegeler <cz...@apache.org>.
Adding a value to the enum would be a change in the api, which I would like
to avoid.
And if you're just interested if execution is successful, returning a warn
looks like a good idea to me.

Carsten


2014/1/14 Bertrand Delacretaz <bd...@apache.org>

> On Tue, Jan 14, 2014 at 3:34 PM, Carsten Ziegeler <cz...@apache.org>
> wrote:
> > ...what about adding a boolean method, hasTimedOut() instead and always
> > return a correct date?...
>
> why not but then hasTimedOut() really means "no result available" so
> isn't the NO_RESULT status that we discussed earlier clearer and
> simpler?
>
> -Bertrand
>



-- 
Carsten Ziegeler
cziegeler@apache.org

Re: HealthCheckExecutor.execute(ServiceReference) ??

Posted by Bertrand Delacretaz <bd...@apache.org>.
On Tue, Jan 14, 2014 at 3:34 PM, Carsten Ziegeler <cz...@apache.org> wrote:
> ...what about adding a boolean method, hasTimedOut() instead and always
> return a correct date?...

why not but then hasTimedOut() really means "no result available" so
isn't the NO_RESULT status that we discussed earlier clearer and
simpler?

-Bertrand

Re: HealthCheckExecutor.execute(ServiceReference) ??

Posted by Carsten Ziegeler <cz...@apache.org>.
Hmm, that doesn't look right to me - in that case I would need to compare
the date with an (arbitrary) fixed value.
I see the point for the null check
So what about adding a boolean method, hasTimedOut() instead and always
return a correct date?

Carsten


2014/1/14 Bertrand Delacretaz <bd...@apache.org>

> Hi,
>
> On Fri, Jan 10, 2014 at 3:57 PM, Carsten Ziegeler <cz...@apache.org>
> wrote:
> > ...the interesting case still is b) - I see the point that we return WARN
> > in that case - I would assume that getElapsedTimeInMs returns -1 and
> > getFinishedAt() returns null. So both could be used as an indicator for
> > this situation....
>
> Turns out that I had an unsent draft on this, sorry...I agree in
> principle, WARN is ok to indicate "no result available".
>
> I'd return the epoch (new Date(1L) ?) for getFinishedAt() instead of
> null which requires you to check for null everywhere you use that.
>
> Returning
>



-- 
Carsten Ziegeler
cziegeler@apache.org

Re: HealthCheckExecutor.execute(ServiceReference) ??

Posted by Bertrand Delacretaz <bd...@apache.org>.
Hi,

On Fri, Jan 10, 2014 at 3:57 PM, Carsten Ziegeler <cz...@apache.org> wrote:
> ...the interesting case still is b) - I see the point that we return WARN
> in that case - I would assume that getElapsedTimeInMs returns -1 and
> getFinishedAt() returns null. So both could be used as an indicator for
> this situation....

Turns out that I had an unsent draft on this, sorry...I agree in
principle, WARN is ok to indicate "no result available".

I'd return the epoch (new Date(1L) ?) for getFinishedAt() instead of
null which requires you to check for null everywhere you use that.

Returning

Re: HealthCheckExecutor.execute(ServiceReference) ??

Posted by Carsten Ziegeler <cz...@apache.org>.
Answering my own question, I think its good if we always return the elapsed
time, but return null for getFinishedDate.

I'll do the change

Carsten


2014/1/10 Carsten Ziegeler <cz...@apache.org>

> I'Ve done the suggested changes:
> - moved jmx stuff into the core
> - changed the signature to execute(String... tags)
> - Added service reference to metadata
>
> The only thing I'm unclear with are the different result possibilites. I
> think there are these cases when execute is called:
> a) no result in the cache/cached result is obsolete, hc executes within
> the timeout -> hc is executed, real result is cached and returned
> b) no result in the cache/cached result is obsolete, hc executes not
> within the timeout -> hc is executed, fake result is returned
> c) valid result in the cache -> hc is not executed, cached result is
> returned
>
> I think there is no need for a client to find out whether it's case a) or
> c) - if interested client code can inspect the getFinishedAt() method.
> Now, the interesting case still is b) - I see the point that we return
> WARN in that case - I would assume that getElapsedTimeInMs returns -1 and
> getFinishedAt() returns null. So both could be used as an indicator for
> this situation.
>
> WDYT?
>
> Regards
> Carsten
>
>
> 2014/1/5 Georg Henzler <sl...@cq-eclipse-plugin.net>
>
>> Hi,
>>
>> I think this looks pretty good now! Thanks for adding HealthCheckMetadata
>> :)
>>
>> 1. "b) merge the jmx implementation into the core"
>> I'm happy with that and the signature execute(String ... tags) is my
>> preferred
>>
>> 2. Status NO_RESULT
>> I think it's probably cleaner to add a property hasTimedOut to the
>> ExecutionResult - this is also better for the case that we can return an
>> old result from cache. The main disadvantage of NO_RESULT is that it is not
>> immediately clear from a caller's perspective, "how bad" the status
>> NO_RESULT is. A naive caller might think this is sort of ok (GREEN) but I
>> think it really is WARN (as the timeout is configured in a way that should
>> be sufficient for all checks). Therefore explicitly returning WARN plus
>> hasTimedOut=true in the ExecutionResult is IMHO the clearer option from a
>> consumer's point of view.
>>
>> 3. Execution Timeouts defined by caller / "executing the HCs tagged with
>> external ... with longer timeout"
>> The globally defined timeout is sufficient, I agree with Carsten. The
>> "external HCs" example is valid and I would define the global timeout to
>> fit to the external checks, the point is that for the "internal HCs" you
>> don't need to define a separate timeout because
>>  * they are usually quick - and if they aren't, they should probably be
>> rewritten! ;-)
>>  * from a conceptual point of view, the maximum time you are prepared to
>> wait is still the same as for the long running ones (why would you set the
>> timeout for internal checks to 500ms? If the checks are quick it makes no
>> difference, if a check requires 800ms I believe you are still better off
>> getting a proper result after 800ms instead of a timeout one after 500ms).
>> Anyway, as suggested let's leave it for later if we find a good use case
>> that justifies the more complex API.
>>
>> 4. HealthCheckDescriptor is obsolete?
>> The current version in SVN uses HealthCheckDescriptor and
>> HealthCheckMetadata in utils - you probably created it that way to not have
>> the property serviceReference in HealthCheckMetadata. On the other hand
>> HealthCheckMetadata is taking a service reference as constructor argument
>> anyway - why not just merge the two classes in HealthCheckMetadata and get
>> rid of HealthCheckDescriptor?
>>
>> 5. Have you had a look at https://issues.apache.org/
>> jira/browse/SLING-3302 ? The tabular result could also include the
>> property hasTimedOut from 2.
>>
>> Best Regards
>> Georg
>>
>> Am 03.01.2014 17:29, schrieb Carsten Ziegeler:
>>
>>  Yes, I would prefer a separate state, parsing the log is too error prone.
>>>
>>> Carsten
>>>
>>>
>>> 2014/1/3 Bertrand Delacretaz <bd...@apache.org>
>>>
>>>  Hi,
>>>>
>>>> On Fri, Jan 3, 2014 at 3:28 PM, Carsten Ziegeler <cz...@apache.org>
>>>> wrote:
>>>> > Bertrand wrote:
>>>> >> As is you can compute the age of a result, that should already help.
>>>> >>
>>>> > Hmm and what about the case, where there is no result yet and the HC
>>>> takes
>>>> > longer than the timeout?
>>>>
>>>> In my prototype I returned an HEALTH_CHECK_ERROR status for that, with
>>>> a Result that includes a log that explains the problem. Haven't
>>>> checked what the current implementation does.
>>>>
>>>> We could also return a specific NO_RESULT state in this case.
>>>>
>>>> -Bertrand
>>>>
>>>>
>>
>
>
> --
> Carsten Ziegeler
> cziegeler@apache.org
>



-- 
Carsten Ziegeler
cziegeler@apache.org

Re: HealthCheckExecutor.execute(ServiceReference) ??

Posted by Carsten Ziegeler <cz...@apache.org>.
I'Ve done the suggested changes:
- moved jmx stuff into the core
- changed the signature to execute(String... tags)
- Added service reference to metadata

The only thing I'm unclear with are the different result possibilites. I
think there are these cases when execute is called:
a) no result in the cache/cached result is obsolete, hc executes within the
timeout -> hc is executed, real result is cached and returned
b) no result in the cache/cached result is obsolete, hc executes not within
the timeout -> hc is executed, fake result is returned
c) valid result in the cache -> hc is not executed, cached result is
returned

I think there is no need for a client to find out whether it's case a) or
c) - if interested client code can inspect the getFinishedAt() method.
Now, the interesting case still is b) - I see the point that we return WARN
in that case - I would assume that getElapsedTimeInMs returns -1 and
getFinishedAt() returns null. So both could be used as an indicator for
this situation.

WDYT?

Regards
Carsten


2014/1/5 Georg Henzler <sl...@cq-eclipse-plugin.net>

> Hi,
>
> I think this looks pretty good now! Thanks for adding HealthCheckMetadata
> :)
>
> 1. "b) merge the jmx implementation into the core"
> I'm happy with that and the signature execute(String ... tags) is my
> preferred
>
> 2. Status NO_RESULT
> I think it's probably cleaner to add a property hasTimedOut to the
> ExecutionResult - this is also better for the case that we can return an
> old result from cache. The main disadvantage of NO_RESULT is that it is not
> immediately clear from a caller's perspective, "how bad" the status
> NO_RESULT is. A naive caller might think this is sort of ok (GREEN) but I
> think it really is WARN (as the timeout is configured in a way that should
> be sufficient for all checks). Therefore explicitly returning WARN plus
> hasTimedOut=true in the ExecutionResult is IMHO the clearer option from a
> consumer's point of view.
>
> 3. Execution Timeouts defined by caller / "executing the HCs tagged with
> external ... with longer timeout"
> The globally defined timeout is sufficient, I agree with Carsten. The
> "external HCs" example is valid and I would define the global timeout to
> fit to the external checks, the point is that for the "internal HCs" you
> don't need to define a separate timeout because
>  * they are usually quick - and if they aren't, they should probably be
> rewritten! ;-)
>  * from a conceptual point of view, the maximum time you are prepared to
> wait is still the same as for the long running ones (why would you set the
> timeout for internal checks to 500ms? If the checks are quick it makes no
> difference, if a check requires 800ms I believe you are still better off
> getting a proper result after 800ms instead of a timeout one after 500ms).
> Anyway, as suggested let's leave it for later if we find a good use case
> that justifies the more complex API.
>
> 4. HealthCheckDescriptor is obsolete?
> The current version in SVN uses HealthCheckDescriptor and
> HealthCheckMetadata in utils - you probably created it that way to not have
> the property serviceReference in HealthCheckMetadata. On the other hand
> HealthCheckMetadata is taking a service reference as constructor argument
> anyway - why not just merge the two classes in HealthCheckMetadata and get
> rid of HealthCheckDescriptor?
>
> 5. Have you had a look at https://issues.apache.org/jira/browse/SLING-3302? The tabular result could also include the property hasTimedOut from 2.
>
> Best Regards
> Georg
>
> Am 03.01.2014 17:29, schrieb Carsten Ziegeler:
>
>  Yes, I would prefer a separate state, parsing the log is too error prone.
>>
>> Carsten
>>
>>
>> 2014/1/3 Bertrand Delacretaz <bd...@apache.org>
>>
>>  Hi,
>>>
>>> On Fri, Jan 3, 2014 at 3:28 PM, Carsten Ziegeler <cz...@apache.org>
>>> wrote:
>>> > Bertrand wrote:
>>> >> As is you can compute the age of a result, that should already help.
>>> >>
>>> > Hmm and what about the case, where there is no result yet and the HC
>>> takes
>>> > longer than the timeout?
>>>
>>> In my prototype I returned an HEALTH_CHECK_ERROR status for that, with
>>> a Result that includes a log that explains the problem. Haven't
>>> checked what the current implementation does.
>>>
>>> We could also return a specific NO_RESULT state in this case.
>>>
>>> -Bertrand
>>>
>>>
>


-- 
Carsten Ziegeler
cziegeler@apache.org

Re: HealthCheckExecutor.execute(ServiceReference)??

Posted by Georg Henzler <sl...@cq-eclipse-plugin.net>.
Hi,

I think this looks pretty good now! Thanks for adding 
HealthCheckMetadata :)

1. "b) merge the jmx implementation into the core"
I'm happy with that and the signature execute(String ... tags) is my 
preferred

2. Status NO_RESULT
I think it's probably cleaner to add a property hasTimedOut to the 
ExecutionResult - this is also better for the case that we can return an 
old result from cache. The main disadvantage of NO_RESULT is that it is 
not immediately clear from a caller's perspective, "how bad" the status 
NO_RESULT is. A naive caller might think this is sort of ok (GREEN) but 
I think it really is WARN (as the timeout is configured in a way that 
should be sufficient for all checks). Therefore explicitly returning 
WARN plus hasTimedOut=true in the ExecutionResult is IMHO the clearer 
option from a consumer's point of view.

3. Execution Timeouts defined by caller / "executing the HCs tagged 
with external ... with longer timeout"
The globally defined timeout is sufficient, I agree with Carsten. The 
"external HCs" example is valid and I would define the global timeout to 
fit to the external checks, the point is that for the "internal HCs" you 
don't need to define a separate timeout because
  * they are usually quick - and if they aren't, they should probably be 
rewritten! ;-)
  * from a conceptual point of view, the maximum time you are prepared 
to wait is still the same as for the long running ones (why would you 
set the timeout for internal checks to 500ms? If the checks are quick it 
makes no difference, if a check requires 800ms I believe you are still 
better off getting a proper result after 800ms instead of a timeout one 
after 500ms).
Anyway, as suggested let's leave it for later if we find a good use 
case that justifies the more complex API.

4. HealthCheckDescriptor is obsolete?
The current version in SVN uses HealthCheckDescriptor and 
HealthCheckMetadata in utils - you probably created it that way to not 
have the property serviceReference in HealthCheckMetadata. On the other 
hand HealthCheckMetadata is taking a service reference as constructor 
argument anyway - why not just merge the two classes in 
HealthCheckMetadata and get rid of HealthCheckDescriptor?

5. Have you had a look at 
https://issues.apache.org/jira/browse/SLING-3302 ? The tabular result 
could also include the property hasTimedOut from 2.

Best Regards
Georg

Am 03.01.2014 17:29, schrieb Carsten Ziegeler:
> Yes, I would prefer a separate state, parsing the log is too error 
> prone.
>
> Carsten
>
>
> 2014/1/3 Bertrand Delacretaz <bd...@apache.org>
>
>> Hi,
>>
>> On Fri, Jan 3, 2014 at 3:28 PM, Carsten Ziegeler 
>> <cz...@apache.org>
>> wrote:
>> > Bertrand wrote:
>> >> As is you can compute the age of a result, that should already 
>> help.
>> >>
>> > Hmm and what about the case, where there is no result yet and the 
>> HC
>> takes
>> > longer than the timeout?
>>
>> In my prototype I returned an HEALTH_CHECK_ERROR status for that, 
>> with
>> a Result that includes a log that explains the problem. Haven't
>> checked what the current implementation does.
>>
>> We could also return a specific NO_RESULT state in this case.
>>
>> -Bertrand
>>


Re: HealthCheckExecutor.execute(ServiceReference) ??

Posted by Carsten Ziegeler <cz...@apache.org>.
Yes, I would prefer a separate state, parsing the log is too error prone.

Carsten


2014/1/3 Bertrand Delacretaz <bd...@apache.org>

> Hi,
>
> On Fri, Jan 3, 2014 at 3:28 PM, Carsten Ziegeler <cz...@apache.org>
> wrote:
> > Bertrand wrote:
> >> As is you can compute the age of a result, that should already help.
> >>
> > Hmm and what about the case, where there is no result yet and the HC
> takes
> > longer than the timeout?
>
> In my prototype I returned an HEALTH_CHECK_ERROR status for that, with
> a Result that includes a log that explains the problem. Haven't
> checked what the current implementation does.
>
> We could also return a specific NO_RESULT state in this case.
>
> -Bertrand
>



-- 
Carsten Ziegeler
cziegeler@apache.org

Re: HealthCheckExecutor.execute(ServiceReference) ??

Posted by Bertrand Delacretaz <bd...@apache.org>.
Hi,

On Fri, Jan 3, 2014 at 3:28 PM, Carsten Ziegeler <cz...@apache.org> wrote:
> Bertrand wrote:
>> As is you can compute the age of a result, that should already help.
>>
> Hmm and what about the case, where there is no result yet and the HC takes
> longer than the timeout?

In my prototype I returned an HEALTH_CHECK_ERROR status for that, with
a Result that includes a log that explains the problem. Haven't
checked what the current implementation does.

We could also return a specific NO_RESULT state in this case.

-Bertrand

Re: HealthCheckExecutor.execute(ServiceReference) ??

Posted by Carsten Ziegeler <cz...@apache.org>.
2014/1/3 Bertrand Delacretaz <bd...@apache.org>

> Hi,
>
> On Fri, Jan 3, 2014 at 2:12 PM, Carsten Ziegeler <cz...@apache.org>
> wrote:
> > ...I'm not so sure about timeouts by the caller - I would assume that the
> > configured timeout for the executor is a sensible value. If you want to
> > specify something else for a specific client, this client would need to
> > know which HCs it's executing, how long it takes etc....
>
> Yes, my use case is executing the HCs tagged with "external" for
> example, I know those take longer than the performance counters for
> example, so I set a longer timeout for their execution to have a
> better chance of having fresh results.
>
> > ...We have discussed other things like forcing a HC to run (and not to
> use the
> > cached value), I'm not sure about those either....
>
> This is less clear for me as well.
>
> > ...We could think about adding an Options argument to the execute method
> where
> > we add getter/setters for these things....
>
> Yes, we can add a second execute method later that takes options.
>
> Ok, if we already know about some options we could add this already now.
Or we gather first some more expierence and add it later :)


> >
> > ...But I think we need a clear result if the timeout is reached to allow
> > clients to correctly display this and retry at a later point of time....
>
> As is you can compute the age of a result, that should already help.
>

Hmm and what about the case, where there is no result yet and the HC takes
longer than the timeout?

Carsten


> -Bertrand
>



-- 
Carsten Ziegeler
cziegeler@apache.org

Re: HealthCheckExecutor.execute(ServiceReference) ??

Posted by Bertrand Delacretaz <bd...@apache.org>.
Hi,

On Fri, Jan 3, 2014 at 2:12 PM, Carsten Ziegeler <cz...@apache.org> wrote:
> ...I'm not so sure about timeouts by the caller - I would assume that the
> configured timeout for the executor is a sensible value. If you want to
> specify something else for a specific client, this client would need to
> know which HCs it's executing, how long it takes etc....

Yes, my use case is executing the HCs tagged with "external" for
example, I know those take longer than the performance counters for
example, so I set a longer timeout for their execution to have a
better chance of having fresh results.

> ...We have discussed other things like forcing a HC to run (and not to use the
> cached value), I'm not sure about those either....

This is less clear for me as well.

> ...We could think about adding an Options argument to the execute method where
> we add getter/setters for these things....

Yes, we can add a second execute method later that takes options.

>
> ...But I think we need a clear result if the timeout is reached to allow
> clients to correctly display this and retry at a later point of time....

As is you can compute the age of a result, that should already help.

-Bertrand

Re: HealthCheckExecutor.execute(ServiceReference) ??

Posted by Carsten Ziegeler <cz...@apache.org>.
I haven't thought about this yet - but I tend to agree that a health check
(or someone else) needs a way to define how long a result should be cached.
This could just be another service registration property as I don't think
that this is a highly dynamic value which changes from execution to
execution of a single HC.

I'm not so sure about timeouts by the caller - I would assume that the
configured timeout for the executor is a sensible value. If you want to
specify something else for a specific client, this client would need to
know which HCs it's executing, how long it takes etc.
We have discussed other things like forcing a HC to run (and not to use the
cached value), I'm not sure about those either.
We could think about adding an Options argument to the execute method where
we add getter/setters for these things.

But I think we need a clear result if the timeout is reached to allow
clients to correctly display this and retry at a later point of time.

Carsten


2014/1/3 Bertrand Delacretaz <bd...@apache.org>

> Hi,
>
> On Wed, Jan 1, 2014 at 4:57 PM, Georg Henzler
> <sl...@cq-eclipse-plugin.net> wrote:
> > ...I don't think it is that useful to be able to specify a timeout in
> the web
> > console HC plugin for the following reasons...
>
> I'm still not convinced, IMO we are very likely to need
>
> a) Results that don't all have the same time to live. The Health Check
> that creates the result would set the TTL.
>
> b) Execution timeouts specified by the caller, to cope with various
> types of health checks and clients
>
> Both are easy to add later however, so if you guys don't want to add
> them now I'm fine.
>
> -Bertrand
>



-- 
Carsten Ziegeler
cziegeler@apache.org

Re: HealthCheckExecutor.execute(ServiceReference) ??

Posted by Bertrand Delacretaz <bd...@apache.org>.
Hi,

On Wed, Jan 1, 2014 at 4:57 PM, Georg Henzler
<sl...@cq-eclipse-plugin.net> wrote:
> ...I don't think it is that useful to be able to specify a timeout in the web
> console HC plugin for the following reasons...

I'm still not convinced, IMO we are very likely to need

a) Results that don't all have the same time to live. The Health Check
that creates the result would set the TTL.

b) Execution timeouts specified by the caller, to cope with various
types of health checks and clients

Both are easy to add later however, so if you guys don't want to add
them now I'm fine.

-Bertrand

Re: HealthCheckExecutor.execute(ServiceReference) ??

Posted by Carsten Ziegeler <cz...@apache.org>.
Yes, only a single execute method with the tags.

Carsten


2014/1/3 Bertrand Delacretaz <bd...@apache.org>

> Hi Carsten,
>
> On Fri, Jan 3, 2014 at 9:22 AM, Carsten Ziegeler <cz...@apache.org>
> wrote:
> > ...I think it makes sense to move the
> > jmx stuff into the core, this keeps the executor service interface
> cleaner....
>
> So IIUC if we move jmx into core the HealthCheckExecutor would have just
>
>   Collection<HealthCheckExecutionResult> execute(String ... tags);
>
> ?
>
> If yes I'm +1 on the move, it doesn't add any dependencies to core.
>
> -Bertrand
>



-- 
Carsten Ziegeler
cziegeler@apache.org

Re: HealthCheckExecutor.execute(ServiceReference) ??

Posted by Bertrand Delacretaz <bd...@apache.org>.
Hi Carsten,

On Fri, Jan 3, 2014 at 9:22 AM, Carsten Ziegeler <cz...@apache.org> wrote:
> ...I think it makes sense to move the
> jmx stuff into the core, this keeps the executor service interface cleaner....

So IIUC if we move jmx into core the HealthCheckExecutor would have just

  Collection<HealthCheckExecutionResult> execute(String ... tags);

?

If yes I'm +1 on the move, it doesn't add any dependencies to core.

-Bertrand

Re: HealthCheckExecutor.execute(ServiceReference) ??

Posted by Carsten Ziegeler <cz...@apache.org>.
If I look at the use cases, we have either executing HCs based on tags or
the JMX support which executes single HCs. For the jmx stuff we need to
pass the service reference into the executor - for all other use cases,
just providing the tags is sufficient.

So basically we have two options:
a) keep the service reference in the signature of the HC
b) merge the jmx implementation into the core

While I like to keep things separate, I think it makes sense to move the
jmx stuff into the core, this keeps the executor service interface cleaner.
And the jmx name constant to register a HC as a JMX bean is defined in the
core anyways.

WDYT?

Regards
Carsten


2014/1/1 Georg Henzler <sl...@cq-eclipse-plugin.net>

>
> I updated the wiki page at https://cwiki.apache.org/
> confluence/display/SLING/Health+Checks+Executor+Design with API Variant
> B: It is mostly in line with the current version in SVN with the two
> changes I already suggested. Carsten, WDYT?
>
>
>
>> That might work...OTOH I want to have the execution timeout specified
>> in the execute call, as being able to change it is very useful when
>> troubleshooting things from the webconsole.
>>
>
> I don't think it is that useful to be able to specify a timeout in the web
> console HC plugin for the following reasons:
> * Health check futures are never canceled, they always keep running until
> they are finished even if the executor returns a timeout => it is possible
> to set the log level of the HC to debug and check the log file even after
> the web console returned a timeout
> * If there is trouble, it is usually necessary to have a look at the
> proper log files anyway (for instance, a HC result is currently not capable
> of carrying an exception)
> * If really a longer timeout is needed, it is always possible to configure
> a higher timeout in the OSGi config of the HealthCheckExecutor (this is
> always possible in DEV/TEST envs)
>
> -Georg
>
>


-- 
Carsten Ziegeler
cziegeler@apache.org