From c19356d08e056c6240e92b759925ff1f4e86b049 Mon Sep 17 00:00:00 2001 From: Sachin Kale Date: Mon, 19 Aug 2024 19:03:13 +0530 Subject: [PATCH] Re-factor some of the remote store settings Signed-off-by: Sachin Kale --- .../index/remote/RemoteStoreUtils.java | 22 +++++++++++++++++++ .../store/RemoteSegmentStoreDirectory.java | 6 ++--- .../indices/RemoteStoreSettings.java | 8 +++---- .../main/java/org/opensearch/node/Node.java | 3 +-- .../RemoteStorePinnedTimestampService.java | 9 +++----- 5 files changed, 33 insertions(+), 15 deletions(-) diff --git a/server/src/main/java/org/opensearch/index/remote/RemoteStoreUtils.java b/server/src/main/java/org/opensearch/index/remote/RemoteStoreUtils.java index ea6b1b13278b4..6569e7c699c2b 100644 --- a/server/src/main/java/org/opensearch/index/remote/RemoteStoreUtils.java +++ b/server/src/main/java/org/opensearch/index/remote/RemoteStoreUtils.java @@ -21,8 +21,10 @@ import org.opensearch.common.collect.Tuple; import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; +import org.opensearch.indices.RemoteStoreSettings; import org.opensearch.node.remotestore.RemoteStoreNodeAttribute; import org.opensearch.node.remotestore.RemoteStoreNodeService; +import org.opensearch.node.remotestore.RemoteStorePinnedTimestampService; import java.nio.ByteBuffer; import java.util.ArrayList; @@ -511,4 +513,24 @@ public static List filterOutMetadataFilesBasedOnAge( } return metadataFilesWithMinAge; } + + /** + * Determines if the pinned timestamp state is stale based on the provided remote store settings. + * + * This method checks if the last successful fetch timestamp is older than a calculated stale buffer time. + * The stale buffer time is computed using the pinned timestamps scheduler interval and lookback interval + * from the remote store settings. + * + * @return true if the pinned timestamp state is considered stale, false otherwise. + * + * @throws NullPointerException if remoteStoreSettings is null. + * @throws IllegalStateException if unable to retrieve the pinned timestamps. + */ + public static boolean isPinnedTimestampStateStale() { + long lastSuccessfulFetchTimestamp = RemoteStorePinnedTimestampService.getPinnedTimestamps().v1(); + long staleBufferInMillis = (RemoteStoreSettings.getPinnedTimestampsSchedulerInterval().millis() * 3) + RemoteStoreSettings + .getPinnedTimestampsLookbackInterval() + .millis(); + return lastSuccessfulFetchTimestamp < (System.currentTimeMillis() - staleBufferInMillis); + } } diff --git a/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java b/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java index f267b7ea8f5b6..6b31f74a3caf2 100644 --- a/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java +++ b/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java @@ -40,6 +40,7 @@ import org.opensearch.index.store.lockmanager.RemoteStoreMetadataLockManager; import org.opensearch.index.store.remote.metadata.RemoteSegmentMetadata; import org.opensearch.index.store.remote.metadata.RemoteSegmentMetadataHandler; +import org.opensearch.indices.RemoteStoreSettings; import org.opensearch.indices.replication.checkpoint.ReplicationCheckpoint; import org.opensearch.node.remotestore.RemoteStorePinnedTimestampService; import org.opensearch.threadpool.ThreadPool; @@ -830,7 +831,7 @@ public void deleteStaleSegments(int lastNMetadataFilesToKeep) throws IOException } // Check last fetch status of pinned timestamps. If stale, return. - if (RemoteStorePinnedTimestampService.isPinnedTimestampStateStale()) { + if (RemoteStoreUtils.isPinnedTimestampStateStale()) { logger.warn("Skipping remote segment store garbage collection as last fetch of pinned timestamp is stale"); return; } @@ -861,8 +862,7 @@ public void deleteStaleSegments(int lastNMetadataFilesToKeep) throws IOException // Along with last N files, we need to keep files since last successful run of scheduler long lastSuccessfulFetchOfPinnedTimestamps = pinnedTimestampsState.v1(); - long minimumAgeInMillis = lastSuccessfulFetchOfPinnedTimestamps + RemoteStorePinnedTimestampService - .getPinnedTimestampsLookbackInterval() + long minimumAgeInMillis = lastSuccessfulFetchOfPinnedTimestamps + RemoteStoreSettings.getPinnedTimestampsLookbackInterval() .getMillis(); metadataFilesEligibleToDelete = RemoteStoreUtils.filterOutMetadataFilesBasedOnAge( metadataFilesEligibleToDelete, diff --git a/server/src/main/java/org/opensearch/indices/RemoteStoreSettings.java b/server/src/main/java/org/opensearch/indices/RemoteStoreSettings.java index bb038a0ea5deb..495288626627b 100644 --- a/server/src/main/java/org/opensearch/indices/RemoteStoreSettings.java +++ b/server/src/main/java/org/opensearch/indices/RemoteStoreSettings.java @@ -163,8 +163,8 @@ public class RemoteStoreSettings { private volatile RemoteStoreEnums.PathHashAlgorithm pathHashAlgorithm; private volatile int maxRemoteTranslogReaders; private volatile boolean isTranslogMetadataEnabled; - private volatile TimeValue pinnedTimestampsSchedulerInterval; - private volatile TimeValue pinnedTimestampsLookbackInterval; + private static volatile TimeValue pinnedTimestampsSchedulerInterval; + private static volatile TimeValue pinnedTimestampsLookbackInterval; public RemoteStoreSettings(Settings settings, ClusterSettings clusterSettings) { clusterRemoteTranslogBufferInterval = CLUSTER_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING.get(settings); @@ -273,11 +273,11 @@ private void setMaxRemoteTranslogReaders(int maxRemoteTranslogReaders) { this.maxRemoteTranslogReaders = maxRemoteTranslogReaders; } - public TimeValue getPinnedTimestampsSchedulerInterval() { + public static TimeValue getPinnedTimestampsSchedulerInterval() { return pinnedTimestampsSchedulerInterval; } - public TimeValue getPinnedTimestampsLookbackInterval() { + public static TimeValue getPinnedTimestampsLookbackInterval() { return pinnedTimestampsLookbackInterval; } } diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index d4605933675c4..e5a2b1720a0f7 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -818,8 +818,7 @@ protected Node( repositoriesServiceReference::get, settings, threadPool, - clusterService, - remoteStoreSettings + clusterService ); resourcesToClose.add(remoteStorePinnedTimestampService); } else { diff --git a/server/src/main/java/org/opensearch/node/remotestore/RemoteStorePinnedTimestampService.java b/server/src/main/java/org/opensearch/node/remotestore/RemoteStorePinnedTimestampService.java index 54f5111944836..fc5ec3c0e83f8 100644 --- a/server/src/main/java/org/opensearch/node/remotestore/RemoteStorePinnedTimestampService.java +++ b/server/src/main/java/org/opensearch/node/remotestore/RemoteStorePinnedTimestampService.java @@ -57,7 +57,6 @@ public class RemoteStorePinnedTimestampService implements Closeable { private final Settings settings; private final ThreadPool threadPool; private final ClusterService clusterService; - private final RemoteStoreSettings remoteStoreSettings; private BlobStoreRepository blobStoreRepository; private BlobStoreTransferService blobStoreTransferService; private RemoteStorePinnedTimestampsBlobStore pinnedTimestampsBlobStore; @@ -68,14 +67,12 @@ public RemoteStorePinnedTimestampService( Supplier repositoriesService, Settings settings, ThreadPool threadPool, - ClusterService clusterService, - RemoteStoreSettings remoteStoreSettings + ClusterService clusterService ) { this.repositoriesService = repositoriesService; this.settings = settings; this.threadPool = threadPool; this.clusterService = clusterService; - this.remoteStoreSettings = remoteStoreSettings; } /** @@ -86,7 +83,7 @@ public RemoteStorePinnedTimestampService( public void start() { validateRemoteStoreConfiguration(); initializeComponents(); - startAsyncUpdateTask(remoteStoreSettings.getPinnedTimestampsSchedulerInterval()); + startAsyncUpdateTask(RemoteStoreSettings.getPinnedTimestampsSchedulerInterval()); } private void validateRemoteStoreConfiguration() { @@ -126,7 +123,7 @@ private void startAsyncUpdateTask(TimeValue pinnedTimestampsSchedulerInterval) { public void pinTimestamp(long timestamp, String pinningEntity, ActionListener listener) { // If a caller uses current system time to pin the timestamp, following check will almost always fail. // So, we allow pinning timestamp in the past upto some buffer - long lookbackIntervalInMills = remoteStoreSettings.getPinnedTimestampsLookbackInterval().millis(); + long lookbackIntervalInMills = RemoteStoreSettings.getPinnedTimestampsLookbackInterval().millis(); if (timestamp < (System.currentTimeMillis() - lookbackIntervalInMills)) { throw new IllegalArgumentException( "Timestamp to be pinned is less than current timestamp - value of cluster.remote_store.pinned_timestamps.lookback_interval"