Skip to content

Commit

Permalink
Refresh after force-merge (#76345)
Browse files Browse the repository at this point in the history
Today a force-merge will write out the newly-merged segments and then
flush them, but it does not automatically release the segments from
before the merge. This will retain extra data on disk until the next
refresh, which could be a long time away if the user has disabled
periodic refreshes or is not searching the index frequently.

With this commit we change the behaviour always to refresh after a
force-merge.

Closes #74649
Backport of #76221
  • Loading branch information
DaveCTurner committed Aug 11, 2021
1 parent 82cec18 commit cb2451d
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2061,6 +2061,14 @@ public void forceMerge(final boolean flush, int maxNumSegments, boolean onlyExpu
if (tryRenewSyncCommit() == false) {
flush(false, true);
}

// If any merges happened then we need to release the unmerged input segments so they can be deleted. A periodic refresh
// will do this eventually unless the user has disabled refreshes or isn't searching this shard frequently, in which
// case we should do something here to ensure a timely refresh occurs. However there's no real need to defer it nor to
// have any should-we-actually-refresh-here logic: we're already doing an expensive force-merge operation at the user's
// request and therefore don't expect any further writes so we may as well do the final refresh immediately and get it
// out of the way.
refresh("force-merge");
}
if (upgrade) {
logger.info("finished segment upgrade");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,9 @@ public void testSegmentsWithMergeFlag() throws Exception {
// now, optimize and wait for merges, see that we have no merge flag
engine.forceMerge(true, 1, false, false, false, UUIDs.randomBase64UUID());

// ensure that we have released the older segments with a refresh so they can be removed
assertFalse(engine.refreshNeeded());

for (Segment segment : engine.segments(false)) {
assertThat(segment.getMergeId(), nullValue());
}
Expand Down

0 comments on commit cb2451d

Please sign in to comment.