You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by "Shai Erera (JIRA)" <ji...@apache.org> on 2010/07/13 21:03:53 UTC

[jira] Created: (LUCENE-2537) FSDirectory.copy() impl is unsafe

FSDirectory.copy() impl is unsafe
---------------------------------

                 Key: LUCENE-2537
                 URL: https://issues.apache.org/jira/browse/LUCENE-2537
             Project: Lucene - Java
          Issue Type: Bug
          Components: Store
            Reporter: Shai Erera
            Assignee: Shai Erera
             Fix For: 3.1, 4.0


There are a couple of issues with it:

# FileChannel.transferFrom documents that it may not copy the number of bytes requested, however we don't check the return value. So need to fix the code to read in a loop until all bytes were copied..
# When calling addIndexes() w/ very large segments (few hundred MBs in size), I ran into the following exception (Java 1.6 -- Java 1.5's exception was cryptic):
{code}
Exception in thread "main" java.io.IOException: Map failed
    at sun.nio.ch.FileChannelImpl.map(FileChannelImpl.java:770)
    at sun.nio.ch.FileChannelImpl.transferToTrustedChannel(FileChannelImpl.java:450)
    at sun.nio.ch.FileChannelImpl.transferTo(FileChannelImpl.java:523)
    at org.apache.lucene.store.FSDirectory.copy(FSDirectory.java:450)
    at org.apache.lucene.index.IndexWriter.addIndexes(IndexWriter.java:3019)
Caused by: java.lang.OutOfMemoryError: Map failed
    at sun.nio.ch.FileChannelImpl.map0(Native Method)
    at sun.nio.ch.FileChannelImpl.map(FileChannelImpl.java:767)
    ... 7 more
{code}

I changed the impl to something like this:
{code}
long numWritten = 0;
long numToWrite = input.size();
long bufSize = 1 << 26;
while (numWritten < numToWrite) {
  numWritten += output.transferFrom(input, numWritten, bufSize);
}
{code}

And the code successfully adds the indexes. This code uses chunks of 64MB, however that might be too large for some applications, so we definitely need a smaller one. The question is how small so that performance won't be affected, and it'd be great if we can let it be configurable, however since that API is called by other API, such as addIndexes, not sure it's easily controllable.

