Adam Lally wrote:
<snip>
I think in fact there is no need for a bounded-size work queue, since
we have a limited CAS pool size.  IIRC the queue sizes used to be
settable in the CPE descriptor but no longer are; now they are all set
to the CAS pool size.  Thus we never hit the condition where a thread
is trying to add something to a queue and the queue is full.

I agree - I did a check and the queue size is the pool size which comes from
a parameter in the CPE descriptor.  Based on this, I think we should remove
all the code related to "max-sizing" this queue.


However, we may hit the case where multiple threads are trying to
dequeue something but the queue is empty.  That seems like it would
happen if the CAS Consumers are slow, so     that the queue feeding
the Analysis Engine pipelines would be empty and all of the pipelines
would be trying to dequeue from it simultaneously.  So it seems like
the dequeue() returning null issue could arise in pratice, unless
there are further checks for null by the caller, which I did not look
for.  My guess is those checks are there, that's the only explanation
I can think of for why this didn't result in a serious bug.

The caller checks for null, and if null, just retries.  The timeout used
is the output queue "dequeue timeout" which is only used when you
are using the "chunker" queue manager which tries to reassemble
chunks of CASes in order. Otherwise it just waits forever, and if
the timer expires earlier due to notifyAll, because this is in a retry loop,
it just goes around the loop.

So the fault at this point is that setting the dequeue timeout won't have
the desired effect of timing out things in this case.  (Note: the dequeue
timeout will have the effect of causing a "FINEST" log message, but
that's it, as far as I can see).

I'm not sure what's reasonable to do at this moment re: all of this...

-Marshall

Reply via email to