subject:"\[jira\] \[Commented\] \(LUCENE\-4848\) Add Directory implementations using NIO2 APIs"

[
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13607476#comment-13607476
]

Dawid Weiss commented on LUCENE-4848:
-

Hi Michael, Uwe pinged me about the problem with async background threads. I'll
take a look at your patch later today, ok?

Add Directory implementations using NIO2 APIs
-

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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs

[
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13607506#comment-13607506
]

Uwe Schindler commented on LUCENE-4848:
---

Hi Michael,
in my previous patch I also added the new Directory implementation into the
list of FSDirectories to be used randomly while testing (when tests.directory)
is not given. This is missing in the latest patch. It should simply be added to
the list:

{noformat}
private static final ListString FS_DIRECTORIES = Arrays.asList(
SimpleFSDirectory,
NIOFSDirectory,
-MMapDirectory
+MMapDirectory,
+AsyncFSDirectory
);
{noformat}

I was not able to validate the thread exclusion lists, but Dawid will do this.
Maybe he has an easier solutions, to me it looks too complicated.

Add Directory implementations using NIO2 APIs
-

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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs

[
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13607507#comment-13607507
]

Uwe Schindler commented on LUCENE-4848:
---

bq. Regarding the 2 new abstract methods in the base class: I'm fine with
making the length be passed in the constructor, which leaves isFDValid(). This
was why I had initially made the length method be protected instead of packaged
private: so that subclasses could be created outside of the o.a.l.store
package. I'd like to just change isFDValid() to be protected. If I do that it
will be visible to subclasses and the package, but not publicly available,
sound OK?

protected is the wrong access flag. Protected should only be used for methods
that should be overridded in subclasses, but never called from outside! I would
make the isFDValid() method public to be consistent and mark it as
@lucene.internal in javadocs.

Add Directory implementations using NIO2 APIs
-

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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs

[
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13607512#comment-13607512
]

Robert Muir commented on LUCENE-4848:
-

I don't agree with Uwe. Please dont make methods like isFDValid() public here!
Protected is ok.

Add Directory implementations using NIO2 APIs
-

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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs


[ 
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13607515#comment-13607515
 ] 

Uwe Schindler commented on LUCENE-4848:
---

Sorry I disagree. Protected is the wrong access flag, protected is only to be 
used for methods that are called from the class itsself and not from anywhere 
else. And it does not hide the method from the javadocs, so it stays public 
but with incorrect acess flag. It must be ideally package-private for tests, or 
@lucene.internal and public (or removed at all). 

 Add Directory implementations using NIO2 APIs
 -

 Key: LUCENE-4848
 URL: https://issues.apache.org/jira/browse/LUCENE-4848
 Project: Lucene - Core
  Issue Type: Task
Reporter: Michael Poindexter
Assignee: Uwe Schindler
Priority: Minor
 Attachments: jdk7directory.zip, LUCENE-4848-MMapDirectory.patch, 
 LUCENE-4848.patch, LUCENE-4848.patch, LUCENE-4848.patch


 I have implemented 3 Directory subclasses using NIO2 API's (available on 
 JDK7).  These may be suitable for inclusion in a Lucene contrib module.
 See the mailing list at http://lucene.markmail.org/thread/lrv7miivzmjm3ml5 
 for more details about this code and the advantages it provides.
 The code is attached as a zip to this issue.  I'll be happy to make any 
 changes requested.  I've included some minimal smoke tests, but any help in 
 how to use the normal Lucene tests to perform more thorough testing would be 
 appreciated.

--
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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs

[
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13607518#comment-13607518
]

Robert Muir commented on LUCENE-4848:
-

{quote}
Sorry I disagree. Protected is the wrong access flag, protected is only to be
used for methods that are called from the class itsself and not from anywhere
else.
{quote}

Apparently the java language specification also disagrees with you :)

Add Directory implementations using NIO2 APIs
-

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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs


[ 
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13607520#comment-13607520
 ] 

Robert Muir commented on LUCENE-4848:
-

