Skip to content

Commit

Permalink
Removing String format in RemoteStoreMigrationAllocationDecider to op…
Browse files Browse the repository at this point in the history
…timise performance(opensearch-project#14612) (opensearch-project#14874)

Signed-off-by: RS146BIJAY <rishavsagar4b1@gmail.com>
Signed-off-by: kkewwei <kkewwei@163.com>
  • Loading branch information
RS146BIJAY authored and kkewwei committed Jul 24, 2024
1 parent 264384d commit 1c21b08
Showing 1 changed file with 16 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,6 @@
import org.opensearch.node.remotestore.RemoteStoreNodeService.CompatibilityMode;
import org.opensearch.node.remotestore.RemoteStoreNodeService.Direction;

import java.util.Locale;

/**
* A new allocation decider for migration of document replication clusters to remote store backed clusters:
* - For STRICT compatibility mode, the decision is always YES
Expand Down Expand Up @@ -101,7 +99,7 @@ public Decision canAllocate(ShardRouting shardRouting, RoutingNode node, Routing
if (migrationDirection.equals(Direction.NONE)) {
// remote backed indices on docrep nodes and non remote backed indices on remote nodes are not allowed
boolean isNoDecision = remoteSettingsBackedIndex ^ targetNode.isRemoteStoreNode();
String reason = String.format(Locale.ROOT, " for %sremote store backed index", remoteSettingsBackedIndex ? "" : "non ");
String reason = " for " + (remoteSettingsBackedIndex ? "" : "non ") + "remote store backed index";
return allocation.decision(
isNoDecision ? Decision.NO : Decision.YES,
NAME,
Expand All @@ -114,11 +112,9 @@ public Decision canAllocate(ShardRouting shardRouting, RoutingNode node, Routing
// check for remote store backed indices
if (remoteSettingsBackedIndex && targetNode.isRemoteStoreNode() == false) {
// allocations and relocations must be to a remote node
String reason = String.format(
Locale.ROOT,
" because a remote store backed index's shard copy can only be %s to a remote node",
((shardRouting.assignedToNode() == false) ? "allocated" : "relocated")
);
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));
}

Expand Down Expand Up @@ -168,16 +164,18 @@ private Decision replicaShardDecision(ShardRouting replicaShardRouting, Discover

// get detailed reason for the decision
private String getDecisionDetails(boolean isYes, ShardRouting shardRouting, DiscoveryNode targetNode, String reason) {
return String.format(
Locale.ROOT,
"[%s migration_direction]: %s shard copy %s be %s to a %s node%s",
migrationDirection.direction,
(shardRouting.primary() ? "primary" : "replica"),
(isYes ? "can" : "can not"),
((shardRouting.assignedToNode() == false) ? "allocated" : "relocated"),
(targetNode.isRemoteStoreNode() ? "remote" : "non-remote"),
reason
);
return new StringBuilder("[").append(migrationDirection.direction)
.append(" migration_direction]: ")
.append(shardRouting.primary() ? "primary" : "replica")
.append(" shard copy ")
.append(isYes ? "can" : "can not")
.append(" be ")
.append((shardRouting.assignedToNode() == false) ? "allocated" : "relocated")
.append(" to a ")
.append(targetNode.isRemoteStoreNode() ? "remote" : "non-remote")
.append(" node")
.append(reason)
.toString();
}

}

0 comments on commit 1c21b08

Please sign in to comment.