You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by Uwe Schindler <us...@apache.org> on 2015/05/12 20:59:21 UTC

RE: Recent Java 9 commit (e5b66323ae45) breaks fsync on directory

Hallo Alan,

I just wanted to come back to this issue, because there was no further communication recently regarding the behavior of Java 9 with opening a FileChannel on a directory to fsync the directory metadata. Unfortunately, this would break the improved data safety after commits to Apache Lucene indexes. This would affect many applications like Apache Solr and Elasticsearch that rely on fsyncing the metadata on UNIX systems (Linux, Solaris, MacOSX). Recently Elasticsearch also started to use the same approach for its transaction log! Because we (Apache Lucene) use atomic rename functionality to "publish" commits, losing the directory metadata after a power failure loses all data in the commit done before the failure. With Java 7 and Java 8 we already did extensive tests with remote controlled power plugs switching a test machine on and off and validating that index was intact. This is no longer working with Java 9 because of the change.

Our question now: The discussion was, to allow maybe another OpenOption to do such special stuff, that is important for other databases, too (I assume, Apache Derby, HSQLDB or other databases written in Java would like to do similar things). Is there anything we can do to make a proposal for a new API, like starting a JEP, opening a bug report,... I would take the opportunity to get involved into the OpenJDK project to help and bring this forward.

Maybe instead of complex open options, we should simply add a new method to the Files class: Files.force/fsync(Path fileOrDir, boolean metadata) that does the right thing depending on the file / operating system?

The Java 7 / Java 8 approach we use at the moment is a bit of undocumented hack already (guarded in a try/catch), because some systems like Windows does not allow fsync on directories (Windows already ensure that the metadata is written correctly after atomic rename). On the other hand, MacOSX looks like ignoring fsync requests completely - also on files - if you don't use a special fnctl. So adding an API that works around the different operating system specialties would be very good.

Uwe

-----
Uwe Schindler
uschindler@apache.org 
ASF Member, Apache Lucene PMC / Committer
Bremen, Germany
http://lucene.apache.org/

> -----Original Message-----
> From: nio-dev [mailto:nio-dev-bounces@openjdk.java.net] On Behalf Of
> Uwe Schindler
> Sent: Friday, January 09, 2015 7:56 PM
> To: 'Alan Bateman'; nio-dev@openjdk.java.net
> Cc: rory.odonnell@oracle.com; 'Balchandra Vaidya'
> Subject: RE: Recent Java 9 commit (e5b66323ae45) breaks fsync on directory
> 
> Hi Alan,
> 
> Thank you for the quick response!
> 
> The requirement to fsync on the directory from Java came already quite
> often on the web (also before Java 7 release - but, before Java 7 it was
> impossible to do from Java code).
> 
> This is one example from before Java 7:
> http://www.quora.com/Is-there-any-way-to-fsync%28%29-a-directory-
> from-Java
> 
> Stackoverflow has some questions about this, too. A famous one (ranked #1
> at Google is this one):
> http://stackoverflow.com/questions/7694307/using-filechannel-to-fsync-a-
> directory-with-nio-2
> 
> In fact this is exactly what we do in Lucene. The question here "Can I count
> on this working on all Unix platforms, in future versions of Java, and in non-
> Oracle JVMs?" is now answered -> NO.
> 
> Fsyncing on a directory is in most cases not needed for "standard java
> programs", but for those who really want to do this (like Lucene or Hadoop),
> maybe the idea with a separate OpenOption would be an idea! In Lucene
> code we can (which has Java 7 as minimum requirement) look with reflection
> for the new OpenOption and pass it. Unfortunately, people using currently
> released Lucene/Solr/Elasticsearch versions can no longer be sure that their
> index survives power outages, if they run it with Java 9. If we can early step
> in and test the new API, we can already release artifacts which at least "try"
> to use the new OpenOption (if available) and fall back to Java 7/Java 8
> semantics otherwise.
> 
> Personally, I would prefer to just document that opening a file channel for
> sure only works with regular files, but may fail with other types of files (think
> of directories, or /dev/xxx devices). The code like it is now was working fine
> for 2 major Java releases, so why change semantics? If somebody
> accidentially opens a directory for reading, it is perfectly fine if he gets an
> IOException a bit delayed. If one opens a block device and writes a non block
> aligned bunch of data, it will fail, too. You patch does not handle this case, it
> only tests for directories. So I think we should leave it up to the operating
> system what you can do with a "file".
> 
> About Windows: In fact, you can also open a directory with CreateFile() [1],
> but with standard flags this fails with access denied (this is what we see in
> Java 7 and Java 8). You have to pass FILE_FLAG_BACKUP_SEMANTICS as
> flags, then you can do it [2]. But FlushFileBuffers does not work, because [2]
> does not list it as "valid call" for directory file handles (because its not needed
> for Windows, in opposite to POSIX). So FileChannel#force() would still fail. In
> that case. For Linux it is important that opening directories only works with
> READ, but not with WRITE, but this is obvious.
> 
> You may also want to read Mike McCandless blog about testing this with his
> installation using remote power switches on a testing machine to test
> durability. With the current Java 7 code in Lucene he got no failures:
> http://blog.mikemccandless.com/2014/04/testing-lucenes-index-durability-
> after.html
> 
> Uwe
> 
> [1] http://msdn.microsoft.com/en-
> us/library/windows/desktop/aa363858(v=vs.85).aspx
> [2] http://msdn.microsoft.com/en-
> us/library/windows/desktop/aa365258(v=vs.85).aspx
> 
> > > We really would like to keep the possibility to fsync a directory on
> > supported operating systems. We hope that the above commit will not be
> > backported into 8u40 and 7u80 releases! In Java 9 we can discuss about
> > other solutions how to handle this:
> > > - Keep current semantics as of Java 7 and Java 8 and just fail if
> > > you really
> > want to READ/WRITE from this FileChannel? This is how the underlying
> > operatinmg system and libc handles this. You can open a file
> > descriptor on anything, file/directory/device/..., but not all
> > operations work on this descriptor, some of them throw exception/return
> error.
> > > - Add a new API for fsyncing a directory (maybe for any file type).
> > > Like
> > Files.fsync(Path)? On Windows this could just be a no-op for directories?
> > Basically something like our IOUtils.fsync() from the link above.
> > >
> > > What's you opinion and how should we proceed?
> > >
> > This use-case may need a new API, one possibility is a new OpenOption
> > (like
> > NOFOLLOW_LINKS) for opening directories. This would allow opening a
> > FileChannel to a directory and also provides somewhere to specify that
> > many of the operations may fail. Implementation-wise it also means you
> > should be able to open directories on Windows.
> >
> > Sorry the original change broken what you are doing but I'm sure you
> > understand that the unspecified behavior to allow directories be
> > opened on some platforms and have subsequent attempts to do common
> > operations (like read) fail wasn't ideal either. There are no plans to
> > back-port this change.
> >
> > -Alan


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