Uwe here you can find a description of the access flags:
http://docs.oracle.com/javase/tutorial/java/javaOO/accesscontrol.html


 Add Directory implementations using NIO2 APIs
 -

 Key: LUCENE-4848
 URL: https://issues.apache.org/jira/browse/LUCENE-4848
 Project: Lucene - Core
  Issue Type: Task
Reporter: Michael Poindexter
Assignee: Uwe Schindler
Priority: Minor
 Attachments: jdk7directory.zip, LUCENE-4848-MMapDirectory.patch, 
 LUCENE-4848.patch, LUCENE-4848.patch, LUCENE-4848.patch


 I have implemented 3 Directory subclasses using NIO2 API's (available on 
 JDK7).  These may be suitable for inclusion in a Lucene contrib module.
 See the mailing list at http://lucene.markmail.org/thread/lrv7miivzmjm3ml5 
 for more details about this code and the advantages it provides.
 The code is attached as a zip to this issue.  I'll be happy to make any 
 changes requested.  I've included some minimal smoke tests, but any help in 
 how to use the normal Lucene tests to perform more thorough testing would be 
 appreciated.

--
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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs

[
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13607526#comment-13607526
]

Uwe Schindler commented on LUCENE-4848:
---

Robert I know those flags, my complaint was *not* that its not working or
inconsistent. The problem with using protected is that its used in a logically
inconsistent way. In common Java usage, methods are made protected if they are
implementations to be implemented in subclasses of a general contract that is
only called from the (e.g. final) code in the superclass itsself. Protected
methods should never-ever called from outside the class. Its just working from
the same package, but thats a well-known design issue in Java (there are tons
of blogs about that).

The protected access flag works around the problem we have, thats true, but it
makes the method public (in visibility) and thats my problem. So in any case
it must get a @lucene.internal.

Add Directory implementations using NIO2 APIs
-

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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs

[
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13607529#comment-13607529
]

Robert Muir commented on LUCENE-4848:
-

{quote}
Protected methods should never-ever called from outside the class. Its just
working from the same package, but thats a well-known design issue in Java
(there are tons of blogs about that).
{quote}

I don't agree with that (and could care less about such blogs).

{quote}
So in any case it must get a @lucene.internal.
{quote}

Yes it should. but it must not be public.

Add Directory implementations using NIO2 APIs
-

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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs

[
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13607538#comment-13607538
]

Uwe Schindler commented on LUCENE-4848:
---

I digged around, because I dont want to make this method visible at all. I
found out:
This method is only used from TestCompoundFile and this test only uses
SimpleFSDirectory. In my opinion, its completely bogus. To fix the visibility
issue:
- the isFDValid() method should be package-protected
- it should only exist in SimpleFSDirIndexInput, not in the abstract base class
If its in the base class every implementor has to implement it just for a
stupid test that only works with SimpleFSDir. So this must be fixed. Just move
it to SimpleFSDir or maybe we remove this method completely and fix the test.

Add Directory implementations using NIO2 APIs
-

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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs

[
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13607546#comment-13607546
]

Robert Muir commented on LUCENE-4848:
-

{quote}
The patch still does not pass the test suite because of AsyncFSDir daemon
threads.
{quote}

Maybe this new directory should be spun off into a separate issue? Do we have
any idea how it performs on different operating systems? Should it be in core
lucene? Can we nuke WindowsDirectory? I just want to understand the benefits,
because it seems it does an async IO request but then blocks on the future to
return back. so this seems no different than sync io to me. Besides, the rest
of this patch is plenty for one issue since its the .store API (we need to
proceed with caution).

Add Directory implementations using NIO2 APIs
-

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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs

[
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13607560#comment-13607560
]

Uwe Schindler commented on LUCENE-4848:
---

Robert: I agree. I will split the patch and provide one on this thread only
with the refactoring done in the already-existing directories.

The thing with windows was that the RAF-based file channel had the problem of
some kind of synchronization internally in Sun's implementation. We should
validate with the new and patched NIOFSDirectory if this is still the case with
the *completely different* FileChannel (it is different class, different
FileDescriptor-type,...) implementation returned by FileChannel.open(). If this
*new* implementation fixes the old Sun Micrososystems synchronization issues in
NIO v1, we can mark NIOFSDirectory working correctly in Windows with Lucene
trunk / Java 7 and we are fine. So we need to benchmark this.

