From 8cfaaaecad11a23652bd7a8eb27be6eac4d47fc4 Mon Sep 17 00:00:00 2001 From: Kiran Prakash Date: Tue, 23 Apr 2024 23:29:42 -0700 Subject: [PATCH] address code comments Signed-off-by: Kiran Prakash --- .../indices/IndicesRequestCacheIT.java | 24 +++++++++++-------- .../common/settings/ClusterSettings.java | 2 +- .../indices/IndicesRequestCache.java | 4 ++-- 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/indices/IndicesRequestCacheIT.java b/server/src/internalClusterTest/java/org/opensearch/indices/IndicesRequestCacheIT.java index 1aa28e6d7a0c0..ee9701d3bdf1a 100644 --- a/server/src/internalClusterTest/java/org/opensearch/indices/IndicesRequestCacheIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/indices/IndicesRequestCacheIT.java @@ -34,6 +34,7 @@ import com.carrotsearch.randomizedtesting.annotations.ParametersFactory; +import org.opensearch.action.admin.cluster.node.stats.NodeStats; import org.opensearch.action.admin.cluster.node.stats.NodesStatsResponse; import org.opensearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest; import org.opensearch.action.admin.indices.alias.Alias; @@ -974,7 +975,7 @@ public void testStaleKeysCleanup_NoStaleThresholdShouldCleanUpStaleKeysFromCache } // when cache cleaner interval setting is not set, cache cleaner is configured appropriately with the fall-back setting - public void testStaleKeysCleanup_NoIntervalSettingFallsBackAppropriately() throws Exception { + public void testStaleKeysCleanup_testFallbackSettings() throws Exception { int cacheCleanIntervalInMillis = 1; String node = internalCluster().startNode( Settings.builder().put(INDICES_CACHE_CLEANUP_INTERVAL_SETTING_KEY, TimeValue.timeValueMillis(cacheCleanIntervalInMillis)) @@ -1067,7 +1068,7 @@ public void testStalesKeyCleanupWithDynamicThresholdUpdate() throws Exception { } // staleness threshold dynamic updates should throw exceptions on invalid input - public void testStaleKeysCleanup_ThresholdUpdatesShouldThrowExceptionsAppropriately() throws Exception { + public void testStaleKeysCleanup_ThresholdUpdatesShouldThrowExceptionsOnInvalidInput() throws Exception { int cacheCleanIntervalInMillis = 1; String node = internalCluster().startNode( Settings.builder() @@ -1087,15 +1088,13 @@ public void testStaleKeysCleanup_ThresholdUpdatesShouldThrowExceptionsAppropriat assertTrue(getRequestCacheStats(client, index1).getMemorySizeInBytes() > 0); // Update indices.requests.cache.cleanup.staleness_threshold to "10%" with illegal argument - try { + assertThrows("Ratio should be in [0-1.0]", IllegalArgumentException.class, () -> { ClusterUpdateSettingsRequest updateSettingsRequest = new ClusterUpdateSettingsRequest(); updateSettingsRequest.persistentSettings( - Settings.builder().put(INDICES_REQUEST_CACHE_STALENESS_THRESHOLD_SETTING.getKey(), 10) + Settings.builder().put(IndicesRequestCache.INDICES_REQUEST_CACHE_CLEANUP_STALENESS_THRESHOLD_SETTING_KEY, 10) ); - assertAcked(client().admin().cluster().updateSettings(updateSettingsRequest).actionGet()); - } catch (Exception e) { - assert (e instanceof IllegalArgumentException); - } + client().admin().cluster().updateSettings(updateSettingsRequest).actionGet(); + }); // everything else should continue to work fine later on. // force refresh so that it creates 1 stale key @@ -1181,7 +1180,7 @@ public void testCacheCleanupAfterDeletingIndex() throws Exception { } // when staleness threshold is lower than staleness, it should clean the cache from all indices having stale keys - public void testStaleKeysCleanup_CleanUpStaleKeysDeletesAppropriatelyAcrossMultipleIndices() throws Exception { + public void testStaleKeysCleanup_CleansUpStaleKeysAcrossMultipleIndices() throws Exception { int cacheCleanIntervalInMillis = 50; String node = internalCluster().startNode( Settings.builder() @@ -1273,6 +1272,11 @@ private static RequestCacheStats getRequestCacheStats(Client client, String inde private static RequestCacheStats getNodeCacheStats(Client client) { NodesStatsResponse stats = client.admin().cluster().prepareNodesStats().execute().actionGet(); - return stats.getNodes().get(0).getIndices().getRequestCache(); + for (NodeStats stat : stats.getNodes()) { + if (stat.getNode().isDataNode()) { + return stat.getIndices().getRequestCache(); + } + } + return null; } } diff --git a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java index af91823d708d3..bea4b3a3d18a8 100644 --- a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java @@ -493,7 +493,7 @@ public void apply(Settings value, Settings current, Settings previous) { IndicesFieldDataCache.INDICES_FIELDDATA_CACHE_SIZE_KEY, IndicesRequestCache.INDICES_CACHE_QUERY_SIZE, IndicesRequestCache.INDICES_CACHE_QUERY_EXPIRE, - IndicesRequestCache.INDICES_REQUEST_CACHE_CLEAN_INTERVAL_SETTING, + IndicesRequestCache.INDICES_REQUEST_CACHE_CLEANUP_INTERVAL_SETTING, IndicesRequestCache.INDICES_REQUEST_CACHE_STALENESS_THRESHOLD_SETTING, HunspellService.HUNSPELL_LAZY_LOAD, HunspellService.HUNSPELL_IGNORE_CASE, diff --git a/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java b/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java index 8a7d501726e38..49d0141546cd2 100644 --- a/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java +++ b/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java @@ -131,7 +131,7 @@ public final class IndicesRequestCache implements RemovalListener INDICES_REQUEST_CACHE_CLEAN_INTERVAL_SETTING = Setting.positiveTimeSetting( + public static final Setting INDICES_REQUEST_CACHE_CLEANUP_INTERVAL_SETTING = Setting.positiveTimeSetting( INDICES_REQUEST_CACHE_CLEANUP_INTERVAL_SETTING_KEY, INDICES_CACHE_CLEAN_INTERVAL_SETTING, Property.NodeScope @@ -171,7 +171,7 @@ public final class IndicesRequestCache implements RemovalListener, BytesReference> weigher = (k, v) -> k.ramBytesUsed(k.key.ramBytesUsed()) + v.ramBytesUsed(); this.cacheCleanupManager = new IndicesRequestCacheCleanupManager( threadPool, - INDICES_REQUEST_CACHE_CLEAN_INTERVAL_SETTING.get(settings), + INDICES_REQUEST_CACHE_CLEANUP_INTERVAL_SETTING.get(settings), getStalenessThreshold(settings) ); this.cacheEntityLookup = cacheEntityFunction;