Re: fsync vs. fcntl(F_FULLFSYNC) on OS X

Posted by Alan Bateman <Al...@oracle.com>.

On 13/05/2015 10:27, Uwe Schindler wrote:
> :
>
> There is one additional issue we found recently on MacOSX, but this is 
> only slightly related to the one here. It looks like on MacOSX, 
> FileChannel#force is mostly a noop regarding syncing data to disk, 
> because the underlying operating system requires a “special” fnctl to 
> force buffers to disk device:
>
> https://developer.apple.com/library/mac/documentation/Darwin/Reference/ManPages/man2/fsync.2.html:
>
>      For applications that require tighter guarantees about the 
> integrity of their data, Mac OS X provides
>
>      the F_FULLFSYNC fcntl.  The F_FULLFSYNC fcntl asks the drive to 
> flush all buffered data to permanent
>
>      storage.  Applications, such as databases, that require a strict 
> ordering of writes should use F_FULLFSYNC
>
>      to ensure that their data is written in the order they expect.  
> Please see fcntl(2) for more
>
>      detail.
>
Thank for brining this up, I don't recall anyone pointing this out 
before. I don't know if Apple's JDK use this fcntl but when the OS X 
port came into OpenJDK then it implemented force as fsync (irrespective 
of the metaData field).

Testing this is going to difficult through.

-Alan

Re: Recent Java 9 commit (e5b66323ae45) breaks fsync on directory

Posted by Brian Burkhalter <br...@oracle.com>.
Hi Uwe,

I am happy to hear that you think this plan reasonable.

On May 18, 2015, at 12:43 PM, Uwe Schindler <uw...@thetaphi.de> wrote:

> the plan looks fine from my perspective! Many thanks!
>  
> I have a few comments on the items:
> 1.       We were using FileChannel.open(Path,…) to open directories, we never used Files.newByteChannel(Path,…): see https://goo.gl/4wDo41;  but I assume they both delegate to the same method and both return a FileChannel instance. So maybe we can just prevent directories from be opened with Files.newByteChannel(). But FileChannel.open() could be documented to also work on Directories. This is just an idea.

Thanks for pointing that out.

> 2.       Thanks!
> 3.       That’s the best of the whole approach. #1 ensures that our current code still works,

I hope to get #1 accomplished fairly quickly so at least you can be “back in business” soon.

> and this one would be the solution once we “detect” Java 9. We can reflect on new OpenOptions or new methods and include that code asap, once a first impl is available. It will take a long time until our code will require Java 9, but we can support those APIs before (using reflection/MethodHandles/StandardOpenOption.valueOf(“StringValueOfNewOption”))

I am sure we will have further discussions once we get to #3. Ideally it would be something which would require no change at all on your end but would be logically consistent with the APIs.

Thanks,

Brian

RE: Recent Java 9 commit (e5b66323ae45) breaks fsync on directory

Posted by Uwe Schindler <uw...@thetaphi.de>.
Hi Brian,

 

the plan looks fine from my perspective! Many thanks!

 

I have a few comments on the items:

