SaketaChalamchala commented on code in PR #5888:
URL: https://github.com/apache/ozone/pull/5888#discussion_r1462554970
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java:
##########
@@ -300,15 +302,15 @@ private void updateStatus(UUID dnId, long scmCmdId,
}
break;
default:
- LOG.error("Can not update to Unknown new Status: {}", newStatus);
+ LOG.error("Unable to update from unknown DN report status: {}",
newStatus);
Review Comment:
Thanks for the patch @xichen01. Shouldn't the log here say `Unable to update
to <status>` instead of `Unable to update from <status>`?
Even in the above log changes for `updateStatus()` it's not really clear if
`DN report status` is the new status that the SCM command should be updated to.
May I ask what was unclear in the old log messages?
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java:
##########
@@ -322,9 +324,6 @@ private void removeTimeoutScmCommand(UUID dnId,
CmdStatusData state = removeScmCommand(dnId, scmCmdId);
LOG.warn("Remove Timeout SCM BlockDeletionCommand {} for DN {} " +
Review Comment:
I think the log message here could be updated to `LOG.warn("SCM
BlockDeletionCommand {} for DN {} was removed after {}ms without update",
state, dnId, timeoutMs)`. Let me know what you think?
--
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]
