adoroszlai commented on code in PR #5944:
URL: https://github.com/apache/ozone/pull/5944#discussion_r1444661826
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/pipeline/Pipeline.java:
##########
@@ -316,15 +316,15 @@ public boolean isAllocationTimeout() {
}
public void setNodesInOrder(List<DatanodeDetails> nodes) {
- nodesInOrder.set(nodes);
+ nodesInOrder = nodes;
}
public List<DatanodeDetails> getNodesInOrder() {
- if (nodesInOrder.get() == null || nodesInOrder.get().isEmpty()) {
+ if (nodesInOrder == null || nodesInOrder.isEmpty()) {
LOG.debug("Nodes in order is empty, delegate to getNodes");
return getNodes();
}
- return nodesInOrder.get();
+ return nodesInOrder;
Review Comment:
Same here.
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/pipeline/Pipeline.java:
##########
@@ -79,7 +79,7 @@ public static Codec<Pipeline> getCodec() {
private Map<DatanodeDetails, Long> nodeStatus;
private Map<DatanodeDetails, Integer> replicaIndexes;
// nodes with ordered distance to client
- private ThreadLocal<List<DatanodeDetails>> nodesInOrder = new
ThreadLocal<>();
+ private volatile List<DatanodeDetails> nodesInOrder = null;
Review Comment:
nit: `= null` is unnecessary
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/pipeline/Pipeline.java:
##########
@@ -287,11 +287,11 @@ public DatanodeDetails
getClosestNode(Set<DatanodeDetails> excluded)
if (excluded == null) {
excluded = Collections.emptySet();
}
- if (nodesInOrder.get() == null || nodesInOrder.get().isEmpty()) {
+ if (nodesInOrder == null || nodesInOrder.isEmpty()) {
LOG.debug("Nodes in order is empty, delegate to getFirstNode");
return getFirstNode(excluded);
}
- for (DatanodeDetails d : nodesInOrder.get()) {
+ for (DatanodeDetails d : nodesInOrder) {
Review Comment:
When using `volatile` member repeatedly in the same method, I think it's
recommended to assign it to a local variable:
* avoid the cost of repeated `volatile` access
* ensure the same instance is used in all statements
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