I dont really like the AsyncFSDirectory, because it looks like a workaround.
If NIOFSDir is now working on windows, we don't need it.

Add Directory implementations using NIO2 APIs
-

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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs

2013-03-20 Thread Michael Poindexter (JIRA)


[ 
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13607577#comment-13607577
 ] 

Michael Poindexter commented on LUCENE-4848:


.bq in my previous patch I also added the new Directory implementation into the 
list of FSDirectories to be used randomly while testing (when tests.directory 
is not given). This is missing in the latest patch. It should simply be added 
to the list in LuceneTestCase.java:

I actually removed it on purpose for now.  I wanted the changes to 
LuceneTestCase to be as uninvasive as possible, so I do some work to poke at 
the thread pool only if AsyncFSDirectory is selected as test.directory.  Given 
that, I didn't want for it to be randomly selected.

.bq If its in the base class every implementor has to implement it just for a 
stupid test that only works with SimpleFSDir. So this must be fixed. Just move 
it to SimpleFSDir or maybe we remove this method completely and fix the test.

Sounds good to me!

.bq Maybe this new directory should be spun off into a separate issue? Do we 
have any idea how it performs on different operating systems? Should it be in 
core lucene? Can we nuke WindowsDirectory? I just want to understand the 
benefits, because it seems it does an async IO request but then blocks on the 
future to return back. so this seems no different than sync io to me. Besides, 
the rest of this patch is plenty for one issue since its the .store API (we 
need to proceed with caution).

You are right, it is no different than sync IO in the end.  It's really only 
useful on Windows where it will use IO completion ports which means there is no 
need to synchronize on the file position (like java does for FileChannel, or 
Lucene does internally for SimpleFSDirectory).  On at least Linux/BSD/Mac the 
Sun JDK will just do what basically FileChannel does anyway and incur the 
additional overhead of notifying a future, so it's unlikely to be useful there 
unless one is highly concerned about the Thread.interrupt thing.

If you want to spin it off I'm fine with that, it's no problem for me to split 
the patch, let me know if everyone thinks that's a good idea. 

.bq I opted not to override the default thread factory because it's never known 
when this thread factory will be constructed (it may be initialized before 
LuceneTestCase is instantiated) and, more importantly, I think we shouldn't be 
messing with the VM we're running on unless we really don't have a choice.

I agree!  If there is a better way I am all ears.  I couldn't find any other 
way to mark the threads as ignorable (the Sun JRE at least doesn't name them in 
any useful way, and they have no property that could be looked up).  We can't 
even figure out from looking at their stacks where they come from since they 
are just from a generic Executor.

I think as gross as it is it should be fairly safe to set the default thread 
factory in LuceneTestCase.  It should work across different vendor VM's since 
it is documented as part of the public Java API (see 
http://docs.oracle.com/javase/7/docs/api/java/nio/channels/AsynchronousChannelGroup.html).
  You are right that it could have been initialized before LuceneTestCase is 
run, but I think that's not a problem.  The two cases here are:
1.) Someone already did asynch io causing the pool to be created before 
LuceneTestCase.  The tests should pass since the threads exist beforehand.
2.) Nobody has done async io yet in our VM (the likely case).  We'll create the 
threads using a special name that says to ignore them.



 Add Directory implementations using NIO2 APIs
 -

 Key: LUCENE-4848
 URL: https://issues.apache.org/jira/browse/LUCENE-4848
 Project: Lucene - Core
  Issue Type: Task
