Skip to content

Commit

Permalink
Skip unnecessary string format in RemoteStoreMigrationAllocationDecid…
Browse files Browse the repository at this point in the history
…er when not in debug mode (opensearch-project#16299)

Signed-off-by: Rishab Nahata <rnnahata@amazon.com>
  • Loading branch information
imRishN authored and dk2k committed Oct 21, 2024
1 parent e905417 commit 4c22df0
Showing 1 changed file with 35 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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)
);
}

Expand All @@ -103,19 +103,23 @@ 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) {
// allocations and relocations must be to a remote node
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()) {
Expand All @@ -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.
Expand All @@ -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() == false) {
return null;
}
return new StringBuilder("[").append(migrationDirection.direction)
.append(" migration_direction]: ")
.append(shardRouting.primary() ? "primary" : "replica")
Expand Down

0 comments on commit 4c22df0

Please sign in to comment.