1.       We were using FileChannel.open(Path,…) to open directories, we never used Files.newByteChannel(Path,…): see https://goo.gl/4wDo41;  but I assume they both delegate to the same method and both return a FileChannel instance. So maybe we can just prevent directories from be opened with Files.newByteChannel(). But FileChannel.open() could be documented to also work on Directories. This is just an idea.

2.       Thanks!

3.       That’s the best of the whole approach. #1 ensures that our current code still works, and this one would be the solution once we “detect” Java 9. We can reflect on new OpenOptions or new methods and include that code asap, once a first impl is available. It will take a long time until our code will require Java 9, but we can support those APIs before (using reflection/MethodHandles/StandardOpenOption.valueOf(“StringValueOfNewOption”))

 

Uwe

 

-----

Uwe Schindler

H.-H.-Meier-Allee 63, D-28213 Bremen

 <http://www.thetaphi.de/> http://www.thetaphi.de

eMail: uwe@thetaphi.de

 

From: nio-dev [mailto:nio-dev-bounces@openjdk.java.net] On Behalf Of Brian Burkhalter
Sent: Monday, May 18, 2015 9:05 PM
To: nio-dev
Cc: rory.odonnell@oracle.com; dev@lucene.apache.org; Balchandra Vaidya
Subject: Re: Recent Java 9 commit (e5b66323ae45) breaks fsync on directory

 

We have revised our approach to this (pair or trio of) problem(s). The following sequence of actions is proposed.

 

1. Revert the patch which fixed https://bugs.openjdk.java.net/browse/JDK-8066915, (fs) Files.newByteChannel opens directories for cases where subsequent reads may fail, which instigated the present situation. I will file and post the link to a new Issue for this.

 

2. Work on a fix for https://bugs.openjdk.java.net/browse/JDK-8080589, (fc) FileChannel.force should use fcntl(F_FULLFSYNC) instead of fsync on OS X.

 

3. Work on a zero to minimal impact fix for https://bugs.openjdk.java.net/browse/JDK-8080235, (fs) Provide ability to flush all modified buffered data to a permanent storage device.

 

Hopefully the foregoing plan of record shall be to everyone’s satisfaction and will converge to an eventual mutually acceptable, logical solution.

 

Thanks,

 

Brian

 

On May 16, 2015, at 9:48 AM, Robert Muir <rc...@gmail.com> wrote:





Is it really not possible to revert the change that broke all this?

 


Re: Recent Java 9 commit (e5b66323ae45) breaks fsync on directory

Posted by Brian Burkhalter <br...@oracle.com>.
Hi Uwe,

You’re welcome!

Brian

On May 21, 2015, at 4:45 PM, Uwe Schindler <us...@apache.org> wrote:

> Hi Brian,
>  
> the patch looks fine to me! Many thanks.
>  
> Uwe
>  
> -----
> Uwe Schindler
> uschindler@apache.org
> ASF Member, Apache Lucene PMC / Committer
> Bremen, Germany
> http://lucene.apache.org/


RE: Recent Java 9 commit (e5b66323ae45) breaks fsync on directory

Posted by Uwe Schindler <us...@apache.org>.
Hi Brian,

 

the patch looks fine to me! Many thanks.

 

Uwe

 

-----

Uwe Schindler

uschindler@apache.org 

ASF Member, Apache Lucene PMC / Committer

Bremen, Germany

http://lucene.apache.org/

 

From: Brian Burkhalter [mailto:brian.burkhalter@oracle.com] 
Sent: Thursday, May 21, 2015 7:32 PM
To: nio-dev
Cc: dev@lucene.apache.org; rory.odonnell@oracle.com; Balchandra Vaidya; Robert Muir
Subject: Re: Recent Java 9 commit (e5b66323ae45) breaks fsync on directory

 

Status update …

 

On May 18, 2015, at 12:04 PM, Brian Burkhalter <br...@oracle.com> wrote:





2. Work on a fix for  <https://bugs.openjdk.java.net/browse/JDK-8080589> https://bugs.openjdk.java.net/browse/JDK-8080589, (fc) FileChannel.force should use fcntl(F_FULLFSYNC) instead of fsync on OS X.

 

Please note that this patch has been pushed:

 

http://hg.openjdk.java.net/jdk9/dev/jdk/rev/e08b856baa26

 

Brian


Re: Recent Java 9 commit (e5b66323ae45) breaks fsync on directory

Posted by Dawid Weiss <da...@cs.put.poznan.pl>.
On this note it'd be great if you guys could at least revisit the
subject of some kind of open feedback from the bug tracker -- just a
way to subscribe to updates ("watch") on a particular issue would be
awesome.

Dawid

On Thu, May 21, 2015 at 9:55 PM, Brian Burkhalter
<br...@oracle.com> wrote:
>
> On May 21, 2015, at 12:54 PM, Robert Muir <rc...@gmail.com> wrote:
>
> Please note that this patch has been pushed:
>
> http://hg.openjdk.java.net/jdk9/dev/jdk/rev/e08b856baa26
>
>
> I just tested this with lucene's test suite without any issues.
>
>
> Fantastic! Thanks for the testing corroboration: that is very good to know.

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


