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

Michael McCandless commented on LUCENE-1658:
--------------------------------------------

{quote}
In my opinion, the cached directories vs. instantiated directories have one big 
advantage:
They are forced to use the same locking mechanism. So if somebody creates a 
directory using one LockFactory, writes to the index and in a parallel thread 
uses another locking mechanism with a separate dir instance, he corrupts his 
index. So from that point of view, only have one directory instance per 
resource is a good thing (it does not work from different JVM processes, sure).
{quote}

I agree.

But, I don't think this is a strong enough reason for Lucene to be
doing such magic under-the-hood, going forward.  This magic leads to
other problems (like LUCENE-1453).

{quote}
> Ie, it's only if you use the new FSDir.open() API that you get the new 
> behavior. I intentionally went and fixed tests to use FSDir.open so that we 
> stress the new functionality, which then led us to discover tests making 
> invalid assumptions, which we should then fix.

This is correct. For unit testing, I found out now, that it is much simplier to 
check, if all tests would also work with other platforms, if you set the FSDir 
system property when running the tests. With open() this is currently not 
possible.
{quote}

Excellent!

Before committing we should confirm all tests pass if we temporarily
hardwire open to return each of the 3 FSDir impls.

But I don't think this is reason enough to leave the global system
property in place for real usage of Lucene.


bq. Maybe I un-comment-out the caching again, but let getDirectory still use 
the new behaviour, if the system property is not set. We could then in 3.0 just 
remove the caching, but let getDirectory() alive. I am not sure.

But you've still unnecessarily broken back-compat with that.  By
making a new method (open), which does neither the magic singleton
caching nor the global system prop, back-compat users are guaranteed
to see no changes.

{quote}
In my opinion, this is not really a more serious bw-change than a small 
behaviour change, that can be written into CHANGES.txt. We have more serious 
ones.

I would strongly tend to remove the cache at all and write a warning into 
CHANGES.txt.

At all, I do not really think anybody has implemented an own subclass of FSDir. 
The current patch's bw-change is more, that the protected no-arg ctors no 
longer exist and are no longer used.
{quote}

Why take that chance unnecessarily?  What are we gaining by changing
getDirectory so much in place, vs switching to a new (open) API?  It's
entirely possible apps have subclassed FSDir, rely on the singleton
cache and rely on the global system prop.  Making a new API, and
deprecating in favor of that, won't affect back-compat users at all.


> Absorb NIOFSDirectory into FSDirectory
> --------------------------------------
>
>                 Key: LUCENE-1658
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1658
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Store
>            Reporter: Michael McCandless
>            Assignee: Uwe Schindler
>            Priority: Minor
>             Fix For: 2.9
>
>         Attachments: LUCENE-1658-take2.patch, LUCENE-1658-take2.patch, 
> LUCENE-1658.patch, LUCENE-1658.patch, LUCENE-1658.patch
>
>
> I think whether one uses java.io.* vs java.nio.* or eventually
> java.nio2.*, or some other means, is an under-the-hood implementation
> detail of FSDirectory and doesn't merit a whole separate class.
> I think FSDirectory should be the core class one uses when one's index
> is in the filesystem.
> So, I'd like to deprecate NIOFSDirectory, absorbing it into
> FSDirectory, and add a setting "useNIO" to FSDirectory.  It should
> default to "true" for non-Windows OSs, because it gives far better
> concurrent performance on all platforms but Windows (due to known Sun
> JRE issue http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6265734).

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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

Reply via email to