Skip to content

Commit 12ac2a8

Browse files
authored
Cleanup Unreferenced file on segment merge failure (opensearch-project#9503) (opensearch-project#9808)
(cherry picked from commit 37bdb6b) Signed-off-by: Rishav Sagar <rissag@amazon.com>
1 parent bae4e76 commit 12ac2a8

8 files changed

Lines changed: 440 additions & 4 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
116116
- Decouple replication lag from logic to fail stale replicas ([#9507](https://github.com/opensearch-project/OpenSearch/pull/9507))
117117
- [Remote Store] Add support for Remote Translog Store stats in `_remotestore/stats/` API ([#9263](https://github.com/opensearch-project/OpenSearch/pull/9263))
118118
- Removing the vec file extension from INDEX_STORE_HYBRID_NIO_EXTENSIONS, to ensure the no performance degradation for vector search via Lucene Engine.([#9528](https://github.com/opensearch-project/OpenSearch/pull/9528)))
119+
- Cleanup Unreferenced file on segment merge failure ([#9503](https://github.com/opensearch-project/OpenSearch/pull/9503))
119120

120121
### Deprecated
121122

server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,7 @@ public final class IndexScopedSettings extends AbstractScopedSettings {
171171
IndexSettings.INDEX_TRANSLOG_RETENTION_SIZE_SETTING,
172172
IndexSettings.INDEX_SEARCH_IDLE_AFTER,
173173
IndexSettings.INDEX_SEARCH_THROTTLED,
174+
IndexSettings.INDEX_UNREFERENCED_FILE_CLEANUP,
174175
IndexFieldDataService.INDEX_FIELDDATA_CACHE_KEY,
175176
FieldMapper.IGNORE_MALFORMED_SETTING,
176177
FieldMapper.COERCE_SETTING,

server/src/main/java/org/opensearch/index/IndexSettings.java

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -514,6 +514,18 @@ public final class IndexSettings {
514514
Property.Dynamic
515515
);
516516

517+
/**
518+
* This setting controls if unreferenced files will be cleaned up in case segment merge fails due to disk full.
519+
*
520+
* Defaults to true which means unreferenced files will be cleaned up in case segment merge fails.
521+
*/
522+
public static final Setting<Boolean> INDEX_UNREFERENCED_FILE_CLEANUP = Setting.boolSetting(
523+
"index.unreferenced_file_cleanup.enabled",
524+
true,
525+
Property.IndexScope,
526+
Property.Dynamic
527+
);
528+
517529
/**
518530
* Determines a balance between file-based and operations-based peer recoveries. The number of operations that will be used in an
519531
* operations-based peer recovery is limited to this proportion of the total number of documents in the shard (including deleted
@@ -678,6 +690,7 @@ private void setRetentionLeaseMillis(final TimeValue retentionLease) {
678690
private volatile String defaultPipeline;
679691
private volatile String requiredPipeline;
680692
private volatile boolean searchThrottled;
693+
private volatile boolean shouldCleanupUnreferencedFiles;
681694
private volatile long mappingNestedFieldsLimit;
682695
private volatile long mappingNestedDocsLimit;
683696
private volatile long mappingTotalFieldsLimit;
@@ -794,6 +807,7 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti
794807
extendedCompatibilitySnapshotVersion = Version.CURRENT.minimumIndexCompatibilityVersion();
795808
}
796809
this.searchThrottled = INDEX_SEARCH_THROTTLED.get(settings);
810+
this.shouldCleanupUnreferencedFiles = INDEX_UNREFERENCED_FILE_CLEANUP.get(settings);
797811
this.queryStringLenient = QUERY_STRING_LENIENT_SETTING.get(settings);
798812
this.queryStringAnalyzeWildcard = QUERY_STRING_ANALYZE_WILDCARD.get(nodeSettings);
799813
this.queryStringAllowLeadingWildcard = QUERY_STRING_ALLOW_LEADING_WILDCARD.get(nodeSettings);
@@ -906,6 +920,7 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti
906920
scopedSettings.addSettingsUpdateConsumer(FINAL_PIPELINE, this::setRequiredPipeline);
907921
scopedSettings.addSettingsUpdateConsumer(INDEX_SOFT_DELETES_RETENTION_OPERATIONS_SETTING, this::setSoftDeleteRetentionOperations);
908922
scopedSettings.addSettingsUpdateConsumer(INDEX_SEARCH_THROTTLED, this::setSearchThrottled);
923+
scopedSettings.addSettingsUpdateConsumer(INDEX_UNREFERENCED_FILE_CLEANUP, this::setShouldCleanupUnreferencedFiles);
909924
scopedSettings.addSettingsUpdateConsumer(INDEX_SOFT_DELETES_RETENTION_LEASE_PERIOD_SETTING, this::setRetentionLeaseMillis);
910925
scopedSettings.addSettingsUpdateConsumer(INDEX_MAPPING_NESTED_FIELDS_LIMIT_SETTING, this::setMappingNestedFieldsLimit);
911926
scopedSettings.addSettingsUpdateConsumer(INDEX_MAPPING_NESTED_DOCS_LIMIT_SETTING, this::setMappingNestedDocsLimit);
@@ -1539,6 +1554,18 @@ private void setSearchThrottled(boolean searchThrottled) {
15391554
this.searchThrottled = searchThrottled;
15401555
}
15411556

1557+
/**
1558+
* Returns true if unreferenced files should be cleaned up on merge failure for this index.
1559+
*
1560+
*/
1561+
public boolean shouldCleanupUnreferencedFiles() {
1562+
return shouldCleanupUnreferencedFiles;
1563+
}
1564+
1565+
private void setShouldCleanupUnreferencedFiles(boolean shouldCleanupUnreferencedFiles) {
1566+
this.shouldCleanupUnreferencedFiles = shouldCleanupUnreferencedFiles;
1567+
}
1568+
15421569
public long getMappingNestedFieldsLimit() {
15431570
return mappingNestedFieldsLimit;
15441571
}

server/src/main/java/org/opensearch/index/engine/Engine.java

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,10 @@
3838
import org.apache.lucene.index.IndexCommit;
3939
import org.apache.lucene.index.IndexFileNames;
4040
import org.apache.lucene.index.IndexReader;
41+
import org.apache.lucene.index.IndexWriter;
42+
import org.apache.lucene.index.IndexWriterConfig;
4143
import org.apache.lucene.index.LeafReaderContext;
44+
import org.apache.lucene.index.NoMergePolicy;
4245
import org.apache.lucene.index.SegmentCommitInfo;
4346
import org.apache.lucene.index.SegmentInfos;
4447
import org.apache.lucene.index.SegmentReader;
@@ -135,6 +138,8 @@ public abstract class Engine implements Closeable {
135138
public static final String MAX_UNSAFE_AUTO_ID_TIMESTAMP_COMMIT_ID = "max_unsafe_auto_id_timestamp";
136139
public static final String SEARCH_SOURCE = "search"; // TODO: Make source of search enum?
137140
public static final String CAN_MATCH_SEARCH_SOURCE = "can_match";
141+
public static final String FORCE_MERGE = "force merge";
142+
public static final String MERGE_FAILED = "merge failed";
138143

139144
protected final ShardId shardId;
140145
protected final Logger logger;
@@ -983,6 +988,10 @@ protected void fillSegmentStats(SegmentReader segmentReader, boolean includeSegm
983988
}
984989
}
985990

991+
boolean shouldCleanupUnreferencedFiles() {
992+
return engineConfig.getIndexSettings().shouldCleanupUnreferencedFiles();
993+
}
994+
986995
private Map<String, Long> getSegmentFileSizes(SegmentReader segmentReader) {
987996
Directory directory = null;
988997
SegmentCommitInfo segmentCommitInfo = segmentReader.getSegmentInfo();
@@ -1343,6 +1352,14 @@ public void failEngine(String reason, @Nullable Exception failure) {
13431352
);
13441353
}
13451354
}
1355+
1356+
// If cleanup of unreferenced flag is enabled and force merge or regular merge failed due to IOException,
1357+
// clean all unreferenced files on best effort basis created during failed merge and reset the
1358+
// shard state back to last Lucene Commit.
1359+
if (shouldCleanupUnreferencedFiles() && isMergeFailureDueToIOException(failure, reason)) {
1360+
cleanUpUnreferencedFiles();
1361+
}
1362+
13461363
eventListener.onFailedEngine(reason, failure);
13471364
}
13481365
} catch (Exception inner) {
@@ -1361,6 +1378,32 @@ public void failEngine(String reason, @Nullable Exception failure) {
13611378
}
13621379
}
13631380

1381+
/**
1382+
* Cleanup all unreferenced files generated during failed segment merge. This resets shard state to last Lucene
1383+
* commit.
1384+
*/
1385+
private void cleanUpUnreferencedFiles() {
1386+
try (
1387+
IndexWriter writer = new IndexWriter(
1388+
store.directory(),
1389+
new IndexWriterConfig(Lucene.STANDARD_ANALYZER).setSoftDeletesField(Lucene.SOFT_DELETES_FIELD)
1390+
.setCommitOnClose(false)
1391+
.setMergePolicy(NoMergePolicy.INSTANCE)
1392+
.setOpenMode(IndexWriterConfig.OpenMode.APPEND)
1393+
)
1394+
) {
1395+
// do nothing and close this will kick off IndexFileDeleter which will remove all unreferenced files.
1396+
} catch (Exception ex) {
1397+
logger.error("Error while deleting unreferenced file ", ex);
1398+
}
1399+
}
1400+
1401+
/** Check whether the merge failure happened due to IOException. */
1402+
private boolean isMergeFailureDueToIOException(Exception failure, String reason) {
1403+
return (reason.equals(FORCE_MERGE) || reason.equals(MERGE_FAILED))
1404+
&& ExceptionsHelper.unwrap(failure, IOException.class) instanceof IOException;
1405+
}
1406+
13641407
/** Check whether the engine should be failed */
13651408
protected boolean maybeFailEngine(String source, Exception e) {
13661409
if (Lucene.isCorruptionException(e)) {

server/src/main/java/org/opensearch/index/engine/InternalEngine.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2189,7 +2189,7 @@ public void forceMerge(
21892189
throw ex;
21902190
} catch (Exception e) {
21912191
try {
2192-
maybeFailEngine("force merge", e);
2192+
maybeFailEngine(FORCE_MERGE, e);
21932193
} catch (Exception inner) {
21942194
e.addSuppressed(inner);
21952195
}
@@ -2639,7 +2639,7 @@ protected void doRun() throws Exception {
26392639
* confidence that the call stack does not contain catch statements that would cause the error that might be thrown
26402640
* here from being caught and never reaching the uncaught exception handler.
26412641
*/
2642-
failEngine("merge failed", new MergePolicy.MergeException(exc));
2642+
failEngine(MERGE_FAILED, new MergePolicy.MergeException(exc));
26432643
}
26442644
});
26452645
}

0 commit comments

Comments
 (0)