[
https://issues.apache.org/jira/browse/LUCENE-2402?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12859369#action_12859369
]
Michael McCandless commented on LUCENE-2402:
--------------------------------------------
bq. The failure happens in CommitPoint's ctor in the assert statement which
verifies the SegmentInos does not have external Directory.
Urgh.... indeed we must protect against one thread doing addIndexes
and another thread calling deleteUnusedFiles.
The way things work today (and I agree we should fix this) is
addIndexes* immediately modify the in memory segments to include
foreign (external Dir) segments, then proceed to target these foreign
segments by merging them away.
bq. I've tried to sync on commitLock (which seems good anyway), but the test
kept failing.
This isn't strictly necessary, I think? The two ops (ongoing commit
-- takes time since fync can be so slow -- and deleting unused files)
are orthogonal. They both invoke IDP/IFD, but this is still protected
(sync'd on IW)...
bq. Only when passing rollbackSI to checkpoint does the test pass.
In fact this is the right track, I think... rollbackSI is a clone of
the last committed segments, whereas the "live" segments contains all
uncommitted stuff that's happened since. We really should not be
treating these pending changes as if they were a commit point... so
using rollbackSI makes sense.
But, the problem is, IFD.checkpoint will hold a new commit point when
you pass isCommit=true, which is no good. I think we need to open up
a new package private method in IFD, eg "revisitPolicy" or some such, which
just does:
{code}
if (infoStream != null) {
message("now visit...");
}
deletePendingFiles();
policy.onCommit(commits);
deleteCommits();
{code}
Ie, most of what IFD.checkpoint does when isCommit=true, minus the
incRef (which has already been done, in the past, for this segments)
and the commits.add of a new commit point.
Invoking IDP.onCommit still isn't quite right (no new commit was done)
but I think it's OK for now? (Adding some kind of "visit" method
feels like overkill...).
bq. BTW, the test fails on DirReader.doClose, where it checks if writer != null
and then calls deleteUnusedFiles. So I guess it's a NRT problem only.
Hmm the problem should be wider than just NRT. Any time one thread
calls deleteUnusedFiles while another is doing addIndexes*, this bug
should be hit-able.
bq. In general, that that addIndexesNoOptimize messes w/ SI seems dangerous to
me, because that's undocumented and unprotected
I agree.
I'd love to [eventually] change addIndexes*, so that it does all its
work "privately" and only in the end atomically "checks in" the
(not-foreign) segments it produced. It gets tricky, though, since
"normal" segment merging, and flushing, is still ongoing, and we'd not
want to do redundant merging work.
This also messes up NRT, ie, if you open an NRT reader during an
addIndexes*, you can see some segments already added and some now --
ie NRT violates the advertised atomicity of addIndexes* (the javadocs
note this).
I think we really need to factor IW apart:
# Indexer (add/update/delete), also flushes new segments
# Keeper of the segments file (exposes API to make atomic changes to
segments file, does commits, interacts w/ IDP/IFD)
# Merger (normal merging, optimize, expungeDeletes, addIndexes)
# Reader pool
> Add an explicit method to invoke IndexDeletionPolicy
> ----------------------------------------------------
>
> Key: LUCENE-2402
> URL: https://issues.apache.org/jira/browse/LUCENE-2402
> Project: Lucene - Java
> Issue Type: Improvement
> Components: Index
> Reporter: Shai Erera
> Assignee: Shai Erera
> Fix For: 3.1
>
> Attachments: LUCENE-2402.patch
>
>
> Today, if one uses an IDP which holds onto segments, such as
> SnapshotDeletionPolicy, or any other IDP in the tests, those segments are
> left in the index even if the IDP no longer references them, until
> IW.commit() is called (and actually does something). I'd like to add a
> specific method to IW which will invoke the IDP's logic and get rid of the
> unused segments w/o forcing the user to call IW.commit(). There are a couple
> of reasons for that:
> * Segments take up sometimes valuable HD space, and the application may wish
> to reclaim that space immediately. In some scenarios, the index is updated
> once in several hours (or even days), and waiting until then may not be
> acceptable.
> * I think it's a cleaner solution than waiting for the next commit() to
> happen. One can still wait for it if one wants, but otherwise it will give
> you the ability to immediately get rid of those segments.
> * TestSnapshotDeletionPolicy includes this code, which only strengthens (IMO)
> the need for such method:
> {code}
> // Add one more document to force writer to commit a
> // final segment, so deletion policy has a chance to
> // delete again:
> Document doc = new Document();
> doc.add(new Field("content", "aaa", Field.Store.YES, Field.Index.ANALYZED,
> Field.TermVector.WITH_POSITIONS_OFFSETS));
> writer.addDocument(doc);
> {code}
> If IW had an explicit method, that code would not need to exist there at all
> ...
> Here comes the fun part - naming the baby:
> * invokeDeletionPolicy -- describes exactly what is going to happen. However,
> if the user did not set IDP at all (relying on default, which I think many
> do), users won't understand what is it.
> * deleteUnusedSegments - more user-friendly, assuming users understand what
> 'segments' are.
> BTW, IW already has deleteUnusedFiles() which only tries to delete
> unreferenced files that failed to delete before (such as on Windows, due to
> e.g. open readers). Perhaps instead of inventing a new name, we can change
> IW.deleteUnusedFiles to call IndexFileDeleter.checkpoint (instead of
> deletePendingFiles) which deletes those files + calls IDP.onCommit().
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
