[jira] [Comment Edited] (LUCENE-6770) FSDirectory ctor should use getAbsolutePath instead of getRealPath for directory
[
https://issues.apache.org/jira/browse/LUCENE-6770?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14736362#comment-14736362
]
Uwe Schindler edited comment on LUCENE-6770 at 9/9/15 7:25 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}
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 release a lock on a file in
the same JVM it releases all locks on all FileChannels (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.
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
[jira] [Comment Edited] (LUCENE-6770) FSDirectory ctor should use getAbsolutePath instead of getRealPath for directory
[
https://issues.apache.org/jira/browse/LUCENE-6770?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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
[jira] [Comment Edited] (LUCENE-6770) FSDirectory ctor should use getAbsolutePath instead of getRealPath for directory
[
https://issues.apache.org/jira/browse/LUCENE-6770?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14737164#comment-14737164
]
Uwe Schindler edited comment on LUCENE-6770 at 9/9/15 4:43 PM:
---
bq. So, it calls toRealPath also. Does it mean that it relies on that after
FSDirectory called directory = path.toRealPath(); second call
directory.toRealPath() doesn't resolve it but returns some internal state? I
wasn't able to find this at documentation.
This is just a test if the previous Files.createFile() worked (as this
suppresses Exceptions). If the file was already existing and was something
else, the toRealPath() call fails and we can bail out.
was (Author: thetaphi):
bq. So, it calls toRealPath also. Does it mean that it relies on that after
FSDirectory called directory = path.toRealPath(); second call
directory.toRealPath() doesn't resolve it but returns some internal state? I
wasn't able to find this at documentation.
This is just a test if the previous Files.createFile() worked (as this
suppresses Exceptions). If the file was already existing or was something else,
the toRealPath() call fails.
> 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
[jira] [Comment Edited] (LUCENE-6770) FSDirectory ctor should use getAbsolutePath instead of getRealPath for directory
[
https://issues.apache.org/jira/browse/LUCENE-6770?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14731290#comment-14731290
]
Uwe Schindler edited comment on LUCENE-6770 at 9/4/15 7:27 PM:
---
The reason why we have the canonic path is the following: The
NativeFSLockFactory has the limitation that the underlying OS does not lock
files for the same process. It only prevents other processes from using the
locked file/directory. So NativeFSLockFactory internally uses a static set of
"locked index directories" and during aquiring locks it first checks if the
given directory is in the set. If this is true it refuses to aquire lock.
Otherwise it fall backs to OS kernel in checking the lock.
For this check with a simple set to work correctly, the path must be canonic.
If this is not done, it may happen that a user opens in the same JVM an index
with 2 different Path objects (which somehow point to same dir using
symlink/hardlinks/junctions), causing index corrumption.
As getting canonic path is quite expensive, we dont expand it on every try to
lock (which may also break if people change links while having index open). So
we do it on FSDirectory init.
To work around the issue mentioned here, one possibility would be to save the
original Path as given in Ctor and return that one getDirectory(). The canonic
path would be an implementation detail.
was (Author: thetaphi):
The reason why we have the canonic path is the following: The
NativeFSLockFactory has the limitation that the underlying OS does not lock
files for the same process. It only prevents other processes from using the
locked file/directory. So NativeFSLockFactory internally uses a static set of
"locked index directories" and during aquiring locks it first checks if the
given directory is in the set. If this is true it refuses to aquire lock.
Otherwise it fall backs to OS kernel in checking the lock.
For this check with a simple set to work correctly, the path must be canonic.
If this is not done, it may happen that a user opens in the same JVM an index
with 2 different Path objects (which somehow point to same dir using
symlink/hardlinks/junctions), leading broken indexes.
As getting canonic path is quite expensive, we dont expand it on every try to
lock (which may also break if people change links while having index open). So
we do it on FSDirectory init.
To work around the issue mentioned here, one possibility would be to save the
original Path as given in Ctor and return that one getDirectory(). The canonic
path would be an implementation detail.
> 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
> 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
