[
https://issues.apache.org/jira/browse/HDFS-16593?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17599237#comment-17599237
]
ASF GitHub Bot commented on HDFS-16593:
---------------------------------------
Hexiaoqiao commented on code in PR #4353:
URL: https://github.com/apache/hadoop/pull/4353#discussion_r960686589
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java:
##########
@@ -2350,6 +2350,9 @@ private void invalidate(String bpid, Block[] invalidBlks,
boolean async)
removing = volumeMap.remove(bpid, invalidBlks[i]);
addDeletingBlock(bpid, removing.getBlockId());
LOG.debug("Block file {} is to be deleted", removing.getBlockURI());
+ if (datanode.getMetrics() != null) {
Review Comment:
@ZanderXu Sorry to pollute this flow. After review this context carefully, I
realize `removing` will never be null actually. It will continue when meets
null, and will never go to this logic (ref to Line2317). Another one, it is
unnecessary to decide if `datanode.getMetrics()` is null or not. The first
version is without any issues.
I have made a common mistake "ask a programmer to review 10 lines of code
he'll find 10 issues, ask him to do 500 lines and he'll say it looks good."(via
twitter)
"Wish every PR could be merged without any issues reviewers feedback." ^_^
> Correct inaccurate BlocksRemoved metric on DataNode side
> --------------------------------------------------------
>
> Key: HDFS-16593
> URL: https://issues.apache.org/jira/browse/HDFS-16593
> Project: Hadoop HDFS
> Issue Type: Bug
> Reporter: ZanderXu
> Assignee: ZanderXu
> Priority: Minor
> Labels: pull-request-available
> Time Spent: 50m
> Remaining Estimate: 0h
>
> When tracing the root cause of production issue, I found that the
> BlocksRemoved metric on Datanode size was inaccurate.
> {code:java}
> case DatanodeProtocol.DNA_INVALIDATE:
> //
> // Some local block(s) are obsolete and can be
> // safely garbage-collected.
> //
> Block toDelete[] = bcmd.getBlocks();
> try {
> // using global fsdataset
> dn.getFSDataset().invalidate(bcmd.getBlockPoolId(), toDelete);
> } catch(IOException e) {
> // Exceptions caught here are not expected to be disk-related.
> throw e;
> }
> dn.metrics.incrBlocksRemoved(toDelete.length);
> break;
> {code}
> Because even if the invalidate method throws an exception, some blocks may
> have been successfully deleted internally.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
