[
https://issues.apache.org/jira/browse/HDFS-17906?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18072799#comment-18072799
]
ASF GitHub Bot commented on HDFS-17906:
---------------------------------------
Copilot commented on code in PR #8416:
URL: https://github.com/apache/hadoop/pull/8416#discussion_r3067690585
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeRpcServer.java:
##########
@@ -1664,7 +1664,10 @@ public DatanodeCommand blockReport(final
DatanodeRegistration nodeReg,
blocks, context));
}
} else {
- throw new InvalidBlockReportLeaseException(context.getReportId(),
context.getLeaseId());
+ if (bm.shouldRejectInvalidBlockReportLease()) {
+ throw new InvalidBlockReportLeaseException(
+ context.getReportId(), context.getLeaseId());
+ }
Review Comment:
When `shouldRejectInvalidBlockReportLease()` is false and
`checkBlockReportLease()` returns false, this method now falls through and
continues as if the RPC succeeded. That means it will still execute
`bm.removeBRLeaseIfNeeded(nodeReg, context)` later, which can remove the
*current* lease for the DN and update `lastBlockReportTime` even though the
report was not processed. Consider returning early (or guarding
`removeBRLeaseIfNeeded` / last-report-time updates) when the lease is invalid
but rejection is disabled, to avoid incorrectly mutating lease/state for a
report that was ignored.
```suggestion
}
return null;
```
##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockReportLease.java:
##########
@@ -208,6 +210,65 @@ public void testExceptionThrownWhenFBRLeaseExpired()
throws Exception {
}
}
+ /**
+ * Test that when dfs.blockreport.reject.invalid.lease is set to false,
+ * the NameNode does not throw InvalidBlockReportLeaseException for an
+ * expired lease. This is needed for rolling upgrade compatibility where
+ * old DataNodes cannot handle InvalidBlockReportLeaseException and would
+ * get stuck in an infinite loop of rejected block reports.
+ */
+ @Test
+ public void testNoExceptionWhenRejectInvalidLeaseDisabled() throws Exception
{
+ HdfsConfiguration conf = new HdfsConfiguration();
+ conf.setBoolean(
+ DFSConfigKeys.DFS_BLOCKREPORT_REJECT_INVALID_LEASE_KEY, false);
+ Random rand = new Random();
+
+ try (MiniDFSCluster cluster = new MiniDFSCluster.Builder(conf)
+ .numDataNodes(1).build()) {
+ cluster.waitActive();
+
+ FSNamesystem fsn = cluster.getNamesystem();
+ BlockManager blockManager = fsn.getBlockManager();
+ BlockManager spyBlockManager = spy(blockManager);
+ fsn.setBlockManagerForTesting(spyBlockManager);
+ String poolId = cluster.getNamesystem().getBlockPoolId();
+
+ NamenodeProtocols rpcServer = cluster.getNameNodeRpc();
+
+ DataNode dn = cluster.getDataNodes().get(0);
+ DatanodeDescriptor datanodeDescriptor = spyBlockManager
+ .getDatanodeManager().getDatanode(dn.getDatanodeId());
+
+ DatanodeRegistration dnRegistration = dn.getDNRegistrationForBP(poolId);
+ StorageReport[] storages = dn.getFSDataset().getStorageReports(poolId);
+
+ // Send heartbeat and request full block report lease
+ HeartbeatResponse hbResponse = rpcServer.sendHeartbeat(
+ dnRegistration, storages, 0, 0, 0, 0, 0, null, true,
+ SlowPeerReports.EMPTY_REPORT, SlowDiskReports.EMPTY_REPORT);
Review Comment:
This test assumes the heartbeat granted a non-zero full block report lease.
If `hbResponse.getFullBlockReportLeaseId()` is 0, the subsequent `blockReport`
bypasses lease checking and the test no longer validates the intended behavior.
Add an assertion that the lease ID is non-zero before removing the lease /
sending the report, so the test fails clearly if a lease wasn’t granted.
```suggestion
SlowPeerReports.EMPTY_REPORT, SlowDiskReports.EMPTY_REPORT);
assertTrue(hbResponse.getFullBlockReportLeaseId() != 0,
"Expected heartbeat to grant a non-zero full block report lease");
```
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java:
##########
@@ -1119,6 +1119,8 @@ public class DFSConfigKeys extends
CommonConfigurationKeys {
public static final int
DFS_NAMENODE_MAX_FULL_BLOCK_REPORT_LEASES_DEFAULT = 6;
public static final String DFS_NAMENODE_FULL_BLOCK_REPORT_LEASE_LENGTH_MS =
"dfs.namenode.full.block.report.lease.length.ms";
public static final long
DFS_NAMENODE_FULL_BLOCK_REPORT_LEASE_LENGTH_MS_DEFAULT = 5L * 60L * 1000L;
+ public static final String DFS_BLOCKREPORT_REJECT_INVALID_LEASE_KEY =
"dfs.blockreport.reject.invalid.lease";
+ public static final boolean DFS_BLOCKREPORT_REJECT_INVALID_LEASE_DEFAULT =
true;
Review Comment:
New configuration keys are typically documented in `hdfs-default.xml`
alongside related `dfs.blockreport.*` properties (e.g.,
`dfs.blockreport.intervalMsec`). Please add a corresponding `<property>` entry
for `dfs.blockreport.reject.invalid.lease` describing its purpose (rolling
upgrade compatibility), scope (NameNode-side), and default value.
> Rolling upgrade: old DataNodes get stuck in infinite invalid block report
> lease loop
> ------------------------------------------------------------------------------------
>
> Key: HDFS-17906
> URL: https://issues.apache.org/jira/browse/HDFS-17906
> Project: Hadoop HDFS
> Issue Type: Bug
> Components: datanode
> Reporter: dzcxzl
> Priority: Major
> Labels: pull-request-available
>
>
> HDFS-16942 introduced `InvalidBlockReportLeaseException`, which the NameNode
> now throws back to the DataNode via RPC when a block report is rejected due
> to an invalid lease. On a DataNode that also includes HDFS-16942, the
> exception is caught and `fullBlockReportLeaseId` is reset to 0, allowing the
> DN to request a new lease on the next heartbeat and retry.
> However, during a rolling upgrade where the NameNode has been upgraded (with
> HDFS-16942) but DataNodes are still running an older version (without
> HDFS-16942), the old DataNode code does not have the
> `InvalidBlockReportLeaseException` handling branch in
> `BPServiceActor.offerService()`. This causes the DN to enter an infinite
> failure loop where it can never successfully send a full block report.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