Reporter: Michael Poindexter
Assignee: Uwe Schindler
Priority: Minor
 Attachments: jdk7directory.zip, LUCENE-4848-MMapDirectory.patch, 
 LUCENE-4848.patch, LUCENE-4848.patch, LUCENE-4848.patch, LUCENE-4848.patch, 
 LUCENE-4848.patch, LUCENE-4848.patch.txt


 I have implemented 3 Directory subclasses using NIO2 API's (available on 
 JDK7).  These may be suitable for inclusion in a Lucene contrib module.
 See the mailing list at http://lucene.markmail.org/thread/lrv7miivzmjm3ml5 
 for more details about this code and the advantages it provides.
 The code is attached as a zip to this issue.  I'll be happy to make any 
 changes requested.  I've included some minimal smoke tests, but any help in 
 how to use the normal Lucene tests to perform more thorough testing would be 
 appreciated.

--
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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs

2013-03-20 Thread Michael Poindexter (JIRA)

[
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13607582#comment-13607582
]

Michael Poindexter commented on LUCENE-4848:

bq. I opted not to override the default thread factory because it's never known
when this thread factory will be constructed (it may be initialized before
LuceneTestCase is instantiated) and, more importantly, I think we shouldn't be
messing with the VM we're running on unless we really don't have a choice.

I looked at the patch you uploaded Dawid, and I think it will only work on
Windows. That IOCP class is only used on that platform. On *nix platforms
there's really nothing in the stack that can distinguish these threads...hence
the ThreadFactory :-)

Add Directory implementations using NIO2 APIs
-

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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs

[
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13607585#comment-13607585
]

Dawid Weiss commented on LUCENE-4848:
-

bq. We can't even figure out from looking at their stacks where they come from
since they are just from a generic Executor.

I think we can safely ignore those that spin inside the internal loop -- see my
patch. I know the sysprop is documented in the contract but if we don't have to
I'd rather not substitute the defaults. This has an additional benefit that
we'll see how other vendors implement these :)

I'm not saying your patch was wrong or anything, I'm just opting for ignoring
instead of manipulating to work strategy.

bq. Someone already did asynch io causing the pool to be created before
LuceneTestCase. The tests should pass since the threads exist beforehand.

This depends on which threadpool is actually used for the default. By default
(from what I can see in JDK code) these threads are not eagerly allocated so
it'd still result in thread leaks because there would be more threads upon
leaving the test case than there were when it was started. Like I said, it's
not the main reason I would like to keep it defensive and filter rather than
substitute the default factory.

Dawid

Add Directory implementations using NIO2 APIs
-

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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs

[
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13607587#comment-13607587
]

Dawid Weiss commented on LUCENE-4848:
-

Can you post a stack trace from linux, for example? I'm on windows right now.
If it's a thread pool those workers need to be idle somewhere, right?

Add Directory implementations using NIO2 APIs
-

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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs

[
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13607589#comment-13607589
]

Uwe Schindler commented on LUCENE-4848:
---

We should move the AsyncFSDirectory issue to a separate issue. Robert and I
only want to get the fixes for the already existing directories into the core.
The other stuff should be a new issue. I hope that makes not too much work. I
already uploaded a new patch, with only fixing the existing directories.

Add Directory implementations using NIO2 APIs
-

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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs

[
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13607593#comment-13607593
]

Dawid Weiss commented on LUCENE-4848:
-

Oh, one more thing -- I debated a bit about whether we could just make the
default executor inside asyncdirectory not the system-default one. Then the
problem of detecting these would pretty much go away because we could use
whatever the hell we wanted.

The downside is that we'd have to mimic what the std library does anyway.

Add Directory implementations using NIO2 APIs
-

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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs

2013-03-19 Thread Robert Muir (JIRA)

[
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13606368#comment-13606368
]

Robert Muir commented on LUCENE-4848:
-

{quote}
For demonstartation puposes, I attached the simple patch for MMapDirectory that
uses the new StandardOpenMode and FileChannel.open() provided by Java 7. I did
not yet really test the deletion of open files on windows, but all tests pass
(as they should).
{quote}

This patch looks great!

Add Directory implementations using NIO2 APIs
-

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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs

[
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13606427#comment-13606427
]

Michael Poindexter commented on LUCENE-4848:

Thanks for the demonstration Uwe! It was very helpful as I misunderstood our
earlier conversation and was attempting to change the internals of FSDirectory
to use Path (instead of File) while keeping the public interface the same
(actually, I was done, but waiting for the tests to run before attaching the
patch, so your timing was perfect :) )

