On 18 Nov 2008, at 20:18, Mark Miller wrote:
Mike Klaas wrote:
autoCommitCount is written in a CommitTracker.synchronized block
only. It is read to print stats in an unsynchronized fashion,
which perhaps could be fixed, though I can't see how it could cause
a problem
lastAddedTime is only written in a call path within a
DirectUpdateHandler2.synchronized block. It is only read in a
CommitTracker.synchronized block. It could read the wrong value,
but I also don't see this causing a problem (a commit might fail to
be scheduled). This could probably also be improved, but doesn't
seem important.
Right. I don't see these as causing a deadlock either, but whatever
happens, its pretty much JVM undefined right, hence 'who
knows' (I'll go with pretty doubtful <g>). I am not so sure its safe
to read a value from an unsynced method whether you care about the
result or not though. Its prob safe for atomic types and volatiles,
but I'm fairly sure your playing with fire doing read/write in and
out of sync. I don't think its just about stale values. But then
again, it probably works 99.9% of the time or something.
pending seems to be the issue. As long as commit are only
triggered by autocommit, there is no issue as manipulation of
pending is always performed inside CommitTracker.synchronized. But
didCommit()/didRollback() could be called via manual commit, and
pending is directly manipulated during DUH2.close(). I'm having
trouble coming up with a plausible deadlock scenario, but this
needs to be fixed. It isn't as easy as synchronizing didCommit/
didRollback, though--this would introduce definite deadlock
scenarios.
Mark, is there any chance you could post the thread dump for the
deadlocked process? Do you issue manual commits during insertion?
Toby reported it. Thread dump Toby?
-Mike
I'll try and post a thread dump when I get to work, can't remote in
from here.
I don't mind helping out with the fix, I've been getting to know
solr's internals quite intimately recently after writing a few
handlers/components for internal projects.
T
