You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2021/02/18 00:55:30 UTC
[GitHub] [lucene-solr] rmuir opened a new pull request #2396: LUCENE-9774: TestDirectIODirectory fails with EINVAL on some filesystems
rmuir opened a new pull request #2396:
URL: https://github.com/apache/lucene-solr/pull/2396
TestDirectIODirectory will currently fail if run on an unsupported filesystem (e.g. tmpfs). Add an "assume" that probes if the filesystem supports Direct I/O.
Also tweak javadocs to indicate correct @throws clauses for the IndexInput and IndexOutput. You'll get an IOException (translated from EINVAL) if the filesystem doesn't support it, not a UOE.
cc: @zacharymorn I think you helped on this Directory, if you have time to review, thank you. I know Uwe is currently busy. Mainly I want the lucene tests passing on my machine again :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on pull request #2396: LUCENE-9774: TestDirectIODirectory fails with EINVAL on some filesystems
Posted by GitBox <gi...@apache.org>.
dweiss commented on pull request #2396:
URL: https://github.com/apache/lucene-solr/pull/2396#issuecomment-781355093
Maybe it's something that should be fixed in tmpfs? This is an ancient answer about a different tmpfs issue but made me laugh :)
http://lkml.iu.edu/hypermail/linux/kernel/0506.2/0123.html
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on pull request #2396: LUCENE-9774: TestDirectIODirectory fails with EINVAL on some filesystems
Posted by GitBox <gi...@apache.org>.
dweiss commented on pull request #2396:
URL: https://github.com/apache/lucene-solr/pull/2396#issuecomment-781139315
I think this check should be built-in in the directory itself somehow - verify both the JDK's NIO support flag, check if the location is writeable and brute-force try to create a direct-access file in the pointed directory (in a static method checkFilesystemSupported(Path) perhaps)? Yes, it may mask some other issues but it'll ensure some consistency? checkFilesystemSupported can throw UnsupportedOperationException with a suppressed original IOException so that the cause is not lost.
If the JDK throws an inconsistent exception - add this hacky test to isFilesystemSupported... It'd be also great to have a small repro test case and submit it to core-libs-dev@openjdk.java.net perhaps? Or NIO-appropriate mailing list (I'm not subscribed to all of them...).
Btw. @zacharymorn you have to be careful about where you look and copy code from as legal issues may kick in (incompatible licenses, copyright ownership)... even if it's not the case this time, you know... ;)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] rmuir merged pull request #2396: LUCENE-9774: TestDirectIODirectory fails with EINVAL on some filesystems
Posted by GitBox <gi...@apache.org>.
rmuir merged pull request #2396:
URL: https://github.com/apache/lucene-solr/pull/2396
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] rmuir commented on pull request #2396: LUCENE-9774: TestDirectIODirectory fails with EINVAL on some filesystems
Posted by GitBox <gi...@apache.org>.
rmuir commented on pull request #2396:
URL: https://github.com/apache/lucene-solr/pull/2396#issuecomment-781357420
@dweiss maybe, but it seems controversial. you can see attempted patch/thread here: https://lwn.net/Articles/216885/
at the same time it is annoying, if you research the issue enough, you will find numerous users complaining that they cannot use their favorite database on tmpfs :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on pull request #2396: LUCENE-9774: TestDirectIODirectory fails with EINVAL on some filesystems
Posted by GitBox <gi...@apache.org>.
dweiss commented on pull request #2396:
URL: https://github.com/apache/lucene-solr/pull/2396#issuecomment-781347722
Ok, go ahead.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] zacharymorn commented on pull request #2396: LUCENE-9774: TestDirectIODirectory fails with EINVAL on some filesystems
Posted by GitBox <gi...@apache.org>.
zacharymorn commented on pull request #2396:
URL: https://github.com/apache/lucene-solr/pull/2396#issuecomment-781053065
Hi @rmuir, thanks for cc-ing me! For the exception that's being thrown when the file system does not support direct IO, per java doc of `ExtendedOpenOption.DIRECT` it should actually throw `UnsupportedOperationException`:
```
Attempting to open a file with this option set will result in an {@code UnsupportedOperationException} if the operating system or file system does not support Direct I/O or a sufficient equivalent.
```
so it's a bit unexpected to me that IOException is being thrown there. Could it be a jdk implementation bug?
For probing alternative in addition to checking `IOException` / `UnsupportedOperationException`, I searched around a bit and found this:
https://github.com/openjdk/jdk11u-dev/blob/58082bd009883c6dcb779ac02333ea225097e182/test/jdk/java/nio/channels/FileChannel/directio/DirectIOTest.java#L82-L94
Maybe we can do something similar if file system such as `tmpfs` has different file system type naming?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] rmuir commented on pull request #2396: LUCENE-9774: TestDirectIODirectory fails with EINVAL on some filesystems
Posted by GitBox <gi...@apache.org>.
rmuir commented on pull request #2396:
URL: https://github.com/apache/lucene-solr/pull/2396#issuecomment-780961236
I'm curious if there's a better way to probe this, I'm not happy that the check here could mask real bugs, but at the same time the test is failing always for me...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] rmuir commented on pull request #2396: LUCENE-9774: TestDirectIODirectory fails with EINVAL on some filesystems
Posted by GitBox <gi...@apache.org>.
rmuir commented on pull request #2396:
URL: https://github.com/apache/lucene-solr/pull/2396#issuecomment-781355180
@mikemccand I think it isn't in the sense that usually the user wants to do their own caching and disable the kernel's caching, so if tmpfs just ignored the flag it would not be respecting the user's wishes
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] rmuir commented on pull request #2396: LUCENE-9774: TestDirectIODirectory fails with EINVAL on some filesystems
Posted by GitBox <gi...@apache.org>.
rmuir commented on pull request #2396:
URL: https://github.com/apache/lucene-solr/pull/2396#issuecomment-781756563
I opened followup for any future (non-tests) improvements related to this: https://issues.apache.org/jira/browse/LUCENE-9788
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] rmuir commented on pull request #2396: LUCENE-9774: TestDirectIODirectory fails with EINVAL on some filesystems
Posted by GitBox <gi...@apache.org>.
rmuir commented on pull request #2396:
URL: https://github.com/apache/lucene-solr/pull/2396#issuecomment-781344792
I looked more and I don't think its a JDK bug FWIW, you have to open a file to even get the `EINVAL` from the kernel. Actually I think it would be a bad idea for the JDK to translate `EINVAL` to something like `UnsupportedOperationException` across the board, forget about lucene, just think about the bugs *that* could hide :)
And by the way, when it fails, your probe file is left on the filesystem, it doesn't even bother to clean it up: https://github.com/torvalds/linux/blob/faf145d6f3f3d6f2c066f65602ba9d0a03106915/fs/open.c#L839
This is all no problem for our tests, but just letting you know there is some complexity to move this out of test code into a runtime check.
You can see this behavior easily, if you have a tmpfs mount somewhere, e.g.: `dd if=/dev/zero bs=512 count=1 of=/tmp/testfile oflag=direct`
I also don't think it helps to try to inspect filesystem type and be "helpful" because there are so many out there (think FUSE and network filesystems and so on) that the logic would never be really correct. The only way to know for sure is to test it.
Can we start with this tests-only PR just so that the test suite passes for me again? We can open followup if we want to do more...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org