[ 
https://issues.apache.org/jira/browse/LUCENE-3230?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13053872#comment-13053872
 ] 

Michael McCandless commented on LUCENE-3230:
--------------------------------------------

This seems OK, but my only worry is.... I'm not sure this way of fsync'ing 
really "works"?  Ie, this code opens a r/w RAF, calls sync, closes it.  It's 
not clear that this is guaranteed to sync file handles open in the past against 
the same file.  This is something we separately should look into / fix, but 
with this uncertainty it makes me nervous exposing this as a public API... 
maybe we could expose it with a big warning.

bq. Also, while reviewing the code, I noticed that if IOE occurs, the code 
sleeps for 5 msec. If an InterruptedException occurs then, it immediately 
throws ThreadIE, completely ignoring the fact that it slept due to IOE. 
Shouldn't we at least pass IOE.getMessage() on ThreadIE?

+1

> Make FSDirectory.fsync() public and static
> ------------------------------------------
>
>                 Key: LUCENE-3230
>                 URL: https://issues.apache.org/jira/browse/LUCENE-3230
>             Project: Lucene - Java
>          Issue Type: New Feature
>          Components: core/store
>            Reporter: Shai Erera
>            Assignee: Shai Erera
>            Priority: Minor
>             Fix For: 3.3, 4.0
>
>
> I find FSDirectory.fsync() (today protected and instance method) very useful 
> as a utility to sync() files. I'd like create a FSDirectory.sync() utility 
> which contains the exact same impl of FSDir.fsync(), and have the latter call 
> it. We can have it part of IOUtils too, as it's a completely standalone 
> utility.
> I would get rid of FSDir.fsync() if it wasn't protected (as if encouraging 
> people to override it). I doubt anyone really overrides it (our core 
> Directories don't).
> Also, while reviewing the code, I noticed that if IOE occurs, the code sleeps 
> for 5 msec. If an InterruptedException occurs then, it immediately throws 
> ThreadIE, completely ignoring the fact that it slept due to IOE. Shouldn't we 
> at least pass IOE.getMessage() on ThreadIE?
> The patch is trivial, so I'd like to get some feedback before I post it.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to