Also, I read somewhere (can't remember now where) that on Linux the native impl is better and does copy in chunks. So perhaps we should make a Linux specific impl?

-- 
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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[jira] Resolved: (LUCENE-2537) FSDirectory.copy() impl is unsafe

Posted by "Shai Erera (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/LUCENE-2537?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Shai Erera resolved LUCENE-2537.
--------------------------------

    Resolution: Fixed

Committed revision 979161 (3x).
Committed revision 979185.

> FSDirectory.copy() impl is unsafe
> ---------------------------------
>
>                 Key: LUCENE-2537
>                 URL: https://issues.apache.org/jira/browse/LUCENE-2537
>             Project: Lucene - Java
>          Issue Type: Bug
>          Components: Store
>            Reporter: Shai Erera
>            Assignee: Shai Erera
>             Fix For: 3.1, 4.0
>
>         Attachments: FileCopyTest.java, LUCENE-2537.patch, LUCENE-2537.patch
>
>
> There are a couple of issues with it:
> # FileChannel.transferFrom documents that it may not copy the number of bytes requested, however we don't check the return value. So need to fix the code to read in a loop until all bytes were copied..
> # When calling addIndexes() w/ very large segments (few hundred MBs in size), I ran into the following exception (Java 1.6 -- Java 1.5's exception was cryptic):
> {code}
> Exception in thread "main" java.io.IOException: Map failed
>     at sun.nio.ch.FileChannelImpl.map(FileChannelImpl.java:770)
>     at sun.nio.ch.FileChannelImpl.transferToTrustedChannel(FileChannelImpl.java:450)
>     at sun.nio.ch.FileChannelImpl.transferTo(FileChannelImpl.java:523)
>     at org.apache.lucene.store.FSDirectory.copy(FSDirectory.java:450)
>     at org.apache.lucene.index.IndexWriter.addIndexes(IndexWriter.java:3019)
> Caused by: java.lang.OutOfMemoryError: Map failed
>     at sun.nio.ch.FileChannelImpl.map0(Native Method)
>     at sun.nio.ch.FileChannelImpl.map(FileChannelImpl.java:767)
>     ... 7 more
> {code}
> I changed the impl to something like this:
> {code}
> long numWritten = 0;
> long numToWrite = input.size();
> long bufSize = 1 << 26;
> while (numWritten < numToWrite) {
>   numWritten += output.transferFrom(input, numWritten, bufSize);
> }
> {code}
> And the code successfully adds the indexes. This code uses chunks of 64MB, however that might be too large for some applications, so we definitely need a smaller one. The question is how small so that performance won't be affected, and it'd be great if we can let it be configurable, however since that API is called by other API, such as addIndexes, not sure it's easily controllable.
> Also, I read somewhere (can't remember now where) that on Linux the native impl is better and does copy in chunks. So perhaps we should make a Linux specific impl?

-- 
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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[jira] Commented: (LUCENE-2537) FSDirectory.copy() impl is unsafe

Posted by "Shai Erera (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/LUCENE-2537?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12887935#action_12887935 ] 

Shai Erera commented on LUCENE-2537:
------------------------------------

Oh .. found the thread we discussed that on the list, to which I've actually last posted w/ the following text:

{quote}
I've Googled around a bit and came across this: http://markmail.org/message/l67bierbmmedrfw5. Apparently, there's a long standing bug against SUN since May 2006 (http://bugs.sun.com/view_bug.do?bug_id=6431344) that's still open and reports the exact same behavior that I'm seeing.

If I understand correctly, this might be a Windows limitation and is expected to work well on Linux. I'll give it a try. But this makes me think if we should keep the current behavior for Linux-based directories, and fallback to the chunks approach for Windows ones? Since eventually I'll be running on Linux, I don't want to lose performance ...

This isn't the first that we've witnessed the "write once, run everywhere" misconception of Java :). I'm thinking if in general we should have a Windows/Linux FSDirectory impl, or handlers, to prepare for future cases as well. Mike already started this with LUCENE-2500 (DirectIOLinuxDirectory). Instead of writing a Directory, perhaps we could have a handler object or something, or a generic LinuxDirectory that impls some stuff the 'linux' way. In FSDirectory we already have code which detects the OS and JRE used to decide between Simple, NIO and MMAP Directories ...
{code}

> FSDirectory.copy() impl is unsafe
> ---------------------------------
>
>                 Key: LUCENE-2537
>                 URL: https://issues.apache.org/jira/browse/LUCENE-2537
>             Project: Lucene - Java
>          Issue Type: Bug
>          Components: Store
>            Reporter: Shai Erera
>            Assignee: Shai Erera
>             Fix For: 3.1, 4.0
>
>
> There are a couple of issues with it:
> # FileChannel.transferFrom documents that it may not copy the number of bytes requested, however we don't check the return value. So need to fix the code to read in a loop until all bytes were copied..
> # When calling addIndexes() w/ very large segments (few hundred MBs in size), I ran into the following exception (Java 1.6 -- Java 1.5's exception was cryptic):
> {code}
> Exception in thread "main" java.io.IOException: Map failed
>     at sun.nio.ch.FileChannelImpl.map(FileChannelImpl.java:770)
>     at sun.nio.ch.FileChannelImpl.transferToTrustedChannel(FileChannelImpl.java:450)
>     at sun.nio.ch.FileChannelImpl.transferTo(FileChannelImpl.java:523)
>     at org.apache.lucene.store.FSDirectory.copy(FSDirectory.java:450)
>     at org.apache.lucene.index.IndexWriter.addIndexes(IndexWriter.java:3019)
> Caused by: java.lang.OutOfMemoryError: Map failed
>     at sun.nio.ch.FileChannelImpl.map0(Native Method)
>     at sun.nio.ch.FileChannelImpl.map(FileChannelImpl.java:767)
>     ... 7 more
> {code}
> I changed the impl to something like this:
> {code}
> long numWritten = 0;
> long numToWrite = input.size();
> long bufSize = 1 << 26;
> while (numWritten < numToWrite) {
>   numWritten += output.transferFrom(input, numWritten, bufSize);
> }
> {code}
> And the code successfully adds the indexes. This code uses chunks of 64MB, however that might be too large for some applications, so we definitely need a smaller one. The question is how small so that performance won't be affected, and it'd be great if we can let it be configurable, however since that API is called by other API, such as addIndexes, not sure it's easily controllable.
> Also, I read somewhere (can't remember now where) that on Linux the native impl is better and does copy in chunks. So perhaps we should make a Linux specific impl?

-- 
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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[jira] Updated: (LUCENE-2537) FSDirectory.copy() impl is unsafe

Posted by "Shai Erera (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/LUCENE-2537?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Shai Erera updated LUCENE-2537:
-------------------------------

    Attachment: FileCopyTest.java

I wrote a test which compares FileChannel API to intermediate buffer copies. The test runs each method 3 times and reports the best time of each. It can be run w/ different file and chunk sizes.

Here are the results of copying a 1GB file using different chunk sizes (the chunk is used as the intermediate buffer size as well).

Machine spec:
* Linux, 64-bit (IBM) JVM
* 2xQuad (+hyper-threading) - 16 cores overall
* 16GB RAM
* SAS HD

||Chunk Size||FileChannel||Intermediate Buffer||Diff||
|64K|1865|1528|{color:red}-18%{color}|
|128K|1660|1526|{color:red}-9%{color}|
|512K|1514|1493|{color:red}-2%{color}|
|1M|1552|2072|{color:green}+33%{color}|
|2M|1488|1559|{color:green}5%{color}|
|4M|1596|1831|{color:green}13%{color}|
|16M|1563|1964|{color:green}21%{color}|
|64M|1494|2442|{color:green}39%{color}|
|128M|1469|2445|{color:green}40%{color}|

For small buffer sizes, intermediate byte[] copies is preferable. However, FileChannel method performs pretty much consistently, irregardless of the buffer size (except for the first run), while the byte[] approach degrades a lot, as the buffer size increases.

I think, given these results, we can use the FileChannel method w/ a chunk size of 4 (or even 2) MB, to be on the safe side and don't eat up too much RAM?

> FSDirectory.copy() impl is unsafe
> ---------------------------------
>
>                 Key: LUCENE-2537
>                 URL: https://issues.apache.org/jira/browse/LUCENE-2537
>             Project: Lucene - Java
>          Issue Type: Bug
>          Components: Store
>            Reporter: Shai Erera
>            Assignee: Shai Erera
>             Fix For: 3.1, 4.0
>
>         Attachments: FileCopyTest.java
>
>
> There are a couple of issues with it:
> # FileChannel.transferFrom documents that it may not copy the number of bytes requested, however we don't check the return value. So need to fix the code to read in a loop until all bytes were copied..
> # When calling addIndexes() w/ very large segments (few hundred MBs in size), I ran into the following exception (Java 1.6 -- Java 1.5's exception was cryptic):
> {code}
> Exception in thread "main" java.io.IOException: Map failed
>     at sun.nio.ch.FileChannelImpl.map(FileChannelImpl.java:770)
>     at sun.nio.ch.FileChannelImpl.transferToTrustedChannel(FileChannelImpl.java:450)
>     at sun.nio.ch.FileChannelImpl.transferTo(FileChannelImpl.java:523)
>     at org.apache.lucene.store.FSDirectory.copy(FSDirectory.java:450)
>     at org.apache.lucene.index.IndexWriter.addIndexes(IndexWriter.java:3019)
> Caused by: java.lang.OutOfMemoryError: Map failed
>     at sun.nio.ch.FileChannelImpl.map0(Native Method)
>     at sun.nio.ch.FileChannelImpl.map(FileChannelImpl.java:767)
>     ... 7 more
> {code}
> I changed the impl to something like this:
> {code}
> long numWritten = 0;
> long numToWrite = input.size();
> long bufSize = 1 << 26;
> while (numWritten < numToWrite) {
>   numWritten += output.transferFrom(input, numWritten, bufSize);
> }
> {code}
> And the code successfully adds the indexes. This code uses chunks of 64MB, however that might be too large for some applications, so we definitely need a smaller one. The question is how small so that performance won't be affected, and it'd be great if we can let it be configurable, however since that API is called by other API, such as addIndexes, not sure it's easily controllable.
> Also, I read somewhere (can't remember now where) that on Linux the native impl is better and does copy in chunks. So perhaps we should make a Linux specific impl?

-- 
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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[jira] Updated: (LUCENE-2537) FSDirectory.copy() impl is unsafe

Posted by "Shai Erera (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/LUCENE-2537?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Shai Erera updated LUCENE-2537:
-------------------------------

    Attachment: LUCENE-2537.patch

Patch copies the files in chunks of 2MB. All core tests pass. I'll wait a day or two in case someone wants to suggests a different approach, or chunk size limit before I commit.

> FSDirectory.copy() impl is unsafe
> ---------------------------------
>
>                 Key: LUCENE-2537
>                 URL: https://issues.apache.org/jira/browse/LUCENE-2537
>             Project: Lucene - Java
>          Issue Type: Bug
>          Components: Store
>            Reporter: Shai Erera
>            Assignee: Shai Erera
>             Fix For: 3.1, 4.0
>
>         Attachments: FileCopyTest.java, LUCENE-2537.patch
>
>
> There are a couple of issues with it:
> # FileChannel.transferFrom documents that it may not copy the number of bytes requested, however we don't check the return value. So need to fix the code to read in a loop until all bytes were copied..
> # When calling addIndexes() w/ very large segments (few hundred MBs in size), I ran into the following exception (Java 1.6 -- Java 1.5's exception was cryptic):
> {code}
> Exception in thread "main" java.io.IOException: Map failed
>     at sun.nio.ch.FileChannelImpl.map(FileChannelImpl.java:770)
>     at sun.nio.ch.FileChannelImpl.transferToTrustedChannel(FileChannelImpl.java:450)
>     at sun.nio.ch.FileChannelImpl.transferTo(FileChannelImpl.java:523)
>     at org.apache.lucene.store.FSDirectory.copy(FSDirectory.java:450)
>     at org.apache.lucene.index.IndexWriter.addIndexes(IndexWriter.java:3019)
> Caused by: java.lang.OutOfMemoryError: Map failed
>     at sun.nio.ch.FileChannelImpl.map0(Native Method)
>     at sun.nio.ch.FileChannelImpl.map(FileChannelImpl.java:767)
>     ... 7 more
> {code}
> I changed the impl to something like this:
> {code}
> long numWritten = 0;
> long numToWrite = input.size();
> long bufSize = 1 << 26;
> while (numWritten < numToWrite) {
>   numWritten += output.transferFrom(input, numWritten, bufSize);
> }
> {code}
> And the code successfully adds the indexes. This code uses chunks of 64MB, however that might be too large for some applications, so we definitely need a smaller one. The question is how small so that performance won't be affected, and it'd be great if we can let it be configurable, however since that API is called by other API, such as addIndexes, not sure it's easily controllable.
> Also, I read somewhere (can't remember now where) that on Linux the native impl is better and does copy in chunks. So perhaps we should make a Linux specific impl?

-- 
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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[jira] Updated: (LUCENE-2537) FSDirectory.copy() impl is unsafe

Posted by "Shai Erera (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/LUCENE-2537?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Shai Erera updated LUCENE-2537:
-------------------------------

    Attachment: LUCENE-2537.patch

Patch adds the following:
* FSIndexOutput overrides copyBytes to do optimized copies as well (when possible).
* CompoundFileWriter changed to call copyBytes, instead of using an intermediate buffer.
* Some minor typos and code corrections.

*NOTE:* with the changes to CFW, CheckAbort is accessed only after an entire file is written. The estimated amount of work is the length copied. This means that if abort() is called, it might take a tad longer until CFW will detect it.
I don't think it's serious, since (1) abort() is not called often, (2) for small segments this will probably have no effect (OneMerge was accessed roughly every ~2MB copied) and (3) as reported, the optimized copy is faster when using FileChannel, therefore the time that passes between checks may not be that long.
And there's a (4) -- for really large segments, the amount of work done to merge them is far larger than copying them into the CFS. Therefore the chances that abort() will be called during that process is relatively small ...

All tests pass.

> FSDirectory.copy() impl is unsafe
> ---------------------------------
>
>                 Key: LUCENE-2537
>                 URL: https://issues.apache.org/jira/browse/LUCENE-2537
>             Project: Lucene - Java
>          Issue Type: Bug
>          Components: Store
>            Reporter: Shai Erera
>            Assignee: Shai Erera
>             Fix For: 3.1, 4.0
>
>         Attachments: FileCopyTest.java, LUCENE-2537.patch, LUCENE-2537.patch
>
>
> There are a couple of issues with it:
> # FileChannel.transferFrom documents that it may not copy the number of bytes requested, however we don't check the return value. So need to fix the code to read in a loop until all bytes were copied..
> # When calling addIndexes() w/ very large segments (few hundred MBs in size), I ran into the following exception (Java 1.6 -- Java 1.5's exception was cryptic):
> {code}
> Exception in thread "main" java.io.IOException: Map failed
>     at sun.nio.ch.FileChannelImpl.map(FileChannelImpl.java:770)
>     at sun.nio.ch.FileChannelImpl.transferToTrustedChannel(FileChannelImpl.java:450)
>     at sun.nio.ch.FileChannelImpl.transferTo(FileChannelImpl.java:523)
>     at org.apache.lucene.store.FSDirectory.copy(FSDirectory.java:450)
>     at org.apache.lucene.index.IndexWriter.addIndexes(IndexWriter.java:3019)
> Caused by: java.lang.OutOfMemoryError: Map failed
>     at sun.nio.ch.FileChannelImpl.map0(Native Method)
>     at sun.nio.ch.FileChannelImpl.map(FileChannelImpl.java:767)
>     ... 7 more
> {code}
> I changed the impl to something like this:
> {code}
> long numWritten = 0;
> long numToWrite = input.size();
> long bufSize = 1 << 26;
> while (numWritten < numToWrite) {
>   numWritten += output.transferFrom(input, numWritten, bufSize);
> }
> {code}
> And the code successfully adds the indexes. This code uses chunks of 64MB, however that might be too large for some applications, so we definitely need a smaller one. The question is how small so that performance won't be affected, and it'd be great if we can let it be configurable, however since that API is called by other API, such as addIndexes, not sure it's easily controllable.
> Also, I read somewhere (can't remember now where) that on Linux the native impl is better and does copy in chunks. So perhaps we should make a Linux specific impl?

-- 
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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[jira] Commented: (LUCENE-2537) FSDirectory.copy() impl is unsafe

Posted by "Michael McCandless (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/LUCENE-2537?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12891123#action_12891123 ] 

Michael McCandless commented on LUCENE-2537:
--------------------------------------------

Nice results Shai!

bq. I think, given these results, we can use the FileChannel method w/ a chunk size of 4 (or even 2) MB, to be on the safe side and don't eat up too much RAM?

+1

> FSDirectory.copy() impl is unsafe
> ---------------------------------
>
>                 Key: LUCENE-2537
>                 URL: https://issues.apache.org/jira/browse/LUCENE-2537
>             Project: Lucene - Java
>          Issue Type: Bug
>          Components: Store
>            Reporter: Shai Erera
>            Assignee: Shai Erera
>             Fix For: 3.1, 4.0
>
>         Attachments: FileCopyTest.java
>
>
> There are a couple of issues with it:
> # FileChannel.transferFrom documents that it may not copy the number of bytes requested, however we don't check the return value. So need to fix the code to read in a loop until all bytes were copied..
> # When calling addIndexes() w/ very large segments (few hundred MBs in size), I ran into the following exception (Java 1.6 -- Java 1.5's exception was cryptic):
> {code}
> Exception in thread "main" java.io.IOException: Map failed
>     at sun.nio.ch.FileChannelImpl.map(FileChannelImpl.java:770)
>     at sun.nio.ch.FileChannelImpl.transferToTrustedChannel(FileChannelImpl.java:450)
>     at sun.nio.ch.FileChannelImpl.transferTo(FileChannelImpl.java:523)
>     at org.apache.lucene.store.FSDirectory.copy(FSDirectory.java:450)
>     at org.apache.lucene.index.IndexWriter.addIndexes(IndexWriter.java:3019)
> Caused by: java.lang.OutOfMemoryError: Map failed
>     at sun.nio.ch.FileChannelImpl.map0(Native Method)
>     at sun.nio.ch.FileChannelImpl.map(FileChannelImpl.java:767)
>     ... 7 more
> {code}
> I changed the impl to something like this:
> {code}
> long numWritten = 0;
> long numToWrite = input.size();
> long bufSize = 1 << 26;
> while (numWritten < numToWrite) {
>   numWritten += output.transferFrom(input, numWritten, bufSize);
> }
> {code}
> And the code successfully adds the indexes. This code uses chunks of 64MB, however that might be too large for some applications, so we definitely need a smaller one. The question is how small so that performance won't be affected, and it'd be great if we can let it be configurable, however since that API is called by other API, such as addIndexes, not sure it's easily controllable.
> Also, I read somewhere (can't remember now where) that on Linux the native impl is better and does copy in chunks. So perhaps we should make a Linux specific impl?

-- 
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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[jira] Commented: (LUCENE-2537) FSDirectory.copy() impl is unsafe

Posted by "Shai Erera (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/LUCENE-2537?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12892073#action_12892073 ] 

Shai Erera commented on LUCENE-2537:
------------------------------------

If there are no objections, I'll commit this today,

> FSDirectory.copy() impl is unsafe
> ---------------------------------
>
>                 Key: LUCENE-2537
>                 URL: https://issues.apache.org/jira/browse/LUCENE-2537
>             Project: Lucene - Java
>          Issue Type: Bug
>          Components: Store
>            Reporter: Shai Erera
>            Assignee: Shai Erera
>             Fix For: 3.1, 4.0
>
>         Attachments: FileCopyTest.java, LUCENE-2537.patch, LUCENE-2537.patch
>
>
> There are a couple of issues with it:
> # FileChannel.transferFrom documents that it may not copy the number of bytes requested, however we don't check the return value. So need to fix the code to read in a loop until all bytes were copied..
> # When calling addIndexes() w/ very large segments (few hundred MBs in size), I ran into the following exception (Java 1.6 -- Java 1.5's exception was cryptic):
> {code}
> Exception in thread "main" java.io.IOException: Map failed
>     at sun.nio.ch.FileChannelImpl.map(FileChannelImpl.java:770)
>     at sun.nio.ch.FileChannelImpl.transferToTrustedChannel(FileChannelImpl.java:450)
>     at sun.nio.ch.FileChannelImpl.transferTo(FileChannelImpl.java:523)
>     at org.apache.lucene.store.FSDirectory.copy(FSDirectory.java:450)
>     at org.apache.lucene.index.IndexWriter.addIndexes(IndexWriter.java:3019)
> Caused by: java.lang.OutOfMemoryError: Map failed
>     at sun.nio.ch.FileChannelImpl.map0(Native Method)
>     at sun.nio.ch.FileChannelImpl.map(FileChannelImpl.java:767)
>     ... 7 more
> {code}
> I changed the impl to something like this:
> {code}
> long numWritten = 0;
> long numToWrite = input.size();
> long bufSize = 1 << 26;
> while (numWritten < numToWrite) {
>   numWritten += output.transferFrom(input, numWritten, bufSize);
> }
> {code}
> And the code successfully adds the indexes. This code uses chunks of 64MB, however that might be too large for some applications, so we definitely need a smaller one. The question is how small so that performance won't be affected, and it'd be great if we can let it be configurable, however since that API is called by other API, such as addIndexes, not sure it's easily controllable.
> Also, I read somewhere (can't remember now where) that on Linux the native impl is better and does copy in chunks. So perhaps we should make a Linux specific impl?

-- 
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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[jira] Issue Comment Edited: (LUCENE-2537) FSDirectory.copy() impl is unsafe

Posted by "Shai Erera (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/LUCENE-2537?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12887935#action_12887935 ] 

Shai Erera edited comment on LUCENE-2537 at 7/22/10 8:09 AM:
-------------------------------------------------------------

Oh .. found the thread we discussed that on the list, to which I've actually last posted w/ the following text:

{quote}
I've Googled around a bit and came across this: http://markmail.org/message/l67bierbmmedrfw5. Apparently, there's a long standing bug against SUN since May 2006 (http://bugs.sun.com/view_bug.do?bug_id=6431344) that's still open and reports the exact same behavior that I'm seeing.

If I understand correctly, this might be a Windows limitation and is expected to work well on Linux. I'll give it a try. But this makes me think if we should keep the current behavior for Linux-based directories, and fallback to the chunks approach for Windows ones? Since eventually I'll be running on Linux, I don't want to lose performance ...

This isn't the first that we've witnessed the "write once, run everywhere" misconception of Java :). I'm thinking if in general we should have a Windows/Linux FSDirectory impl, or handlers, to prepare for future cases as well. Mike already started this with LUCENE-2500 (DirectIOLinuxDirectory). Instead of writing a Directory, perhaps we could have a handler object or something, or a generic LinuxDirectory that impls some stuff the 'linux' way. In FSDirectory we already have code which detects the OS and JRE used to decide between Simple, NIO and MMAP Directories ...
{quote}

      was (Author: shaie):
    Oh .. found the thread we discussed that on the list, to which I've actually last posted w/ the following text:

{quote}
I've Googled around a bit and came across this: http://markmail.org/message/l67bierbmmedrfw5. Apparently, there's a long standing bug against SUN since May 2006 (http://bugs.sun.com/view_bug.do?bug_id=6431344) that's still open and reports the exact same behavior that I'm seeing.

If I understand correctly, this might be a Windows limitation and is expected to work well on Linux. I'll give it a try. But this makes me think if we should keep the current behavior for Linux-based directories, and fallback to the chunks approach for Windows ones? Since eventually I'll be running on Linux, I don't want to lose performance ...

This isn't the first that we've witnessed the "write once, run everywhere" misconception of Java :). I'm thinking if in general we should have a Windows/Linux FSDirectory impl, or handlers, to prepare for future cases as well. Mike already started this with LUCENE-2500 (DirectIOLinuxDirectory). Instead of writing a Directory, perhaps we could have a handler object or something, or a generic LinuxDirectory that impls some stuff the 'linux' way. In FSDirectory we already have code which detects the OS and JRE used to decide between Simple, NIO and MMAP Directories ...
{code}
  
> FSDirectory.copy() impl is unsafe
> ---------------------------------
>
>                 Key: LUCENE-2537
>                 URL: https://issues.apache.org/jira/browse/LUCENE-2537
>             Project: Lucene - Java
>          Issue Type: Bug
>          Components: Store
>            Reporter: Shai Erera
>            Assignee: Shai Erera
>             Fix For: 3.1, 4.0
>
>
> There are a couple of issues with it:
> # FileChannel.transferFrom documents that it may not copy the number of bytes requested, however we don't check the return value. So need to fix the code to read in a loop until all bytes were copied..
> # When calling addIndexes() w/ very large segments (few hundred MBs in size), I ran into the following exception (Java 1.6 -- Java 1.5's exception was cryptic):
> {code}
> Exception in thread "main" java.io.IOException: Map failed
>     at sun.nio.ch.FileChannelImpl.map(FileChannelImpl.java:770)
>     at sun.nio.ch.FileChannelImpl.transferToTrustedChannel(FileChannelImpl.java:450)
>     at sun.nio.ch.FileChannelImpl.transferTo(FileChannelImpl.java:523)
>     at org.apache.lucene.store.FSDirectory.copy(FSDirectory.java:450)
>     at org.apache.lucene.index.IndexWriter.addIndexes(IndexWriter.java:3019)
> Caused by: java.lang.OutOfMemoryError: Map failed
>     at sun.nio.ch.FileChannelImpl.map0(Native Method)
>     at sun.nio.ch.FileChannelImpl.map(FileChannelImpl.java:767)
>     ... 7 more
> {code}
> I changed the impl to something like this:
> {code}
> long numWritten = 0;
> long numToWrite = input.size();
> long bufSize = 1 << 26;
> while (numWritten < numToWrite) {
>   numWritten += output.transferFrom(input, numWritten, bufSize);
> }
> {code}
> And the code successfully adds the indexes. This code uses chunks of 64MB, however that might be too large for some applications, so we definitely need a smaller one. The question is how small so that performance won't be affected, and it'd be great if we can let it be configurable, however since that API is called by other API, such as addIndexes, not sure it's easily controllable.
> Also, I read somewhere (can't remember now where) that on Linux the native impl is better and does copy in chunks. So perhaps we should make a Linux specific impl?

-- 
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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org