I just pushed a couple of intrusive changes, lets first see how the TC output looks like and then try fixing the fsync thing.
-- Itamar Syn-Hershko http://code972.com | @synhershko <https://twitter.com/synhershko> Freelance Developer & Consultant Author of RavenDB in Action <http://manning.com/synhershko/> On Tue, Jan 6, 2015 at 2:18 AM, Laimonas Simutis <[email protected]> wrote: > Itamar, > > I get the same impression that for the folder we should just return. I did > a PR with the fix. > > > Laimonas > > On Mon Jan 05 2015 at 6:23:07 PM Itamar Syn-Hershko <[email protected]> > wrote: > > > fsync is required to make sure data is flushed to disk on IW.Commit -- at > > least to have that guaranteed on the OS level. This one is important, > > although I don't think there's any use of this for folders > > > > -- > > > > Itamar Syn-Hershko > > http://code972.com | @synhershko <https://twitter.com/synhershko> > > Freelance Developer & Consultant > > Author of RavenDB in Action <http://manning.com/synhershko/> > > > > On Mon, Jan 5, 2015 at 5:04 PM, Paul Irwin <[email protected]> wrote: > > > > > I believe the code was commented out because it doesn't seem like it is > > > needed on Windows, although my understanding of that could be wrong. > I've > > > used Lucene.net code with the sync stuff commented out in apps in > > > production for a couple years and no issues. > > > > > > However, that doesn't mean that there can't be a possible issue, > > especially > > > if you factor in Mono on Linux/OS X with different filesystems. Does > > anyone > > > know the actual purpose of the fsync code in Java Lucene? Is recreating > > it > > > even needed with System.IO? > > > > > > > > > Paul Irwin > > > Lead Software Engineer > > > feature[23] > > > > > > Email: [email protected] > > > Cell: 863-698-9294 > > > > > > On Mon, Jan 5, 2015 at 8:56 AM, Laimonas Simutis <[email protected]> > > wrote: > > > > > > > Tests occassionally fail with Unauthorized access exception with > stack > > > > trace pointing here: > > > > > > > > > > > > > > > https://github.com/apache/lucenenet/blob/master/src/ > > Lucene.Net.Core/Util/IOUtils.cs#L444 > > > > > > > > To understand the full issue, you can see how it is being called from > > > here: > > > > > > > > > > > > > > > https://github.com/apache/lucenenet/blob/master/src/ > > Lucene.Net.Core/Store/FSDirectory.cs#L387 > > > > > > > > Note how it first fsyncs the files and then if there were any that > were > > > > fsynced, it fsyncs a directory containing the files. Directory part > is > > > the > > > > one that is causing the problems. > > > > > > > > The issue is that fsync implementation in the IOUtils is using > > FileStream > > > > class to flush both files and directories. Doing so for directories > > > throws > > > > Access Denied exception, always. FileStream class cannot be used to > > > "open" > > > > directories. > > > > > > > > > > > > Trying to think how to fix this. The simplest one is to catch Access > > > Denied > > > > thrown and ignore it. You can see how the existing implementation > does > > > this > > > > for IOException catch branch. if dir is true, the IOException is > > ignored > > > > and method passes. That would be the simplest thing to do to get the > > > tests > > > > passing. Heck, even ignore the whole fsync if it is for directory. > > > > > > > > I do think the complete approach would involve falling back to native > > > > functions (CreateFile for Windows to get directory's handle: > > > > > > > > > > > http://msdn.microsoft.com/en-us/library/windows/desktop/ > > aa363858(v=vs.85).aspx > > > > or equivalent in non Windows) and then call FlushFileBuffers or > > > equivalent > > > > in non-Windows. It is kind of what is present in FileSupport class ( > > > > > > > > > > > https://github.com/apache/lucenenet/blob/master/src/ > > Lucene.Net.Core/Support/FileSupport.cs > > > > ) > > > > but not fully implemented. It seems like IOUtils tried to use > > FileSupport > > > > Sync implementation but it was commented out. Does anybody know > > anything > > > > about that? Why it was commented out, etc? > > > > > > > > Looking for advice here on how to proceed. There is a good number of > > > tests > > > > failing this way so it would be a nice issue to take care off. > Perhaps > > go > > > > with the simple route and mark the full implementation for TODO? > > > > > > > > > > > > Laimonas > > > > > > > > > >
