Le 24/08/2018 à 18:21, Eric Covener a écrit :
I've only found it in mod_proxy_balancer and, IIUC, the meaning is "slightly" different from its use in hcheck! :) Looks like this 'updated' field was dedicated for recording the time a worker has been added.On Fri, Aug 24, 2018 at 12:13 PM Christophe JAILLET <christophe.jaillet@xxxxxxxxxx> wrote:Le 24/08/2018 à 17:56, Christophe JAILLET a écrit :Le 24/08/2018 à 16:40, Jim Jagielski a écrit :I was wondering if someone wanted to provide a sanity check on the above PR and what's "expected" by the health check code. It would be very easy to adjust so that hcinterval was not the time between successive checks but the interval between the end of one and the start of another, but I'm not sure that is as useful. In other words, I think the current behavior is right (but think the docs need to be updated), but am willing to have my mind changed :)Hi Jim, the current behavior is also what I would expect. If I configure a check every 10s, I would expect 6 checks each minute, even if the test itself takes time to perform. Not related, but is there any use for 'hc_pre_config()'? We already have: static int tpsize = HC_THREADPOOL_SIZE; Having both looks redundant. CJbut shouldn't we worker->s->update = now; when the check is started (in hc_watchdog_callback()) instead of when it is funished (at the end of hc_check())?Looks like s->updated is not used elsewhere in HC but is used elsewhere in proxy modules and is in the API. I don't know if that calls for a 2nd timestamp or a just a bit for when checks are in progress. Could be useful in the future to keep track of the addl information.
So, my understanding is that, either:- hcheck already changed the meaning of this field, and broke the API when it has been introduced.
or- the API only says that 'updated' is "timestamp of last update", without telling which kind of update! So why couldn't be used by hcheck to keep record of the "timestamp of last update"... of its check?
I still think that moving when s->updated is updated (sic!) in hcheck should be OK, and wouldn't be an API breakage for me. I don't thing that it can interfere in any way with mod_proxy_balancer, at least with the actual code. And we should clarify what is the use of thee fields to avoid someelse to 'steal' them.
just my 2c. CJ