I use 1.6.0_18. Maybe seeding w/ System.nanoTime() will help, but don't you think NativeFSLock should be more robust anyway? Even for the regular lock file this can happen if at the same time delete() is attempted the file is held by another process ...
Shai On Wednesday, April 28, 2010, Michael McCandless <[email protected]> wrote: > OK... > > But this must mean that "new Random()" selects a bad (easily > conflicts) seed? Which JRE are you using? > > Maybe it's just using System.currentTimeMills()? This is what the > javadocs state for JDK 1.4, but for JDK 1.5 it says it tries to pick > something unique :) > > Maybe we should seed w/ System.nanoTime(), when we create the test lock name? > > Mike > > On Wed, Apr 28, 2010 at 1:24 PM, Shai Erera <[email protected]> wrote: >> Mike - I think I'm pretty sure that's what happened. The reason is >> that even w/ the reported failure, the lock dir is empty when the >> tests finish and the lock file isn't there. I believe that if the >> collision was not the case, then I should have seen the test lock file >> in there? >> >> So overall these changes will 99.9% of the time delete the lock file. >> It's in those (I agree) super rare cases that the rest of the code >> will be invoked. >> >> In addition, I don't have any other explanation to why this sometimes >> happens, all started after the tests parallelism. And since then it >> happened too many times ... >> >> Shai >> >> On Wednesday, April 28, 2010, Michael McCandless >> <[email protected]> wrote: >>> Nice! >>> >>> Mike >>> >>> On Wed, Apr 28, 2010 at 12:54 PM, Robert Muir <[email protected]> wrote: >>>> As far as the build system goes, I implemented the two ideas mentioned >>>> earlier in this message (not creating a new Formatter for each test, and >>>> not >>>> spawning 26 jvms for each batch) >>>> >>>> Jira is down, but if you want to help test you can try a patch here: >>>> http://pastebin.com/iqwb73H2 (click Raw/Download) >>>> >>>> Additionally this cuts 1:20 off the total Solrcene 'ant clean test' for me. >>>> >>>> before: >>>> BUILD SUCCESSFUL >>>> Total time: 7 minutes 42 seconds >>>> >>>> after: >>>> BUILD SUCCESSFUL >>>> Total time: 6 minutes 23 seconds >>>> >>>> On Wed, Apr 28, 2010 at 12:25 PM, Michael McCandless >>>> <[email protected]> wrote: >>>>> >>>>> I think this are good changes to NativeFSLockFactory. >>>>> >>>>> But: the chances that N JVMs launched at once would conflict on the >>>>> randomly generated lock file name should be miniscule... though it >>>>> does depend on how good new Random() is at seeding itself. Do we >>>>> really think this explains your exceptions Shai? (And, if so, even w/ >>>>> these changes, the conflict could still happen?) Maybe we should >>>>> explicitly seed it? >>>>> >>>>> Mike >>>>> >>>>> On Wed, Apr 28, 2010 at 11:22 AM, Shai Erera <[email protected]> wrote: >>>>> > I'd like to summarize the IRC discussion Mark and I had: >>>>> > >>>>> > The lock file's existence in the directory should not fail obtain() from >>>>> > retrieving obtaining a lock. That's the whole difference between Simple >>>>> > and >>>>> > Native. So we should make a best-effort to delete it. If the delete >>>>> > fails on >>>>> > release(), then ok. On obtain(), we won't return false if the lock >>>>> > exists, >>>>> > but attempt to really obtain it and fail appropriately. >>>>> > >>>>> > While the previously proposed fix (add "&& path.exists()" to release()) >>>>> > might work most of the times, it will only work "most of the times". >>>>> > I.e., >>>>> > between release() and delete(), an external process, like AntiVirus, >>>>> > might >>>>> > lock the file, and delete will fail, but the file will still be there, >>>>> > and >>>>> > we'll throw an exception still. >>>>> > >>>>> > So, the proposed changes are: >>>>> > * release() is allowed to fail to delete the lock file. >>>>> > * obtain() should not return false if the lock file exists - it should >>>>> > really attempt to obtain it. >>>>> > * in acquireTestLock(), if after release() is called, the lock file >>>>> > still >>>> --------------------------------------------------------------------- >> To unsubscribe, e-mail: [email protected] >> For additional commands, e-mail: [email protected] >> >> > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > > --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