I've attached a patch in the same spirit as your MMapDirectory patch that makes
some minor changes to FSDirectory to allow different FSIndexInput and
FSIndexOutput subclasses that use different methods of accessing the file (i.e.
RandomAccessFile vs. FileChannel). It updates MMapDirectory, SimpleFSDirectory
and NIOFSDirectory to use appropriate subclasses, and adds a new
AsyncFSDirectory class.

Add Directory implementations using NIO2 APIs
-

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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs

[
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13606439#comment-13606439
]

Uwe Schindler commented on LUCENE-4848:
---

bq. Thanks for the demonstration Uwe! It was very helpful as I misunderstood
our earlier conversation and was attempting to change the internals of
FSDirectory to use Path (instead of File) while keeping the public interface
the same (actually, I was done, but waiting for the tests to run before
attaching the patch, so your timing was perfect )

We can move to Path later, but before doing that we should get this in as a
first step. This issue is unrelated.

I just skimmed your patch, this looks quite good. I have to look closer into
it, will report back later. I have seen that you almost completely reused my
patch - thanks! But you used try-with-resources to open,mmap,close the channel
- nice!

To run all Lucene+SOLR tests with a specific directory implementation use e.g.:
ant test -Dtests.directory=MMapDirectory, otheriwse Lucene uses in most cases
RAMDirectory and only rarely other ones. By that you should also be able to
test your new directory (it might be needed that you add a hook for
instantiating it inside LuceneTestCase where -Dtests.directory is parsed).

Add Directory implementations using NIO2 APIs
-

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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs

[
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13606443#comment-13606443
]

Uwe Schindler commented on LUCENE-4848:
---

One small thing: The protected method FSIndexInput#length() does not need the
generic FD, it should be parameterless? The FD is known to the subclass, isnt
it?

Add Directory implementations using NIO2 APIs
-

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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs

[
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13606462#comment-13606462
]

Michael Poindexter commented on LUCENE-4848:

blockquote
One small thing: The protected method FSIndexInput#length() does not need the
generic FD, it should be parameterless? The FD is known to the subclass, isnt
it?
/blockquote

2 reasons not to:
1.) I think there is already a parameterless length() method that behaves
slightly differently. This length(T) is intended to extract the full length
from the file accessor, while length() returns the configured length of the
slice.
2.) It is called from the constructor, so it might be considered bad practice
to access member variables since that can be error prone.

Add Directory implementations using NIO2 APIs
-

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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs

[
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13606474#comment-13606474
]

Michael Poindexter commented on LUCENE-4848:

bq. It would also be interesting if this patch maybe solves the
ClosedChannelException problem on interrupt? The time window in MMap is very
short that the bug can happen (only after opening the channel, while mmap is
doing its work before the channel is closed).

I don't think this will change the behavior much at all. Before the patch the
channel was only open briefly (just long enough to do the map()), and after the
change it is the same.

Add Directory implementations using NIO2 APIs
-

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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs

2013-03-19 Thread Robert Muir (JIRA)


[ 
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13606475#comment-13606475
 ] 

Robert Muir commented on LUCENE-4848:
-

I like this patch, thanks Michael!

 Add Directory implementations using NIO2 APIs
 -

 Key: LUCENE-4848
 URL: https://issues.apache.org/jira/browse/LUCENE-4848
 Project: Lucene - Core
  Issue Type: Task
Reporter: Michael Poindexter
Assignee: Uwe Schindler
Priority: Minor
 Attachments: jdk7directory.zip, LUCENE-4848-MMapDirectory.patch, 
 LUCENE-4848.patch


 I have implemented 3 Directory subclasses using NIO2 API's (available on 
 JDK7).  These may be suitable for inclusion in a Lucene contrib module.
 See the mailing list at http://lucene.markmail.org/thread/lrv7miivzmjm3ml5 
 for more details about this code and the advantages it provides.
 The code is attached as a zip to this issue.  I'll be happy to make any 
 changes requested.  I've included some minimal smoke tests, but any help in 
 how to use the normal Lucene tests to perform more thorough testing would be 
 appreciated.

