Addressing Comments

opensearch-project · Jun 22, 2023 · 6b94980 · 6b94980
1 parent aa3c392
commit 6b94980
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 13 deletions.
diff --git a/...src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreBaseIntegTestCase.java b/...src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreBaseIntegTestCase.java
@@ -60,7 +60,6 @@ private Settings defaultIndexSettings() {
  .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, REPLICA_COUNT)
  .put(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey(), "300s")
  .put(IndexMetadata.SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT)
- .put(MapperService.INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING.getKey(), Long.MAX_VALUE)
  .build();
  }
 
@@ -72,6 +71,13 @@ protected Settings remoteStoreIndexSettings(int numberOfReplicas) {
  .build();
  }
 
+ protected Settings remoteStoreIndexSettings(int numberOfReplicas, long totalFieldLimit) {
+ return Settings.builder()
+ .put(remoteStoreIndexSettings(numberOfReplicas))
+ .put(MapperService.INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING.getKey(), totalFieldLimit)
+ .build();
+ }
+
  protected Settings remoteTranslogIndexSettings(int numberOfReplicas) {
  return Settings.builder()
  .put(remoteStoreIndexSettings(numberOfReplicas))

diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreIT.java
@@ -281,7 +281,7 @@ public void testRemoteTranslogCleanup() throws Exception {
 
  public void testStaleCommitDeletion() throws Exception {
  internalCluster().startDataOnlyNodes(3);
- createIndex(INDEX_NAME, remoteStoreIndexSettings(1));
+ createIndex(INDEX_NAME, remoteStoreIndexSettings(1, 10000));
  int numberOfIterations = randomIntBetween(5, 15);
  boolean invokeFlush = randomBoolean();
  indexData(numberOfIterations, invokeFlush);
@@ -292,12 +292,17 @@ public void testStaleCommitDeletion() throws Exception {
  .getSetting(INDEX_NAME, IndexMetadata.SETTING_INDEX_UUID);
  Path indexPath = Path.of(String.valueOf(absolutePath), indexUUID, "/0/segments/metadata");
  if (invokeFlush) {
+ // Delete is async.
  assertBusy(() -> {
  try {
- int expectedFileCount = numberOfIterations <= RemoteStoreRefreshListener.LAST_N_METADATA_FILES_TO_KEEP
- ? numberOfIterations
- : 11;
- assertEquals(expectedFileCount, getFileCount(indexPath));
+ int actualFileCount = getFileCount(indexPath);
+ if (numberOfIterations <= RemoteStoreRefreshListener.LAST_N_METADATA_FILES_TO_KEEP) {
+ assertEquals(numberOfIterations, actualFileCount);
+ } else {
+ // As delete is async its possible that the file gets created before the deletion or after
+ // deletion.
+ assertTrue(actualFileCount >= 10 || actualFileCount <= 11);
+ }
  } catch (Exception e) {}
  }, 30, TimeUnit.SECONDS);
  } else {

diff --git a/server/src/main/java/org/opensearch/index/shard/RemoteStoreRefreshListener.java b/server/src/main/java/org/opensearch/index/shard/RemoteStoreRefreshListener.java
@@ -378,14 +378,19 @@ private String getChecksumOfLocalFile(String file) throws IOException {
  }
 
  private void deleteStaleCommitsAsync(String threadpoolName, Runnable onCompletion) {
- indexShard.getThreadPool().executor(threadpoolName).execute(() -> {
- try {
- remoteDirectory.deleteStaleSegments(LAST_N_METADATA_FILES_TO_KEEP);
- } catch (IOException e) {
- logger.info("Exception while deleting stale commits from remote segment store, will retry delete post next commit", e);
- }
+ try {
+ indexShard.getThreadPool().executor(threadpoolName).execute(() -> {
+ try {
+ remoteDirectory.deleteStaleSegments(LAST_N_METADATA_FILES_TO_KEEP);
+ } catch (IOException e) {
+ logger.info("Exception while deleting stale commits from remote segment store, will retry delete post next commit", e);
+ }
+ });
+ } catch (Exception e) {
+ logger.info("Exception occurred while scheduling deleteStaleCommits", e);
+ } finally {
  onCompletion.run();
- });
+ }
  }
 
  /**