[
https://issues.apache.org/jira/browse/LUCENE-5544?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13943057#comment-13943057
]
Simon Willnauer commented on LUCENE-5544:
-----------------------------------------
I think the patch looks awesome.. yet here are a couple of comments:
In rollback we should notify with a try / finally since processEvents could
throw an exception:
{code}
try {
processEvents(false, true);
} finally {
notifyAll();
}
{code}
I am not 100% sure about this but I think we need / should refresh the delete
after processing events since it can publish pending flushes etc?
{code}
deleter.refresh();
lastCommitChangeCount = changeCount;
processEvents(false, true);
deleter.refresh(); // refresh here again after processing events?
deleter.close();
{code}
maybe the exception message should be "BOOM!" :)
thanks for doing this
> exceptions during IW.rollback can leak files and locks
> ------------------------------------------------------
>
> Key: LUCENE-5544
> URL: https://issues.apache.org/jira/browse/LUCENE-5544
> Project: Lucene - Core
> Issue Type: Bug
> Reporter: Robert Muir
> Fix For: 4.8, 5.0, 4.7.1
>
> Attachments: LUCENE-5544.patch
>
>
> Today, rollback() doesn't always succeed: if it does, it closes the writer
> nicely. otherwise, if it hits exception, it leaves you with a half-broken
> writer, still potentially holding file handles and write lock.
> This is especially bad if you use Native locks, because you are kind of
> hosed, the static map prevents you from forcefully unlocking (e.g.
> IndexWriter.unlock) so you have no real course of action to try to recover.
> If rollback() hits exception, it should still deliver the exception, but
> release things (e.g. like IOUtils.close).
--
This message was sent by Atlassian JIRA
(v6.2#6252)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