--
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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs

[
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13606862#comment-13606862
]

Uwe Schindler commented on LUCENE-4848:
---

Hi Michael,

your new Directory does not pass any test:
{noformat}
ant test -Dtests.directory=AsyncFSDirectory
{noformat}

This fails with crazy errors. One of them is that it complains about zombie
threads (the Lucene test framework does not allow that thready not seen before
are running after a test has finished). I think it might be caused by some
thread created when the executor=null? But this is not the only error, there
are more test failures. Just try it out.

Add Directory implementations using NIO2 APIs
-

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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs


[ 
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13606891#comment-13606891
 ] 

Uwe Schindler commented on LUCENE-4848:
---

There is one other problem, caused by the 2 new abstract methods in the base 
class: the check for FS validity is package proetcted and is only used by 
Lucene tests. If somebody from outside Lucene code wants to implement a custom 
FSIndexOutput, it is impossible because the package proetcted abstract methods. 
Previously this was not a problem, as the method had a method body and the 
compiler was happy.

In my opinion, we should also remove the fileLength(FD) method and simply 
require the file size to be passed in ctor.

 Add Directory implementations using NIO2 APIs
 -

 Key: LUCENE-4848
 URL: https://issues.apache.org/jira/browse/LUCENE-4848
 Project: Lucene - Core
  Issue Type: Task
Reporter: Michael Poindexter
Assignee: Uwe Schindler
Priority: Minor
 Attachments: jdk7directory.zip, LUCENE-4848-MMapDirectory.patch, 
 LUCENE-4848.patch, LUCENE-4848.patch


 I have implemented 3 Directory subclasses using NIO2 API's (available on 
 JDK7).  These may be suitable for inclusion in a Lucene contrib module.
 See the mailing list at http://lucene.markmail.org/thread/lrv7miivzmjm3ml5 
 for more details about this code and the advantages it provides.
 The code is attached as a zip to this issue.  I'll be happy to make any 
 changes requested.  I've included some minimal smoke tests, but any help in 
 how to use the normal Lucene tests to perform more thorough testing would be 
 appreciated.

--
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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs

[
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13606920#comment-13606920
]

Michael Poindexter commented on LUCENE-4848:

I'll run the tests tonight with -Dtests.directory for the new directory, I
didn't know how to do it before. I think the threads thing is actually the
JRE...if you pass null for the executor it means to use the default system
thread pool the JRE creates for IO notifications. I'm guessing it is lazily
created and the test framework detects and flags this. I'll simply create and
use an executor explicitly for the test cases so that there is thread pool that
can be cleaned up.

I know the channel based FSIndexOutput is unused. I can remove it if you'd
like, but I wanted to put it in based on our previous conversation simply in
case it made sense at some point to use an IndexOuput that did not lock files
for delete.

Regarding the 2 new abstract methods in the base class: I'm fine with making
the length be passed in the constructor, which leaves isFDValid(). This was
why I had initially made the length method be protected instead of packaged
private: so that subclasses could be created outside of the o.a.l.store
package. I'd like to just change isFDValid() to be protected. If I do that it
will be visible to subclasses and the package, but not publicly available,
sound OK?

Add Directory implementations using NIO2 APIs
-

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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs

[
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13604935#comment-13604935
]

Uwe Schindler commented on LUCENE-4848:
---

bq. These may be suitable for inclusion in a Lucene contrib module.

There are no more cntrib modules and we want all normal modules use the same
JDK version, as we had major problems for the release manager to build and test
the realease when multiple JDK versions are involved. So this can only go into
Lucene 5.0 (which will use Java 7). Parts of if MMap and NIOFS changes may go
into Lucene 4, too - as the changes are minimal and could be solved by
reflection (see above).

Add Directory implementations using NIO2 APIs
-

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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs

[
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13604972#comment-13604972
]

Uwe Schindler commented on LUCENE-4848:
---

Hi Michael, one request to you:

