Skip to content

Commit 8aaba9f

Browse files
committed
Revert "Adjust CombinedDeletionPolicy for multiple commits (#27456)"
The commit looks harmless, unfortunately it can break the engine flush scheduler and the translog rolling. Both `uncommittedOperations` and `uncommittedSizeInBytes` are currently calculated based on the minimum required generation for recovery rather than the translog generation of the last index commit. This is not correct if other index commits are reserved for snapshotting even though we are keeping the last index commit only. This reverts commit e95d18e.
1 parent c9e6524 commit 8aaba9f

File tree

2 files changed

+11
-20
lines changed

2 files changed

+11
-20
lines changed

core/src/main/java/org/elasticsearch/index/engine/CombinedDeletionPolicy.java

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -71,18 +71,12 @@ public void onCommit(List<? extends IndexCommit> commits) throws IOException {
7171
}
7272

7373
private void setLastCommittedTranslogGeneration(List<? extends IndexCommit> commits) throws IOException {
74-
// We need to keep translog since the smallest translog generation of un-deleted commits.
75-
// However, there are commits that are not deleted just because they are being snapshotted (rather than being kept by the policy).
76-
// TODO: We need to distinguish those commits and skip them in calculating the minimum required translog generation.
77-
long minRequiredGen = Long.MAX_VALUE;
78-
for (IndexCommit indexCommit : commits) {
79-
if (indexCommit.isDeleted() == false) {
80-
long translogGen = Long.parseLong(indexCommit.getUserData().get(Translog.TRANSLOG_GENERATION_KEY));
81-
minRequiredGen = Math.min(translogGen, minRequiredGen);
82-
}
83-
}
84-
assert minRequiredGen != Long.MAX_VALUE : "All commits are deleted";
85-
translogDeletionPolicy.setMinTranslogGenerationForRecovery(minRequiredGen);
74+
// when opening an existing lucene index, we currently always open the last commit.
75+
// we therefore use the translog gen as the one that will be required for recovery
76+
final IndexCommit indexCommit = commits.get(commits.size() - 1);
77+
assert indexCommit.isDeleted() == false : "last commit is deleted";
78+
long minGen = Long.parseLong(indexCommit.getUserData().get(Translog.TRANSLOG_GENERATION_KEY));
79+
translogDeletionPolicy.setMinTranslogGenerationForRecovery(minGen);
8680
}
8781

8882
public SnapshotDeletionPolicy getIndexDeletionPolicy() {

core/src/test/java/org/elasticsearch/index/engine/CombinedDeletionPolicyTests.java

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -60,23 +60,20 @@ public void testSettingMinTranslogGen() throws IOException {
6060
EngineConfig.OpenMode.OPEN_INDEX_AND_TRANSLOG);
6161
List<IndexCommit> commitList = new ArrayList<>();
6262
long count = randomIntBetween(10, 20);
63-
long minGen = Long.MAX_VALUE;
63+
long lastGen = 0;
6464
for (int i = 0; i < count; i++) {
65-
long lastGen = randomIntBetween(10, 20000);
66-
minGen = Math.min(minGen, lastGen);
65+
lastGen += randomIntBetween(10, 20000);
6766
commitList.add(mockIndexCommitWithTranslogGen(lastGen));
6867
}
6968
combinedDeletionPolicy.onInit(commitList);
70-
verify(translogDeletionPolicy, times(1)).setMinTranslogGenerationForRecovery(minGen);
69+
verify(translogDeletionPolicy, times(1)).setMinTranslogGenerationForRecovery(lastGen);
7170
commitList.clear();
72-
minGen = Long.MAX_VALUE;
7371
for (int i = 0; i < count; i++) {
74-
long lastGen = randomIntBetween(10, 20000);
75-
minGen = Math.min(minGen, lastGen);
72+
lastGen += randomIntBetween(10, 20000);
7673
commitList.add(mockIndexCommitWithTranslogGen(lastGen));
7774
}
7875
combinedDeletionPolicy.onCommit(commitList);
79-
verify(translogDeletionPolicy, times(1)).setMinTranslogGenerationForRecovery(minGen);
76+
verify(translogDeletionPolicy, times(1)).setMinTranslogGenerationForRecovery(lastGen);
8077
}
8178

8279
IndexCommit mockIndexCommitWithTranslogGen(long gen) throws IOException {

0 commit comments

Comments
 (0)