Skip to content

Commit 5d2d392

Browse files
authored
Skip unnecessary string format in RemoteStoreMigrationAllocationDecider when not in debug mode (#16299)
Signed-off-by: Rishab Nahata <rnnahata@amazon.com>
1 parent 20536ee commit 5d2d392

File tree

1 file changed

+35
-10
lines changed

1 file changed

+35
-10
lines changed

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

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ public Decision canAllocate(ShardRouting shardRouting, RoutingNode node, Routing
8989
return allocation.decision(
9090
Decision.YES,
9191
NAME,
92-
getDecisionDetails(true, shardRouting, targetNode, " for strict compatibility mode")
92+
getDecisionDetails(true, shardRouting, targetNode, " for strict compatibility mode", allocation)
9393
);
9494
}
9595

@@ -103,19 +103,23 @@ public Decision canAllocate(ShardRouting shardRouting, RoutingNode node, Routing
103103
return allocation.decision(
104104
isNoDecision ? Decision.NO : Decision.YES,
105105
NAME,
106-
getDecisionDetails(!isNoDecision, shardRouting, targetNode, reason)
106+
getDecisionDetails(!isNoDecision, shardRouting, targetNode, reason, allocation)
107107
);
108108
} else if (migrationDirection.equals(Direction.DOCREP)) {
109109
// docrep migration direction is currently not supported
110-
return allocation.decision(Decision.YES, NAME, getDecisionDetails(true, shardRouting, targetNode, " for DOCREP direction"));
110+
return allocation.decision(
111+
Decision.YES,
112+
NAME,
113+
getDecisionDetails(true, shardRouting, targetNode, " for DOCREP direction", allocation)
114+
);
111115
} else {
112116
// check for remote store backed indices
113117
if (remoteSettingsBackedIndex && targetNode.isRemoteStoreNode() == false) {
114118
// allocations and relocations must be to a remote node
115119
String reason = new StringBuilder(" because a remote store backed index's shard copy can only be ").append(
116120
(shardRouting.assignedToNode() == false) ? "allocated" : "relocated"
117121
).append(" to a remote node").toString();
118-
return allocation.decision(Decision.NO, NAME, getDecisionDetails(false, shardRouting, targetNode, reason));
122+
return allocation.decision(Decision.NO, NAME, getDecisionDetails(false, shardRouting, targetNode, reason, allocation));
119123
}
120124

121125
if (shardRouting.primary()) {
@@ -128,9 +132,9 @@ public Decision canAllocate(ShardRouting shardRouting, RoutingNode node, Routing
128132
// handle scenarios for allocation of a new shard's primary copy
129133
private Decision primaryShardDecision(ShardRouting primaryShardRouting, DiscoveryNode targetNode, RoutingAllocation allocation) {
130134
if (targetNode.isRemoteStoreNode() == false) {
131-
return allocation.decision(Decision.NO, NAME, getDecisionDetails(false, primaryShardRouting, targetNode, ""));
135+
return allocation.decision(Decision.NO, NAME, getDecisionDetails(false, primaryShardRouting, targetNode, "", allocation));
132136
}
133-
return allocation.decision(Decision.YES, NAME, getDecisionDetails(true, primaryShardRouting, targetNode, ""));
137+
return allocation.decision(Decision.YES, NAME, getDecisionDetails(true, primaryShardRouting, targetNode, "", allocation));
134138
}
135139

136140
// Checks if primary shard is on a remote node.
@@ -150,20 +154,41 @@ private Decision replicaShardDecision(ShardRouting replicaShardRouting, Discover
150154
return allocation.decision(
151155
Decision.NO,
152156
NAME,
153-
getDecisionDetails(false, replicaShardRouting, targetNode, " since primary shard copy is not yet migrated to remote")
157+
getDecisionDetails(
158+
false,
159+
replicaShardRouting,
160+
targetNode,
161+
" since primary shard copy is not yet migrated to remote",
162+
allocation
163+
)
154164
);
155165
}
156166
return allocation.decision(
157167
Decision.YES,
158168
NAME,
159-
getDecisionDetails(true, replicaShardRouting, targetNode, " since primary shard copy has been migrated to remote")
169+
getDecisionDetails(
170+
true,
171+
replicaShardRouting,
172+
targetNode,
173+
" since primary shard copy has been migrated to remote",
174+
allocation
175+
)
160176
);
161177
}
162-
return allocation.decision(Decision.YES, NAME, getDecisionDetails(true, replicaShardRouting, targetNode, ""));
178+
return allocation.decision(Decision.YES, NAME, getDecisionDetails(true, replicaShardRouting, targetNode, "", allocation));
163179
}
164180

165181
// get detailed reason for the decision
166-
private String getDecisionDetails(boolean isYes, ShardRouting shardRouting, DiscoveryNode targetNode, String reason) {
182+
private String getDecisionDetails(
183+
boolean isYes,
184+
ShardRouting shardRouting,
185+
DiscoveryNode targetNode,
186+
String reason,
187+
RoutingAllocation allocation
188+
) {
189+
if (allocation.debugDecision() == false) {
190+
return null;
191+
}
167192
return new StringBuilder("[").append(migrationDirection.direction)
168193
.append(" migration_direction]: ")
169194
.append(shardRouting.primary() ? "primary" : "replica")

0 commit comments

Comments
 (0)