On 03/26/2012 11:46 PM, Rainer Jung wrote:
Hi Mladen,
for (i = 0; i< p->num_of_workers; i++) {
lb_sub_worker_t *w =&p->lb_workers[i];
- if (w->sequence != w->s->h.sequence) {
+ if (w->sequence< w->s->h.sequence) {
I think this one is wrong. It is inside push not pull, so it should be if (local > shared)
and not "<".
Might be.
The real question is why we need a difference in sequences for a push?
IMO if the push is needed it should be unconditional, and heap
sequence number should be updated *only* on pull (never directly)
The other changes in this commit look right.
I'm not sure that's the case :)
Think that the entire sequence push/pull should be
reviewed. IMO the only reason for it is to lower the
number of lock/unlock calls.
The reason for this I see only in case of explicit
batch update trough status worker. All other operations
should just depend on volatile shared mem parameters
which should ensure enough atomicity.
Also we should apply the check in this way
if (w->sequence < w->s->h.sequence) {
jk_shm_lock();
/* Now check the value again after we obtained a lock */
if (w->sequence < w->s->h.sequence) {
}
}
The reason for IIS crash is the fact that we can have
N processes updating the shared memory (no parent process,
so no one that would set initial values).
Lock is done per worker basis, and this means that one worker
can update the sequence, other will see it as next generation
although the first one was not done updating all workers from
config and then you have ka-boom.
This makes the entire sequence based push/pull conceptually
wrong, and we need to think of something else.
Regards
--
^TM
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
