[
https://issues.apache.org/jira/browse/LUCENE-4639?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13536925#comment-13536925
]
Shai Erera commented on LUCENE-4639:
------------------------------------
bq. Perhaps there should be a retry counter here with some upper limit of
retries just to cater for things like access denied errors or disk full errors
(which will make this loop run forever).
This loop doesn't catch exceptions/errors. Access denied and disk full errors
will be thrown further. In general, adding a retry counter is not a bad idea,
but I didn't change the current logic (which looped while !f.createNewFile()).
So I think we can avoid the retry counter for now?
bq. What's this magic for?
I have no idea :). I just copied the lazily init code that was in genTempFile.
At first I thought it increases randomness or uniqueness, but it's not true. if
two randoms return the same int in nextInt(), counter will be set the same. The
division by 65535 and "AND"-ing with 0xFFFF is I guess to keep the number small
and positive.
bq. Use BeforeClass hooks or (better) initialize a volatile lazily.
BeforeClass isn't good I think, b/c this counter is reset for the entire JVM,
and note that baseCounter is served as the JVM identifier. Perhaps
randomized-runner could expose a method which returns the JVM PID and we'd just
use it?
If we go with the lazy initalization, then we're basically back to the previous
code, which synchronized in genTempFile on a lock and did the initialization
trick there. It wasn't bad, I just thought that it's a waste to do this sync
for every file. I have an idea how to avoid the sync unnecessarily. Will post a
new patch soon.
> Improving _TestUtil.getTempDir
> ------------------------------
>
> Key: LUCENE-4639
> URL: https://issues.apache.org/jira/browse/LUCENE-4639
> Project: Lucene - Core
> Issue Type: Bug
> Reporter: Shai Erera
> Assignee: Shai Erera
> Priority: Minor
> Attachments: LUCENE-4639.patch
>
>
> Spinoff from here:
> http://lucene.472066.n3.nabble.com/TestUtil-getTempFile-may-fail-on-quot-Access-Denied-quot-td4028048.html.
> _TestUtil.getTempDir uses createTempFile and then deletes the file. While
> this usually works, if someone runs tests by multiple JVMs and does not
> ensure each JVM gets an isolated temp.dir to work in, that my result in two
> JVMs sharing the same directory.
> Also, on Windows, if you call getTempDir on an existing directory, you get an
> "Access is denied" exception.
> Dawid proposed a simple solution to just call mkdirs() continuously until
> success. I'd like to try that.
> Also, I think that genTempFile could use some house cleaning, e.g.:
> * tempFileLocker can be just an Object instance? Why do we need a class?
> * If we initialize counter and counterBase in a static clause, we can avoid
> checking if counter==0 as well as passing Random to genTempFile (that will
> remove any suspicion that it does anything randomly)
> ** Also, instead of synchronizing on tempFileLocker, can we just use
> AtomicInteger for the counter?
> I'll modify getTempDir first. It documents "does not create the directory", I
> want to make sure no test fails due that.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
