Skip to content

Commit b4c9fdd

Browse files
Removing String format in RemoteStoreMigrationAllocationDecider to optimise performance(#14612)
Signed-off-by: RS146BIJAY <[email protected]> (cherry picked from commit e82b432) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
1 parent 47526e7 commit b4c9fdd

File tree

1 file changed

+16
-18
lines changed

1 file changed

+16
-18
lines changed

server/src/main/java/org/opensearch/cluster/routing/allocation/decider/RemoteStoreMigrationAllocationDecider.java

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,6 @@
4444
import org.opensearch.node.remotestore.RemoteStoreNodeService.CompatibilityMode;
4545
import org.opensearch.node.remotestore.RemoteStoreNodeService.Direction;
4646

47-
import java.util.Locale;
48-
4947
/**
5048
* A new allocation decider for migration of document replication clusters to remote store backed clusters:
5149
* - For STRICT compatibility mode, the decision is always YES
@@ -101,7 +99,7 @@ public Decision canAllocate(ShardRouting shardRouting, RoutingNode node, Routing
10199
if (migrationDirection.equals(Direction.NONE)) {
102100
// remote backed indices on docrep nodes and non remote backed indices on remote nodes are not allowed
103101
boolean isNoDecision = remoteSettingsBackedIndex ^ targetNode.isRemoteStoreNode();
104-
String reason = String.format(Locale.ROOT, " for %sremote store backed index", remoteSettingsBackedIndex ? "" : "non ");
102+
String reason = " for " + (remoteSettingsBackedIndex ? "" : "non ") + "remote store backed index";
105103
return allocation.decision(
106104
isNoDecision ? Decision.NO : Decision.YES,
107105
NAME,
@@ -114,11 +112,9 @@ public Decision canAllocate(ShardRouting shardRouting, RoutingNode node, Routing
114112
// check for remote store backed indices
115113
if (remoteSettingsBackedIndex && targetNode.isRemoteStoreNode() == false) {
116114
// allocations and relocations must be to a remote node
117-
String reason = String.format(
118-
Locale.ROOT,
119-
" because a remote store backed index's shard copy can only be %s to a remote node",
120-
((shardRouting.assignedToNode() == false) ? "allocated" : "relocated")
121-
);
115+
String reason = new StringBuilder(" because a remote store backed index's shard copy can only be ").append(
116+
(shardRouting.assignedToNode() == false) ? "allocated" : "relocated"
117+
).append(" to a remote node").toString();
122118
return allocation.decision(Decision.NO, NAME, getDecisionDetails(false, shardRouting, targetNode, reason));
123119
}
124120

@@ -168,16 +164,18 @@ private Decision replicaShardDecision(ShardRouting replicaShardRouting, Discover
168164

169165
// get detailed reason for the decision
170166
private String getDecisionDetails(boolean isYes, ShardRouting shardRouting, DiscoveryNode targetNode, String reason) {
171-
return String.format(
172-
Locale.ROOT,
173-
"[%s migration_direction]: %s shard copy %s be %s to a %s node%s",
174-
migrationDirection.direction,
175-
(shardRouting.primary() ? "primary" : "replica"),
176-
(isYes ? "can" : "can not"),
177-
((shardRouting.assignedToNode() == false) ? "allocated" : "relocated"),
178-
(targetNode.isRemoteStoreNode() ? "remote" : "non-remote"),
179-
reason
180-
);
167+
return new StringBuilder("[").append(migrationDirection.direction)
168+
.append(" migration_direction]: ")
169+
.append(shardRouting.primary() ? "primary" : "replica")
170+
.append(" shard copy ")
171+
.append(isYes ? "can" : "can not")
172+
.append(" be ")
173+
.append((shardRouting.assignedToNode() == false) ? "allocated" : "relocated")
174+
.append(" to a ")
175+
.append(targetNode.isRemoteStoreNode() ? "remote" : "non-remote")
176+
.append(" node")
177+
.append(reason)
178+
.toString();
181179
}
182180

183181
}

0 commit comments

Comments
 (0)