Re: Recent Java 9 commit (e5b66323ae45) breaks fsync on directory

Posted by Brian Burkhalter <br...@oracle.com>.
On May 21, 2015, at 12:54 PM, Robert Muir <rc...@gmail.com> wrote:

>> Please note that this patch has been pushed:
>> 
>> http://hg.openjdk.java.net/jdk9/dev/jdk/rev/e08b856baa26
>> 
> 
> I just tested this with lucene's test suite without any issues.

Fantastic! Thanks for the testing corroboration: that is very good to know.

Re: Recent Java 9 commit (e5b66323ae45) breaks fsync on directory

Posted by Robert Muir <rc...@gmail.com>.
On Thu, May 21, 2015 at 1:31 PM, Brian Burkhalter
<br...@oracle.com> wrote:
>
> 2. Work on a fix for https://bugs.openjdk.java.net/browse/JDK-8080589, (fc)
> FileChannel.force should use fcntl(F_FULLFSYNC) instead of fsync on OS X.
>
>
> Please note that this patch has been pushed:
>
> http://hg.openjdk.java.net/jdk9/dev/jdk/rev/e08b856baa26
>

I just tested this with lucene's test suite without any issues.

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


Re: Recent Java 9 commit (e5b66323ae45) breaks fsync on directory

Posted by Brian Burkhalter <br...@oracle.com>.
Status update …

On May 18, 2015, at 12:04 PM, Brian Burkhalter <br...@oracle.com> wrote:

> 2. Work on a fix for https://bugs.openjdk.java.net/browse/JDK-8080589, (fc) FileChannel.force should use fcntl(F_FULLFSYNC) instead of fsync on OS X.

Please note that this patch has been pushed:

http://hg.openjdk.java.net/jdk9/dev/jdk/rev/e08b856baa26

Brian

Re: Recent Java 9 commit (e5b66323ae45) breaks fsync on directory

Posted by Brian Burkhalter <br...@oracle.com>.
Status update ,,,

On May 18, 2015, at 12:04 PM, Brian Burkhalter <br...@oracle.com> wrote:

> We have revised our approach to this (pair or trio of) problem(s). The following sequence of actions is proposed.
> 
> 1. Revert the patch which fixed https://bugs.openjdk.java.net/browse/JDK-8066915, (fs) Files.newByteChannel opens directories for cases where subsequent reads may fail, which instigated the present situation. I will file and post the link to a new Issue for this.

Please note that this patch has been pushed:

http://hg.openjdk.java.net/jdk9/dev/jdk/rev/eb7d3e11a8cb

Things in the JDK 9 dev branch are at least back to where they used to be.

> 2. Work on a fix for https://bugs.openjdk.java.net/browse/JDK-8080589, (fc) FileChannel.force should use fcntl(F_FULLFSYNC) instead of fsync on OS X.

Work in progress.

Thanks,

Brian

Re: Recent Java 9 commit (e5b66323ae45) breaks fsync on directory

Posted by Brian Burkhalter <br...@oracle.com>.
On May 18, 2015, at 6:34 PM, Robert Muir <rc...@gmail.com> wrote:

>> Hopefully the foregoing plan of record shall be to everyone’s satisfaction
>> and will converge to an eventual mutually acceptable, logical solution.
>> 
> 
> Thank you very much for your consideration here!

We aim to please when possible!

Re: Recent Java 9 commit (e5b66323ae45) breaks fsync on directory

Posted by Robert Muir <rc...@gmail.com>.
On Mon, May 18, 2015 at 3:04 PM, Brian Burkhalter
<br...@oracle.com> wrote:
> We have revised our approach to this (pair or trio of) problem(s). The
> following sequence of actions is proposed.
>
> 1. Revert the patch which fixed
> https://bugs.openjdk.java.net/browse/JDK-8066915, (fs) Files.newByteChannel
> opens directories for cases where subsequent reads may fail, which
> instigated the present situation. I will file and post the link to a new
> Issue for this.
>
> 2. Work on a fix for https://bugs.openjdk.java.net/browse/JDK-8080589, (fc)
> FileChannel.force should use fcntl(F_FULLFSYNC) instead of fsync on OS X.
>
> 3. Work on a zero to minimal impact fix for
> https://bugs.openjdk.java.net/browse/JDK-8080235, (fs) Provide ability to
> flush all modified buffered data to a permanent storage device.
>
> Hopefully the foregoing plan of record shall be to everyone’s satisfaction
> and will converge to an eventual mutually acceptable, logical solution.
>

Thank you very much for your consideration here!

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


Re: Recent Java 9 commit (e5b66323ae45) breaks fsync on directory

Posted by Brian Burkhalter <br...@oracle.com>.
We have revised our approach to this (pair or trio of) problem(s). The following sequence of actions is proposed.

1. Revert the patch which fixed https://bugs.openjdk.java.net/browse/JDK-8066915, (fs) Files.newByteChannel opens directories for cases where subsequent reads may fail, which instigated the present situation. I will file and post the link to a new Issue for this.

