Skip to content
Draft
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package org.apache.solr.index;

import java.io.IOException;
import java.util.Map;
import org.apache.lucene.index.MergePolicy;
import org.apache.lucene.index.MergeTrigger;
import org.apache.lucene.index.SegmentCommitInfo;
import org.apache.lucene.index.SegmentInfos;
import org.apache.lucene.index.TieredMergePolicy;
import org.apache.lucene.util.Version;

/**
* Only allows latest version segments to be considered for merges. That way a snapshot of older
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest this wording instead:

Prevents merging segments with differing Lucene major version number origins. This assists in upgrading to a future Lucene major version.

* segments can remain consistent
*/
public class LatestVersionFilterMergePolicy extends MergePolicy {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you seen FilterMergePolicy for delegation?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great suggestion. Much better for delegation.

MergePolicy delegatePolicy = new TieredMergePolicy();

@Override
public MergeSpecification findMerges(
MergeTrigger mergeTrigger, SegmentInfos infos, MergeContext mergeContext) throws IOException {
/*we don't want to remove from the original SegmentInfos, else the segments may not carry forward upon a commit.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you could merely say that input SegmentInfos should be treated as immutable, and that modifying it is dangerous (ideally would be impossible). An FYI. Your text had me thinking for a moment that the concern was specific to this MP but it's a general statement across Lucene, I see. When I see other usages in Lucene, I don't see dire warnings.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rephrased to not sound alarming

That would be catastrophic. Hence we clone.*/
SegmentInfos infosClone = infos.clone();
infosClone.clear();
for (SegmentCommitInfo info : infos) {
if (info.info.getMinVersion() != null
&& info.info.getMinVersion().major == Version.LATEST.major) {
infosClone.add(info);
}
}

return delegatePolicy.findMerges(mergeTrigger, infosClone, mergeContext);
}

@Override
public MergeSpecification findForcedMerges(
SegmentInfos segmentInfos,
int maxSegmentCount,
Map<SegmentCommitInfo, Boolean> segmentsToMerge,
MergeContext mergeContext)
throws IOException {
return delegatePolicy.findForcedMerges(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the constraint isn't applied, but shouldn't it?

segmentInfos, maxSegmentCount, segmentsToMerge, mergeContext);
}

@Override
public MergeSpecification findForcedDeletesMerges(
SegmentInfos segmentInfos, MergeContext mergeContext) throws IOException {
return delegatePolicy.findForcedDeletesMerges(segmentInfos, mergeContext);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the constraint isn't applied, but shouldn't it?

}
}