From 0887cbc964ffa8b4f4d746fbb39b686f91d6feb0 Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Fri, 1 Nov 2019 09:41:45 -0400 Subject: [PATCH] Fix testForceMergeWithSoftDeletesRetentionAndRecoverySource (#48766) This test failure manifests the limitation of the recovery source merge policy explained in #41628. If we already merge down to a single segment then subsequent force merges will be noop although they can prune recovery source. We need to adjust this test until we have a fix for the merge policy. Relates #41628 Closes #48735 --- .../index/engine/InternalEngineTests.java | 30 ++++++++++++------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java b/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java index 3c467a902dbf7..62810ce857856 100644 --- a/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java +++ b/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java @@ -1720,18 +1720,26 @@ public void testForceMergeWithSoftDeletesRetentionAndRecoverySource() throws Exc settings.put(IndexSettings.INDEX_SOFT_DELETES_RETENTION_OPERATIONS_SETTING.getKey(), 0); indexSettings.updateIndexMetaData(IndexMetaData.builder(defaultSettings.getIndexMetaData()).settings(settings).build()); engine.onSettingsChanged(); - // If the global checkpoint equals to the local checkpoint, the next force-merge will be a noop - // because all deleted documents are expunged in the previous force-merge already. We need to flush - // a new segment to make merge happen so that we can verify that all _recovery_source are pruned. - if (globalCheckpoint.get() == engine.getPersistedLocalCheckpoint() && liveDocs.isEmpty() == false) { - String deleteId = randomFrom(liveDocs); - engine.delete(new Engine.Delete("test", deleteId, newUid(deleteId), primaryTerm.get())); - liveDocsWithSource.remove(deleteId); - liveDocs.remove(deleteId); - engine.flush(); + // If we already merged down to 1 segment, then the next force-merge will be a noop. We need to add an extra segment to make + // merges happen so we can verify that _recovery_source are pruned. See: https://github.com/elastic/elasticsearch/issues/41628. + final int numSegments; + try (Engine.Searcher searcher = engine.acquireSearcher("test", Engine.SearcherScope.INTERNAL)) { + numSegments = searcher.getDirectoryReader().leaves().size(); + } + if (numSegments == 1) { + boolean useRecoverySource = randomBoolean() || omitSourceAllTheTime; + ParsedDocument doc = testParsedDocument("dummy", null, testDocument(), B_1, null, useRecoverySource); + engine.index(indexForDoc(doc)); + if (useRecoverySource == false) { + liveDocsWithSource.add(doc.id()); + } + engine.syncTranslog(); + globalCheckpoint.set(engine.getPersistedLocalCheckpoint()); + engine.flush(randomBoolean(), true); + } else { + globalCheckpoint.set(engine.getPersistedLocalCheckpoint()); + engine.syncTranslog(); } - globalCheckpoint.set(engine.getPersistedLocalCheckpoint()); - engine.syncTranslog(); engine.forceMerge(true, 1, false, false, false); assertConsistentHistoryBetweenTranslogAndLuceneIndex(engine, mapperService); assertThat(readAllOperationsInLucene(engine, mapperService), hasSize(liveDocsWithSource.size()));