Would it be possible to create a *patch* that does not add new classes prefixed
with JDK7 (which should be Java7 btw), but instead directly modify the
existing classes in Lucene 5 (aka trunk)? Of course the new Async variant would
be a new class, but for includion in Lucene 5, a real patch without code
duplication would be fine. And please try to keep the logic changes as
minimal as possible in the existing classes.

I am working on MMapDirectory at the moment to make a patch that also works
with Lucene 4 using reflection.

Add Directory implementations using NIO2 APIs
-

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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs

2013-03-18 Thread Michael Poindexter (JIRA)

[
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13605005#comment-13605005
]

Michael Poindexter commented on LUCENE-4848:

No problem, I'll produce a patch against trunk that just changes the existing
directory implementations as little as possible.

Two questions:
1.) Since this changes the file writing behavior for NIOFSDirectory and
MMapDirectory (writes can now throw a ClosedChannelException if the thread is
interrupted, where I believe they couldn't before) should the changes be
controllable via a flag? Or should I just not change how writes are done for
these two classes (since it shouldn't be necessary to delete a file while it is
open for write, only if it is open read)

2.) I was using a Path instead of a File internally to represent the directory
location. This is somewhat nice in that it works with the Java 7 pluggable
filesystems implementation (i.e. to zip up an index one could just use the zip
filesystem provider with a directory and then do a Directory.copyTo). I assume
you want to not add a dependency on using a Path since that would change the
return type of FSDirectory.getDirectory()?

Add Directory implementations using NIO2 APIs
-

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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs

[
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13605015#comment-13605015
]

Uwe Schindler commented on LUCENE-4848:
---

About 2: I am fine with using Path. When we are on Java 7, Path is fine to hold
the pointer to the directory. Of course FSDirectory could add another ctor, but
not replace the File-based ones. I think you current code does the right thing.

About writing: I know, you changed the whole FSDirectory base class to use
Channel. Maybe keep the base class mostly as it is (with a generic descriptor
parameter, that can be a RAF or Channel). I would prefer to make writing for
now use the RAF as before, but provide a channel-based impl, too?

About 1: The problem with interruptions is a bigger one - we should avoid that
any Directory implementation in Lucene is reacting to interruptions and produce
failures. We had lots of bug reports (regards a bug in Sun's original
implementation, that auto-closed a channel when interrupted). So in general,
interrupted file io in lucene should be repeated.

I will now setup Java 7 build for trunk and after short confirmation from the
mailing lists, I will move Lucene trunk's build to require Java 7!

Add Directory implementations using NIO2 APIs
-

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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs

2013-03-18 Thread Michael Poindexter (JIRA)

[
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13605027#comment-13605027
]

Michael Poindexter commented on LUCENE-4848:

OK, I will use Path, and produce 2 IndexInputs, one that uses a RAF (used by
default), and another that uses a Channel that share most of their code (I will
just do one super class with hooks for every place that depends on the actual
descriptor parameter and two subclasses).

I'll see if there is existing code in 5.x that is calling
FSDirectory.getDirectory() that depends on that being a File. If so, I will
not change it to return a Path, but rather introduce a new method
getDirectoryPath().

I think it's not a bug that the channel is auto-closed when interrupted, but
rather documented behavior (in InterruptibleChannel). Not trying to nitpick,
but just point out that this behavior is unlikely to change in the future since
it's how it's intended to work.

