[ 
https://issues.apache.org/jira/browse/LUCENE-6770?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14736362#comment-14736362
 ] 

Uwe Schindler edited comment on LUCENE-6770 at 9/9/15 7:29 AM:
---------------------------------------------------------------

bq. My concern is that this all internal LOCK_HELD optimization looks useless 
and may even has some unpredictable behavior. I would always delegate it to 
system call.

This is no optimization, it is a bug fix!!! If you close a file channel after 
non-successful locking in the same JVM it releases all locks on all other 
FileChannels,too (on some platforms, including Linux). This caused index 
corruption in some search application, because the lock was unfortunately 
released by this problem: other IndexWriter in same JVM tried to lock a second 
time and released the lock (and unfortunately all other locks) after work done. 
By that another process was able to write to index => BOOM

See LUCENE-5738:
{quote}
Note this from the FileLock documentation 
(http://grepcode.com/file/repository.grepcode.com/java/root/jdk/openjdk/6-b14/java/nio/channels/FileLock.java#28
 )
"On some systems, closing a channel releases all locks held by the Java virtual 
machine on the underlying file regardless of whether the locks were acquired 
via that channel or via another channel open on the same file. It is strongly 
recommended that, within a program, a unique channel be used to acquire all 
locks on any given file."
{quote}

This bug was fixed in Lucene in 4.9 by using the HashSet (we had that long ago, 
too, for similar reasons, so this was a regression). The change in 5.0 is just 
that the canonical path is aquired on FSDirectory ctor, because LockFactories 
are singletons and don't know about directories. In 4.x the canonical patch was 
aquired when creating a lock factory instance for a specific directory.

So having the canonic/real path in a separate variable would mimic the same 
behaviour like 4.x, the "state" is just hold somewhere else.


was (Author: thetaphi):
bq. My concern is that this all internal LOCK_HELD optimization looks useless 
and may even has some unpredictable behavior. I would always delegate it to 
system call.

This is no optimization, it is a bug fix!!! If you close a file channel after 
non-successful locking in the same JVM it releases all locks on all other 
FileChannels,too (on some platforms, including Linux). This caused index 
corruption in some search application, because the lock was unfortunately 
released by this problem: other IndexWriter in same JVM tried to lock a second 
time and released the lock (and unfortunately all other locks) after work done. 
By that another process was able to write to index => BOOM

See LUCENE-5738:
{quote}
Note this from the FileLock documentation 
(http://grepcode.com/file/repository.grepcode.com/java/root/jdk/openjdk/6-b14/java/nio/channels/FileLock.java#28
 )
"On some systems, closing a channel releases all locks held by the Java virtual 
machine on the underlying file regardless of whether the locks were acquired 
via that channel or via another channel open on the same file. It is strongly 
recommended that, within a program, a unique channel be used to acquire all 
locks on any given file."
{quote}

> FSDirectory ctor should use getAbsolutePath instead of getRealPath for 
> directory
> --------------------------------------------------------------------------------
>
>                 Key: LUCENE-6770
>                 URL: https://issues.apache.org/jira/browse/LUCENE-6770
>             Project: Lucene - Core
>          Issue Type: Improvement
>          Components: core/store
>    Affects Versions: 5.2.1
>         Environment: OS X, Linux
>            Reporter: Vladimir Kuzmin
>            Assignee: Uwe Schindler
>         Attachments: LUCENE-6770.patch
>
>
> After upgrade from 4.1 to 5.2.1 I found that one of our test failed. Appeared 
> the guilty was FSDirectory that converts given Path to Path.getRealPath. As 
> result the test will fail:
> Path p = Paths.get("/var/lucene_store");
> FSDirectory d = new FSDirectory(p);
> assertEquals(p.toString(), d.getDirectory().toString());
> It because /var/lucene_store is a symlink and 
> Path directory =path.getRealPath(); 
> resolves it to /private/var/lucene_store
> I think this is bad design decision because "direcrory" isn't just internal 
> state but is exposed in a public interface and "getDirectory()" is widely 
> used to initialize other components. 
> It should use paths.getAbsolutePath() instead.
> build and "ant test" were successful after fix.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org

Reply via email to