Code review minor fixes related formatting and naming convention

Author: Harsh Kothari

server/src/main/java/org/opensearch/autoforcemerge/AutoForceMergeManager.java

@@ -54,10 +54,10 @@ public class AutoForceMergeManager extends AbstractLifecycleComponent {
     private final JvmService jvmService;
     private final IndicesService indicesService;
     private final ClusterService clusterService;
-    private AsyncForceMergeTask task;
-    private ConfigurationValidator configurationValidator;
-    private NodeValidator nodeValidator;
-    private ShardValidator shardValidator;
+    private final AsyncForceMergeTask task;
+    private final ConfigurationValidator configurationValidator;
+    private final NodeValidator nodeValidator;
+    private final ShardValidator shardValidator;
     private final ForceMergeManagerSettings forceMergeManagerSettings;
     private final AtomicBoolean initialCheckDone = new AtomicBoolean(false);
 
@@ -92,20 +92,18 @@ protected void doClose() {
     }
 
     private void triggerForceMerge() {
-        if (!forceMergeManagerSettings.isAutoForceMergeFeatureEnabled()) {
-            logger.info("Cluster configuration shows auto force merge feature is disabled. Closing task.");
+        if (forceMergeManagerSettings.isAutoForceMergeFeatureEnabled() == false) {
+            logger.debug("Cluster configuration shows auto force merge feature is disabled. Closing task.");
             return;
         }
-        if (!configurationValidator.hasWarmNodes()) {
+        if (configurationValidator.hasWarmNodes() == false) {
             logger.debug("No warm nodes found. Skipping Auto Force merge.");
             return;
         }
-
-        if (!(nodeValidator.validate().isAllowed())) {
+        if (nodeValidator.validate().isAllowed() == false) {
             logger.debug("Node capacity constraints are not allowing to trigger auto ForceMerge");
             return;
         }
-
         List<IndexShard> shards = new ArrayList<>();
         for (IndexService indexService : indicesService) {
             for (IndexShard shard : indexService) {
@@ -116,28 +114,23 @@ private void triggerForceMerge() {
                 }
             }
         }
-
         List<IndexShard> sortedShards = getSortedShardsByTranslogAge(shards);
         int iteration = nodeValidator.getMaxConcurrentForceMerges();
         for (IndexShard shard : sortedShards) {
-            if (!nodeValidator.validate().isAllowed()) {
+            if (iteration == 0 || nodeValidator.validate().isAllowed() == false) {
                 logger.debug("Node conditions no longer suitable for force merge");
                 break;
             }
-            if (iteration == 0) {
-                break;
-            }
             iteration--;
             CompletableFuture.runAsync(() -> {
                 try {
                     shard.forceMerge(new ForceMergeRequest()
                         .maxNumSegments(forceMergeManagerSettings.getSegmentCountThreshold()));
-                    logger.info("Merging is completed successfully for the shard {}", shard.shardId());
+                    logger.debug("Merging is completed successfully for the shard {}", shard.shardId());
                 } catch (IOException e) {
                     logger.error("Error during force merge for shard {}", shard.shardId(), e);
                 }
             }, threadPool.executor(ThreadPool.Names.FORCE_MERGE));
-
             logger.debug("Successfully triggered force merge for shard {}", shard.shardId());
             try {
                 Thread.sleep(forceMergeManagerSettings.getForcemergeDelay().getMillis());
@@ -159,13 +152,13 @@ private List<IndexShard> getSortedShardsByTranslogAge(List<IndexShard> shards) {
     private class ShardAgeComparator implements Comparator<IndexShard> {
         @Override
         public int compare(IndexShard s1, IndexShard s2) {
-            long age1 = getTranslogAge(s1);
-            long age2 = getTranslogAge(s2);
+            long age1 = getEarliestLastModifiedAge(s1);
+            long age2 = getEarliestLastModifiedAge(s2);
             return Long.compare(age1, age2);
         }
     }
 
-    private long getTranslogAge(IndexShard shard) {
+    private long getEarliestLastModifiedAge(IndexShard shard) {
         CommonStatsFlags flags = new CommonStatsFlags(CommonStatsFlags.Flag.Translog);
         CommonStats stats = new CommonStats(indicesService.getIndicesQueryCache(), shard, flags);
         return stats.getTranslog() != null ? stats.getTranslog().getEarliestLastModifiedAge() : 0;
@@ -194,16 +187,16 @@ protected class ConfigurationValidator implements ValidationStrategy {
          */
         @Override
         public ValidationResult validate() {
-            if (!forceMergeManagerSettings.isAutoForceMergeFeatureEnabled()) {
+            if (forceMergeManagerSettings.isAutoForceMergeFeatureEnabled() == false) {
                 logger.debug("Cluster configuration shows auto force merge feature is disabled. Closing task.");
                 return new ValidationResult(false);
             }
             initializeIfNeeded();
-            if (!isRemoteStoreEnabled) {
+            if (isRemoteStoreEnabled == false) {
                 logger.debug("Cluster configuration is not meeting the criteria. Closing task.");
                 return new ValidationResult(false);
             }
-            if (!isOnlyDataNode) {
+            if (isOnlyDataNode == false) {
                 logger.debug("Node configuration doesn't meet requirements. Closing task.");
                 task.close();
                 return new ValidationResult(false);
@@ -220,7 +213,7 @@ public ValidationResult validate() {
          * Thread-safe through atomic operation on initialCheckDone.
          */
         private void initializeIfNeeded() {
-            if (!initialCheckDone.get()) {
+            if (initialCheckDone.get() == false) {
                 DiscoveryNode localNode = clusterService.localNode();
                 isOnlyDataNode = localNode.isDataNode() && !localNode.isWarmNode();
                 isRemoteStoreEnabled = isRemoteStorageEnabled();
@@ -239,7 +232,7 @@ private boolean isRemoteStorageEnabled() {
          * Checks if cluster has warm nodes.
          */
         private boolean hasWarmNodes() {
-            if (hasWarmNodes) return true;
+            if (hasWarmNodes == true) return true;
             ClusterState clusterState = clusterService.state();
             return hasWarmNodes = clusterState.getNodes().getNodes()
                 .values()
@@ -270,8 +263,8 @@ public ValidationResult validate() {
                 logger.debug("JVM memory usage too high: {}%", jvmUsedPercent);
                 return new ValidationResult(false);
             }
-            if (!areForceMergeThreadsAvailable()) {
-                logger.info("No force merge threads available");
+            if (areForceMergeThreadsAvailable() == false) {
+                logger.debug("No force merge threads available");
                 return new ValidationResult(false);
             }
             return new ValidationResult(true);
@@ -309,8 +302,8 @@ public ValidationResult validate(IndexShard shard) {
                 logger.debug("No shard found.");
                 return new ValidationResult(false);
             }
-            if (!isIndexWarmCandidate(shard)) {
-                logger.info("Shard {} doesn't belong to a warm candidate index", shard.shardId());
+            if (isIndexWarmCandidate(shard) == false) {
+                logger.debug("Shard {} doesn't belong to a warm candidate index", shard.shardId());
                 return new ValidationResult(false);
             }
             CommonStats stats = new CommonStats(indicesService.getIndicesQueryCache(), shard, flags);
@@ -331,6 +324,10 @@ private boolean isIndexWarmCandidate(IndexShard shard) {
             IndexSettings indexSettings = shard.indexSettings();
             return indexSettings.getScopedSettings().get(IndexSettings.INDEX_IS_WARM_CANDIDATE_INDEX);
         }
+
+        private boolean isRelocating(IndexShard shard){
+            return false;
+        }
     }
 
     /**
@@ -396,7 +393,7 @@ protected boolean mustReschedule() {
          */
         @Override
         protected void runInternal() {
-            if (!initialCheckDone.get() && !(configurationValidator.validate().isAllowed())) {
+            if (initialCheckDone.get() == false && configurationValidator.validate().isAllowed() == false) {
                 return;
             }
             triggerForceMerge();

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

@@ -244,11 +244,11 @@ public Settings getValue(Settings current, Settings previous) {
         public void apply(Settings value, Settings current, Settings previous) {
             for (String key : value.keySet()) {
                 assert loggerPredicate.test(key);
-                String component = key.substring("logger." .length());
-                if ("level" .equals(component)) {
+                String component = key.substring("logger.".length());
+                if ("level".equals(component)) {
                     continue;
                 }
-                if ("_root" .equals(component)) {
+                if ("_root".equals(component)) {
                     final String rootLevel = value.get(key);
                     if (rootLevel == null) {
                         Loggers.setLevel(LogManager.getRootLogger(), Loggers.LOG_DEFAULT_LEVEL_SETTING.get(settings));

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

@@ -978,7 +978,7 @@ public boolean isDerivedFieldAllowed() {
      * while index level settings will overwrite node settings.
      *
      * @param indexMetadata the index metadata this settings object is associated with
-     * @param nodeSettings  the nodes settings this index is allocated on.
+     * @param nodeSettings the nodes settings this index is allocated on.
      */
     public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSettings) {
         this(indexMetadata, nodeSettings, IndexScopedSettings.DEFAULT_SCOPED_SETTINGS);
@@ -989,7 +989,7 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti
      * while index level settings will overwrite node settings.
      *
      * @param indexMetadata the index metadata this settings object is associated with
-     * @param nodeSettings  the nodes settings this index is allocated on.
+     * @param nodeSettings the nodes settings this index is allocated on.
      */
     public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSettings, IndexScopedSettings indexScopedSettings) {
         scopedSettings = indexScopedSettings.copy(nodeSettings, indexMetadata);
@@ -1325,7 +1325,6 @@ public String customDataPath() {
 
     /**
      * Returns the version the index was created on.
-     *
      * @see IndexMetadata#indexCreated(Settings)
      */
     public Version getIndexVersionCreated() {
@@ -1366,7 +1365,7 @@ public boolean isCompositeIndex() {
 
     /**
      * Returns true if segment replication is enabled on the index.
-     * <p>
+     *
      * Every shard on a remote node would also have SegRep enabled even without
      * proper index setting during the migration.
      */
@@ -1529,7 +1528,6 @@ public void setTranslogSyncInterval(TimeValue translogSyncInterval) {
     /**
      * Returns the translog sync/upload buffer interval when remote translog store is enabled and index setting
      * {@code index.translog.durability} is set as {@code request}.
-     *
      * @return the buffer interval.
      */
     public TimeValue getRemoteTranslogUploadBufferInterval() {

server/src/main/java/org/opensearch/node/Node.java

@@ -367,6 +367,7 @@ public class Node implements Closeable {
      * Note that this does not control whether the node stores actual indices (see
      * {@link #NODE_DATA_SETTING}). However, if this is false, {@link #NODE_DATA_SETTING}
      * and {@link #NODE_MASTER_SETTING} must also be false.
+     *
      */
     public static final Setting<Boolean> NODE_LOCAL_STORAGE_SETTING = Setting.boolSetting(
         "node.local_storage",
@@ -382,7 +383,7 @@ public class Node implements Closeable {
                 && (Character.isWhitespace(value.charAt(0)) || Character.isWhitespace(value.charAt(value.length() - 1)))) {
                 throw new IllegalArgumentException(key + " cannot have leading or trailing whitespace " + "[" + value + "]");
             }
-            if (value.length() > 0 && "node.attr.server_name" .equals(key)) {
+            if (value.length() > 0 && "node.attr.server_name".equals(key)) {
                 try {
                     new SNIHostName(value);
                 } catch (IllegalArgumentException e) {
@@ -895,8 +896,8 @@ protected Node(
             directoryFactories.putAll(builtInDirectoryFactories);
 
             final Map<String, IndexStorePlugin.RecoveryStateFactory> recoveryStateFactories = pluginsService.filterPlugins(
-                    IndexStorePlugin.class
-                )
+                IndexStorePlugin.class
+            )
                 .stream()
                 .map(IndexStorePlugin::getRecoveryStateFactories)
                 .flatMap(m -> m.entrySet().stream())
@@ -1445,9 +1446,9 @@ protected Node(
 
             final Optional<TaskManagerClient> taskManagerClientOptional = FeatureFlags.isEnabled(BACKGROUND_TASK_EXECUTION_EXPERIMENTAL)
                 ? pluginsService.filterPlugins(TaskManagerClientPlugin.class)
-                .stream()
-                .map(plugin -> plugin.getTaskManagerClient(client, clusterService, threadPool))
-                .findFirst()
+                    .stream()
+                    .map(plugin -> plugin.getTaskManagerClient(client, clusterService, threadPool))
+                    .findFirst()
                 : Optional.empty();
 
             final PersistentTasksExecutorRegistry registry = new PersistentTasksExecutorRegistry(tasksExecutors);
@@ -2026,12 +2027,9 @@ protected void validateNodeBeforeAcceptingRequests(
         final BootstrapContext context,
         final BoundTransportAddress boundTransportAddress,
         List<BootstrapCheck> bootstrapChecks
-    ) throws NodeValidationException {
-    }
+    ) throws NodeValidationException {}
 
-    /**
-     * Writes a file to the logs dir containing the ports for the given transport type
-     */
+    /** Writes a file to the logs dir containing the ports for the given transport type */
     private void writePortsFile(String type, BoundTransportAddress boundAddress) {
         Path tmpPortsFile = environment.logsDir().resolve(type + ".ports.tmp");
         try (BufferedWriter writer = Files.newBufferedWriter(tmpPortsFile, StandardCharsets.UTF_8)) {
@@ -2059,7 +2057,6 @@ protected PluginsService getPluginsService() {
 
     /**
      * Creates a new {@link CircuitBreakerService} based on the settings provided.
-     *
      * @see #BREAKER_TYPE_KEY
      */
     public static CircuitBreakerService createCircuitBreakerService(
@@ -2135,7 +2132,6 @@ protected ScriptService newScriptService(Settings settings, Map<String, ScriptEn
 
     /**
      * Get Custom Name Resolvers list based on a Discovery Plugins list
-     *
      * @param discoveryPlugins Discovery plugins list
      */
     private List<NetworkService.CustomNameResolver> getCustomNameResolvers(List<DiscoveryPlugin> discoveryPlugins) {
@@ -2149,9 +2145,7 @@ private List<NetworkService.CustomNameResolver> getCustomNameResolvers(List<Disc
         return customNameResolvers;
     }
 
-    /**
-     * Constructs a ClusterInfoService which may be mocked for tests.
-     */
+    /** Constructs a ClusterInfoService which may be mocked for tests. */
     protected ClusterInfoService newClusterInfoService(
         Settings settings,
         ClusterService clusterService,
@@ -2166,9 +2160,7 @@ protected ClusterInfoService newClusterInfoService(
         return service;
     }
 
-    /**
-     * Constructs a {@link org.opensearch.http.HttpServerTransport} which may be mocked for tests.
-     */
+    /** Constructs a {@link org.opensearch.http.HttpServerTransport} which may be mocked for tests. */
     protected HttpServerTransport newHttpTransport(NetworkModule networkModule) {
         return networkModule.getHttpServerTransportSupplier().get();
     }

test/framework/src/main/java/org/opensearch/remotestore/RemoteStoreBaseIntegTestCase.java

@@ -183,7 +183,7 @@ protected Settings remoteStoreRepoSettings() {
     }
 
     protected void setFailRate(String repoName, int value) throws ExecutionException, InterruptedException {
-        GetRepositoriesRequest gr = new GetRepositoriesRequest(new String[]{repoName});
+        GetRepositoriesRequest gr = new GetRepositoriesRequest(new String[] { repoName });
         GetRepositoriesResponse res = client().admin().cluster().getRepositories(gr).get();
         RepositoryMetadata rmd = res.repositories().get(0);
         Settings.Builder settings = Settings.builder()