2. Work on a fix for https://bugs.openjdk.java.net/browse/JDK-8080589, (fc) FileChannel.force should use fcntl(F_FULLFSYNC) instead of fsync on OS X.

3. Work on a zero to minimal impact fix for https://bugs.openjdk.java.net/browse/JDK-8080235, (fs) Provide ability to flush all modified buffered data to a permanent storage device.

Hopefully the foregoing plan of record shall be to everyone’s satisfaction and will converge to an eventual mutually acceptable, logical solution.

Thanks,

Brian

On May 16, 2015, at 9:48 AM, Robert Muir <rc...@gmail.com> wrote:

> Is it really not possible to revert the change that broke all this?


Re: Recent Java 9 commit (e5b66323ae45) breaks fsync on directory

Posted by Alan Bateman <Al...@oracle.com>.

On 16/05/2015 18:26, Robert Muir wrote:
> :
>
> Code that is responsible for synchronizing data to disk is really
> important, its the last place you want this kind of complexity :(
>
>
Yes, we know this. Let's try to come up with a solution for this in the 
coming weeks.

-Alan.

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


Re: Recent Java 9 commit (e5b66323ae45) breaks fsync on directory

Posted by Robert Muir <rc...@gmail.com>.
On Sat, May 16, 2015 at 12:58 PM, Alan Bateman <Al...@oracle.com> wrote:
>>
> JDK 9 is not planned to ship until 2016 so if there is a new API then I
> would hope this is time enough for Lucene to add support. For now then I
> think we need to come up with the right API for this. We can see later if we
> want to get the undocumented behavior to also work in JDK 9.
>

Well we can't really easily and cleanly add support for the new API,
until we "require" it. Honestly that is going to happen after JDK 9 is
released!

This gives us two possible solutions (that we'd have to live with for
a few years):
* two pages of sync code, with reflection etc to handle the new java 9
method if its present.
* two pages of sync code, split across different jars with
multi-version jar support or something like that (not sure if it works
here, its not really relevant).

Code that is responsible for synchronizing data to disk is really
important, its the last place you want this kind of complexity :(

And until such a new API is finalized, we can't even add code to look
for it reflectively.

I'm just trying to explain our situation, and as a project, I think
lucene is pretty aggressive about making new versions require new JDK
versions (I see plenty of open source code sticking with 1.5/6/etc as
a minimum requirement still).

Again, I don't think we are the only people impacted. A great example
of how people interpret the apis is here:
http://stackoverflow.com/questions/7694307/using-filechannel-to-fsync-a-directory-with-nio-2

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


Re: Recent Java 9 commit (e5b66323ae45) breaks fsync on directory

Posted by Alan Bateman <Al...@oracle.com>.
On 16/05/2015 17:48, Robert Muir wrote:
> :
>
> Separately, the addition of an explicit api for this is a fantastic
> idea, but its no solution for users on the existing api.
>
>
JDK 9 is not planned to ship until 2016 so if there is a new API then I 
would hope this is time enough for Lucene to add support. For now then I 
think we need to come up with the right API for this. We can see later 
if we want to get the undocumented behavior to also work in JDK 9.

-Alan

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


Re: Recent Java 9 commit (e5b66323ae45) breaks fsync on directory

Posted by Robert Muir <rc...@gmail.com>.
On Sat, May 16, 2015 at 1:30 PM, Francis Galiegue <fg...@gmail.com> wrote:

> But at the core of the matter I disagree with your basic premise. To
> me, and to 100-epsilon% of users for that matter, it is perfectly
> logical that opening a stream to a directory should fail before you
> even attempt to read().

I'm not sure how important it really is: you will get the error
eventually, that is what counts.

On the other hand, if someone is using a java application and they
"commit" or "save" their stuff and that returns successfully, they
don't expect for a machine crash to leave them with data loss. To
ensure the directory entries really made it, we need to issue an
fsync().

This is seriously the worst API to be "playing games with".

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


Re: Recent Java 9 commit (e5b66323ae45) breaks fsync on directory

Posted by Francis Galiegue <fg...@gmail.com>.
On Sat, May 16, 2015 at 7:30 PM, Francis Galiegue <fg...@gmail.com> wrote:
[...]
>
> As to a new API, well, why not StandardOpenOption.DIRECTORY?
> O_DIRECTORY has existed for open(2) since POSIX.1-2008 (says open(2)),
> and it is even specified that if the path is not a directory the
> failure is ENOTDIR. Excellent, we have NotDirectoryException already.
>

In fact, even better: OpenOption is an interface after all, so this is
only a matter of providing a default FileSystemProvider which
acknowledges a custom OpenOption of yours; as a bonus this will work
even with Java 8, before and after the change.

Regards,
-- 
Francis Galiegue, fgaliegue@gmail.com, https://github.com/fge
JSON Schema in Java: http://json-schema-validator.herokuapp.com
Parsers in pure Java: https://github.com/fge/grappa

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


Re: Recent Java 9 commit (e5b66323ae45) breaks fsync on directory

Posted by Francis Galiegue <fg...@gmail.com>.
Hello,

On Sat, May 16, 2015 at 6:48 PM, Robert Muir <rc...@gmail.com> wrote:
[...]
>
> I don't understand the tradeoff being made here. The JDK will deliver
> ... a little bit earlier exception to morons trying to read bytes from
> a directory, at the cost of damaging impact to user's data for
> everyone else? What kind of tradeoff is that?
>

That's a little harsh (I'm the "moron" here).

The thing is that you don't always get the choice of your inputs and
if I happen to be given a string which turns out to be a path to a
directory, it is expected that opening an `OutputStream` to it will
fail. In fact, I'd even go as far as saying that introducing an
IsDirectoryException would be pretty much ideal here.

As to a new API, well, why not StandardOpenOption.DIRECTORY?
O_DIRECTORY has existed for open(2) since POSIX.1-2008 (says open(2)),
and it is even specified that if the path is not a directory the
failure is ENOTDIR. Excellent, we have NotDirectoryException already.

But at the core of the matter I disagree with your basic premise. To
me, and to 100-epsilon% of users for that matter, it is perfectly
logical that opening a stream to a directory should fail before you
even attempt to read().

Regards,
-- 
Francis Galiegue, fgaliegue@gmail.com, https://github.com/fge
JSON Schema in Java: http://json-schema-validator.herokuapp.com
Parsers in pure Java: https://github.com/fge/grappa

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


Re: Recent Java 9 commit (e5b66323ae45) breaks fsync on directory

Posted by Robert Muir <rc...@gmail.com>.
Is it really not possible to revert the change that broke all this?

I urge you to reconsider the impacts of this. At the very least, we
can't change already-released versions of lucene code: we can't add
retroactively add "two screens of fsync code" to them to do the right
thing. That code is already out there. Users of those releases who
upgrade to java 9 will immediately be a little less safer against
power outages/crashes.

We aren't the only ones using the previous approach, a quick google
search shows other people know about this previous approach.
http://www.quora.com/Is-there-any-way-to-fsync%28%29-a-directory-from-Java

Try not to look at it as "those damn lucene devs were using an
undocumented api". Try to see it from our point of view, where the
linux fsync manual page is telling us we really need to do this, to
ensure directory entries are visible. And nothing in the java API told
you this wasn't ok: plenty of APIs take "file" where it can be a file
or directory. To me the problem is underspecification in the javadocs.

I don't understand the tradeoff being made here. The JDK will deliver
... a little bit earlier exception to morons trying to read bytes from
a directory, at the cost of damaging impact to user's data for
everyone else? What kind of tradeoff is that?

Look at http://www.slideshare.net/nan1nan1/eat-my-data. See page 117,
where its two screens of code to synchronize data to disk. This seems
to be where java is heading too... does it really have to be like
this?

Separately, the addition of an explicit api for this is a fantastic
idea, but its no solution for users on the existing api.

On Fri, May 15, 2015 at 7:00 PM, Brian Burkhalter
<br...@oracle.com> wrote:
>
> On May 13, 2015, at 7:51 AM, Brian Burkhalter <br...@oracle.com>
> wrote:
>
> I agree with Alan that adding an OpenOption would be a good possibility. In
> any case, as Files only contains static methods, we could still add a
> “utility” method that forces file/directory buffers to disk, that just uses
> the new open option under the hood. By that FileSystem SPI interfaces do not
> need to be modified and just need to take care about the new OpenOption (if
> supported).
>
>
> I started to investigate both avenues. Alan says he has some notes on
> previous work on the OpenOption avenue and I would like to see them before
> proceeding much further.
>
>
> Getting back to this topic after a few days, I don’t know what Alan had in
> mind specifically with respect to OpenOption, so I am more than likely
> missing some subtlety. The problem I have with this approach in regards to
>
> http://download.java.net/jdk9/docs/api/java/nio/file/Files.html#newByteChannel-java.nio.file.Path-java.util.Set-java.nio.file.attribute.FileAttribute...-
>
> is the nature of the return value. In general for the case of syncing
> directories will this return value not be a mostly useless stub?
>
> Before https://bugs.openjdk.java.net/browse/JDK-8066915 was fixed, in the
> case of a directory, reading from or writing to the SeekableByteChannel
> returned by newByteChannel would result in either an IOException for read()
> or a NonWritableChannelException for write(). (One wonders “why not a
> NonReadableChannelException?” for the read() case unless there used to be
> one and this was changed (I am testing with reverted code here)). Apparently
> ((FileChannel)sbc).force() would work however and cause the fsync().
>
> It seems like adding a new OpenOption specific to syncing via
> newByteChannel() is going to lead to a similar situation. The returned
> channel could be only partially functional with the non-functional methods
> either throwing appropriate exceptions (like before JDK-8066915), or simply
> being no-ops and not throwing any exceptions, the respective methods just
> doing the “correct” nothings. Another possibility is that the returned
> SeekableByteChannel is already closed, the sync having been done internally
> to newByteChannel(). None of these approaches is appealing to me personally
> as the returned value is always some form of compromised object.
>
> The other idea that was thrown about was something like Files.forceSync(Path
> path, boolean metadata). While this might force an update to the
> FileSystemProvider it seems much cleaner than returning some non- or
> partially functional object.
>
> From what I read the sync issue provoked by JDK-8066915 is a real problem
> for directories only, not files, is that correct?
>
> Any comments on the foregoing?
>
> Thanks,
>
> Brian

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


Re: Recent Java 9 commit (e5b66323ae45) breaks fsync on directory

Posted by Brian Burkhalter <br...@oracle.com>.
On May 13, 2015, at 7:51 AM, Brian Burkhalter <br...@oracle.com> wrote:

>> I agree with Alan that adding an OpenOption would be a good possibility. In any case, as Files only contains static methods, we could still add a “utility” method that forces file/directory buffers to disk, that just uses the new open option under the hood. By that FileSystem SPI interfaces do not need to be modified and just need to take care about the new OpenOption (if supported).
> 
> I started to investigate both avenues. Alan says he has some notes on previous work on the OpenOption avenue and I would like to see them before proceeding much further.

Getting back to this topic after a few days, I don’t know what Alan had in mind specifically with respect to OpenOption, so I am more than likely missing some subtlety. The problem I have with this approach in regards to

http://download.java.net/jdk9/docs/api/java/nio/file/Files.html#newByteChannel-java.nio.file.Path-java.util.Set-java.nio.file.attribute.FileAttribute...-

is the nature of the return value. In general for the case of syncing directories will this return value not be a mostly useless stub? 

Before https://bugs.openjdk.java.net/browse/JDK-8066915 was fixed, in the case of a directory, reading from or writing to the SeekableByteChannel returned by newByteChannel would result in either an IOException for read() or a NonWritableChannelException for write(). (One wonders “why not a NonReadableChannelException?” for the read() case unless there used to be one and this was changed (I am testing with reverted code here)). Apparently ((FileChannel)sbc).force() would work however and cause the fsync().

It seems like adding a new OpenOption specific to syncing via newByteChannel() is going to lead to a similar situation. The returned channel could be only partially functional with the non-functional methods either throwing appropriate exceptions (like before JDK-8066915), or simply being no-ops and not throwing any exceptions, the respective methods just doing the “correct” nothings. Another possibility is that the returned SeekableByteChannel is already closed, the sync having been done internally to newByteChannel(). None of these approaches is appealing to me personally as the returned value is always some form of compromised object.

The other idea that was thrown about was something like Files.forceSync(Path path, boolean metadata). While this might force an update to the FileSystemProvider it seems much cleaner than returning some non- or partially functional object.

From what I read the sync issue provoked by JDK-8066915 is a real problem for directories only, not files, is that correct?

Any comments on the foregoing?

Thanks,

Brian

Re: Recent Java 9 commit (e5b66323ae45) breaks fsync on directory

Posted by Brian Burkhalter <br...@oracle.com>.
Hi Uwe,

On May 13, 2015, at 2:27 AM, Uwe Schindler <us...@apache.org> wrote:

> many thanks for opening this issue!

You’re welcome!

> I agree with Alan that adding an OpenOption would be a good possibility. In any case, as Files only contains static methods, we could still add a “utility” method that forces file/directory buffers to disk, that just uses the new open option under the hood. By that FileSystem SPI interfaces do not need to be modified and just need to take care about the new OpenOption (if supported).

I started to investigate both avenues. Alan says he has some notes on previous work on the OpenOption avenue and I would like to see them before proceeding much further.

> There is one additional issue we found recently on MacOSX, but this is only slightly related to the one here. It looks like on MacOSX, FileChannel#force is mostly a noop regarding syncing data to disk, because the underlying operating system requires a “special” fnctl to force buffers to disk device:
>  
> https://developer.apple.com/library/mac/documentation/Darwin/Reference/ManPages/man2/fsync.2.html:
>      For applications that require tighter guarantees about the integrity of their data, Mac OS X provides
>      the F_FULLFSYNC fcntl.  The F_FULLFSYNC fcntl asks the drive to flush all buffered data to permanent
>      storage.  Applications, such as databases, that require a strict ordering of writes should use F_FULLFSYNC
>      to ensure that their data is written in the order they expect.  Please see fcntl(2) for more
>      detail.
>  
> This different behavior breaks the guarantees of FileChannel#force on MacOSX (as described in Javadocs). So the MacOSX FileSystemProvider implementation should use this special fnctl to force file buffers to disk.


Thanks for mentioning this. I read all about the F_FULLSYNC situation yesterday in the OS X man pages.

> Should I open a bug report on bugs.sun.com?

I don’t think there is any need. Perhaps we can simply handle the OS X variant under this issue unless someone objects.

Thanks,

Brian


RE: Recent Java 9 commit (e5b66323ae45) breaks fsync on directory

Posted by Uwe Schindler <us...@apache.org>.
Hi Brian,

 

many thanks for opening this issue! I agree with Alan that adding an OpenOption would be a good possibility. In any case, as Files only contains static methods, we could still add a “utility” method that forces file/directory buffers to disk, that just uses the new open option under the hood. By that FileSystem SPI interfaces do not need to be modified and just need to take care about the new OpenOption (if supported).

 

There is one additional issue we found recently on MacOSX, but this is only slightly related to the one here. It looks like on MacOSX, FileChannel#force is mostly a noop regarding syncing data to disk, because the underlying operating system requires a “special” fnctl to force buffers to disk device:

 

https://developer.apple.com/library/mac/documentation/Darwin/Reference/ManPages/man2/fsync.2.html:

     For applications that require tighter guarantees about the integrity of their data, Mac OS X provides

     the F_FULLFSYNC fcntl.  The F_FULLFSYNC fcntl asks the drive to flush all buffered data to permanent

     storage.  Applications, such as databases, that require a strict ordering of writes should use F_FULLFSYNC

     to ensure that their data is written in the order they expect.  Please see fcntl(2) for more

     detail.

 

This different behavior breaks the guarantees of FileChannel#force on MacOSX (as described in Javadocs). So the MacOSX FileSystemProvider implementation should use this special fnctl to force file buffers to disk.

 

Should I open a bug report on bugs.sun.com?

 

Uwe

 

-----

Uwe Schindler

uschindler@apache.org 

ASF Member, Apache Lucene PMC / Committer

Bremen, Germany

http://lucene.apache.org/

 

From: nio-dev [mailto:nio-dev-bounces@openjdk.java.net] On Behalf Of Brian Burkhalter
Sent: Wednesday, May 13, 2015 12:26 AM
To: nio-dev
Cc: rory.odonnell@oracle.com; dev@lucene.apache.org; Balchandra Vaidya
Subject: Re: Recent Java 9 commit (e5b66323ae45) breaks fsync on directory

 

I have created an enhancement issue here:

 

https://bugs.openjdk.java.net/browse/JDK-8080235

 

Brian

 

On May 12, 2015, at 3:10 PM, Brian Burkhalter <br...@oracle.com> wrote:





I will create an issue now and post the ID.

 


Re: Recent Java 9 commit (e5b66323ae45) breaks fsync on directory

Posted by Brian Burkhalter <br...@oracle.com>.
I have created an enhancement issue here:

https://bugs.openjdk.java.net/browse/JDK-8080235

Brian

On May 12, 2015, at 3:10 PM, Brian Burkhalter <br...@oracle.com> wrote:

> I will create an issue now and post the ID.


Re: Recent Java 9 commit (e5b66323ae45) breaks fsync on directory

Posted by Brian Burkhalter <br...@oracle.com>.
On May 12, 2015, at 2:11 PM, Alan Bateman <Al...@oracle.com> wrote:

> I've been too busy with other JDK 9 work to spend on time on this issue recently. I do think we should introduce some support for this use-case as it's clearly important.
> 
> The issue with adding a method to Files is that it requires rev'ing the service provider interface so this is why I brought up the possibility of have it work as an OpenOption.
> 
> We should start by creating a bug on. Brian Burkhalter may have created one already.

I will create an issue now and post the ID.

I read the prior discussion thread (http://mail.openjdk.java.net/pipermail/nio-dev/2015-January/002979.html) and am looking into the two approaches already suggested (Files.forceSync() and OpenOption).

Any further comments appreciated.

Thanks,

Brian

Re: Recent Java 9 commit (e5b66323ae45) breaks fsync on directory

Posted by Alan Bateman <Al...@oracle.com>.
On 12/05/2015 19:59, Uwe Schindler wrote:
> Hallo Alan,
>
> I just wanted to come back to this issue, because there was no further communication recently regarding the behavior of Java 9 with opening a FileChannel on a directory to fsync the directory metadata. Unfortunately, this would break the improved data safety after commits to Apache Lucene indexes. This would affect many applications like Apache Solr and Elasticsearch that rely on fsyncing the metadata on UNIX systems (Linux, Solaris, MacOSX). Recently Elasticsearch also started to use the same approach for its transaction log! Because we (Apache Lucene) use atomic rename functionality to "publish" commits, losing the directory metadata after a power failure loses all data in the commit done before the failure. With Java 7 and Java 8 we already did extensive tests with remote controlled power plugs switching a test machine on and off and validating that index was intact. This is no longer working with Java 9 because of the change.
>
> Our question now: The discussion was, to allow maybe another OpenOption to do such special stuff, that is important for other databases, too (I assume, Apache Derby, HSQLDB or other databases written in Java would like to do similar things). Is there anything we can do to make a proposal for a new API, like starting a JEP, opening a bug report,... I would take the opportunity to get involved into the OpenJDK project to help and bring this forward.
>
> Maybe instead of complex open options, we should simply add a new method to the Files class: Files.force/fsync(Path fileOrDir, boolean metadata) that does the right thing depending on the file / operating system?
I've been too busy with other JDK 9 work to spend on time on this issue 
recently. I do think we should introduce some support for this use-case 
as it's clearly important.

The issue with adding a method to Files is that it requires rev'ing the 
service provider interface so this is why I brought up the possibility 
of have it work as an OpenOption.

We should start by creating a bug on. Brian Burkhalter may have created 
one already.

-Alan


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