We could pretty easily retry when we get a ClosedByInterruptException, but do
you want that to happen as part of this patch or as another issue? I think
maybe that should go in as a separate patch since I think to make it work
properly you would have to make FSIndexInput.clone() open a new FileChannel
(essentially duplicating the file descriptor per-thread...that way when the
descriptor is closed due to an interrupt you only have to worry about reopening
that thread's FD, not all threads sharing the same FD).

Add Directory implementations using NIO2 APIs
-

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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs

2013-03-18 Thread Robert Muir (JIRA)


[ 
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13605034#comment-13605034
 ] 

Robert Muir commented on LUCENE-4848:
-

{quote}
We could pretty easily retry when we get a ClosedByInterruptException, but do 
you want that to happen as part of this patch or as another issue?
{quote}

please no!


 Add Directory implementations using NIO2 APIs
 -

 Key: LUCENE-4848
 URL: https://issues.apache.org/jira/browse/LUCENE-4848
 Project: Lucene - Core
  Issue Type: Task
Reporter: Michael Poindexter
Assignee: Uwe Schindler
Priority: Minor
 Attachments: jdk7directory.zip


 I have implemented 3 Directory subclasses using NIO2 API's (available on 
 JDK7).  These may be suitable for inclusion in a Lucene contrib module.
 See the mailing list at http://lucene.markmail.org/thread/lrv7miivzmjm3ml5 
 for more details about this code and the advantages it provides.
 The code is attached as a zip to this issue.  I'll be happy to make any 
 changes requested.  I've included some minimal smoke tests, but any help in 
 how to use the normal Lucene tests to perform more thorough testing would be 
 appreciated.

--
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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs


[ 
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13605037#comment-13605037
 ] 

Uwe Schindler commented on LUCENE-4848:
---

Same here. In my opinion writing should use RAF as this is most compatible.

 Add Directory implementations using NIO2 APIs
 -

 Key: LUCENE-4848
 URL: https://issues.apache.org/jira/browse/LUCENE-4848
 Project: Lucene - Core
  Issue Type: Task
Reporter: Michael Poindexter
Assignee: Uwe Schindler
Priority: Minor
 Attachments: jdk7directory.zip


 I have implemented 3 Directory subclasses using NIO2 API's (available on 
 JDK7).  These may be suitable for inclusion in a Lucene contrib module.
 See the mailing list at http://lucene.markmail.org/thread/lrv7miivzmjm3ml5 
 for more details about this code and the advantages it provides.
 The code is attached as a zip to this issue.  I'll be happy to make any 
 changes requested.  I've included some minimal smoke tests, but any help in 
 how to use the normal Lucene tests to perform more thorough testing would be 
 appreciated.

--
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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs

[
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13605038#comment-13605038
]

Uwe Schindler commented on LUCENE-4848:
---

bq. I think maybe that should go in as a separate patch since I think to make
it work properly you would have to make FSIndexInput.clone() open a new
FileChannel (essentially duplicating the file descriptor per-thread...that way
when the descriptor is closed due to an interrupt you only have to worry about
reopening that thread's FD, not all threads sharing the same FD).

We cannot do this, as Lucene *never* closes clones. This would be the grave for
file handles! Lucene would eat up all file handles in milliseconds :-)

Add Directory implementations using NIO2 APIs
-

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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs

2013-03-18 Thread Michael Poindexter (JIRA)

[
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13605047#comment-13605047
]

Michael Poindexter commented on LUCENE-4848:

That would be bad :) I was under the impression that clones were actually
closed, but the close method just checked if it was a clone and if so didn't
actually do anything. Thanks for pointing this out.

In that case there's really not much that can be done to avoid
ClosedByInterruptExceptions. We have one FD that's shared across threads, the
JDK closed it, and it we were to open a new one there's no place to release the
resource. IMO, this would indicate that perhaps clones should in fact be
closed, but I don't know enough about why they are not to have a good opinion
:-)

Add Directory implementations using NIO2 APIs
-

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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs

2013-03-18 Thread Robert Muir (JIRA)

[
https://issues.apache.org/jira/browse/LUCENE-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13605049#comment-13605049
]

Robert Muir commented on LUCENE-4848:
-

I would be against it myself. It seems users cannot even manage open/close on
IndexReader and IndexWriter today. So its too much that they would have to
close() scorers, docsenum, and so on.

Personally i'm not worried about ClosedByInterruptExceptions: just dont
interrupt threads doing searches. I'm also not willing to pay the cost of
additional file handles if i'm not interrupt()'ing...why should i?

But to me this is all unrelated to this issue, its been discussed over and over
elsewhere and the problem already exists today.

Add Directory implementations using NIO2 APIs
-

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

[jira] [Commented] (LUCENE-4848) Add Directory implementations using NIO2 APIs