From 60c167c8d801172eb735afb149d27b7943004d57 Mon Sep 17 00:00:00 2001 From: Rishab Nahata Date: Sun, 13 Oct 2024 13:52:31 +0530 Subject: [PATCH] Skip unnecessary string format in RemoteStoreMigrationAllocationDecider when not in debug mode Signed-off-by: Rishab Nahata --- ...RemoteStoreMigrationAllocationDecider.java | 45 ++++++++++++++----- 1 file changed, 35 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/routing/allocation/decider/RemoteStoreMigrationAllocationDecider.java b/server/src/main/java/org/opensearch/cluster/routing/allocation/decider/RemoteStoreMigrationAllocationDecider.java index 67fe4ea1dcb1b..518a3e98e64f4 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/allocation/decider/RemoteStoreMigrationAllocationDecider.java +++ b/server/src/main/java/org/opensearch/cluster/routing/allocation/decider/RemoteStoreMigrationAllocationDecider.java @@ -89,7 +89,7 @@ public Decision canAllocate(ShardRouting shardRouting, RoutingNode node, Routing return allocation.decision( Decision.YES, NAME, - getDecisionDetails(true, shardRouting, targetNode, " for strict compatibility mode") + getDecisionDetails(true, shardRouting, targetNode, " for strict compatibility mode", allocation) ); } @@ -103,11 +103,15 @@ public Decision canAllocate(ShardRouting shardRouting, RoutingNode node, Routing return allocation.decision( isNoDecision ? Decision.NO : Decision.YES, NAME, - getDecisionDetails(!isNoDecision, shardRouting, targetNode, reason) + getDecisionDetails(!isNoDecision, shardRouting, targetNode, reason, allocation) ); } else if (migrationDirection.equals(Direction.DOCREP)) { // docrep migration direction is currently not supported - return allocation.decision(Decision.YES, NAME, getDecisionDetails(true, shardRouting, targetNode, " for DOCREP direction")); + return allocation.decision( + Decision.YES, + NAME, + getDecisionDetails(true, shardRouting, targetNode, " for DOCREP direction", allocation) + ); } else { // check for remote store backed indices if (remoteSettingsBackedIndex && targetNode.isRemoteStoreNode() == false) { @@ -115,7 +119,7 @@ public Decision canAllocate(ShardRouting shardRouting, RoutingNode node, Routing String reason = new StringBuilder(" because a remote store backed index's shard copy can only be ").append( (shardRouting.assignedToNode() == false) ? "allocated" : "relocated" ).append(" to a remote node").toString(); - return allocation.decision(Decision.NO, NAME, getDecisionDetails(false, shardRouting, targetNode, reason)); + return allocation.decision(Decision.NO, NAME, getDecisionDetails(false, shardRouting, targetNode, reason, allocation)); } if (shardRouting.primary()) { @@ -128,9 +132,9 @@ public Decision canAllocate(ShardRouting shardRouting, RoutingNode node, Routing // handle scenarios for allocation of a new shard's primary copy private Decision primaryShardDecision(ShardRouting primaryShardRouting, DiscoveryNode targetNode, RoutingAllocation allocation) { if (targetNode.isRemoteStoreNode() == false) { - return allocation.decision(Decision.NO, NAME, getDecisionDetails(false, primaryShardRouting, targetNode, "")); + return allocation.decision(Decision.NO, NAME, getDecisionDetails(false, primaryShardRouting, targetNode, "", allocation)); } - return allocation.decision(Decision.YES, NAME, getDecisionDetails(true, primaryShardRouting, targetNode, "")); + return allocation.decision(Decision.YES, NAME, getDecisionDetails(true, primaryShardRouting, targetNode, "", allocation)); } // Checks if primary shard is on a remote node. @@ -150,20 +154,41 @@ private Decision replicaShardDecision(ShardRouting replicaShardRouting, Discover return allocation.decision( Decision.NO, NAME, - getDecisionDetails(false, replicaShardRouting, targetNode, " since primary shard copy is not yet migrated to remote") + getDecisionDetails( + false, + replicaShardRouting, + targetNode, + " since primary shard copy is not yet migrated to remote", + allocation + ) ); } return allocation.decision( Decision.YES, NAME, - getDecisionDetails(true, replicaShardRouting, targetNode, " since primary shard copy has been migrated to remote") + getDecisionDetails( + true, + replicaShardRouting, + targetNode, + " since primary shard copy has been migrated to remote", + allocation + ) ); } - return allocation.decision(Decision.YES, NAME, getDecisionDetails(true, replicaShardRouting, targetNode, "")); + return allocation.decision(Decision.YES, NAME, getDecisionDetails(true, replicaShardRouting, targetNode, "", allocation)); } // get detailed reason for the decision - private String getDecisionDetails(boolean isYes, ShardRouting shardRouting, DiscoveryNode targetNode, String reason) { + private String getDecisionDetails( + boolean isYes, + ShardRouting shardRouting, + DiscoveryNode targetNode, + String reason, + RoutingAllocation allocation + ) { + if (allocation.debugDecision()) { + return ""; + } return new StringBuilder("[").append(migrationDirection.direction) .append(" migration_direction]: ") .append(shardRouting.primary() ? "primary" : "replica")