peterxcli opened a new pull request, #11687: URL: https://github.com/apache/ozone/pull/11687
## What changes were proposed in this pull request? When a registered datanode registers again with a new IP address, hostname, ports or version, `SCMNodeManager#register` updates it through `NodeStateManager#updateNode`. `NodeStateMap#updateNode` then replaces the node's `DatanodeEntry` with a new one, and the new entry starts with an empty container list. A rolling upgrade changes the version of every datanode, so each restart during the upgrade takes this path. SCM uses this list to find the replicas a datanode no longer has: - `ContainerReportHandler` compares each full container report with the list, and removes the replicas of containers that are in the list but not in the report. - `DeadNodeHandler` removes a dead datanode's replicas using the same list. Take a datanode that loses a replica while it is down, for example on a failed disk, and then comes back with a new version. The full container report it sends after registering no longer has the replica, and neither does the emptied list, so SCM never removes it. `ReplicationManager` keeps counting the replica and does not see that the container is under-replicated. The replica is not removed when the datanode goes DEAD either. From reading the code, `DatanodeAdminMonitorImpl` is affected too: it reads the same list, so until that first container report is processed, a decommissioning datanode looks like it has no containers to wait for. The list has been lost this way since HDDS-12717 (#8199) moved it from a separate map into `DatanodeEntry`. Before that, `updateNode` only replaced the `DatanodeInfo`. With this change, `NodeStateMap#updateNode` copies the container list into the new entry. The bug was found while testing HDDS-16627 (#11349), the deterministic SCM simulation. ## What is the link to the Apache JIRA https://issues.apache.org/jira/browse/HDDS-16632 ## How was this patch tested? - New `TestSCMNodeManager#testRegisterKeepsContainersOnNodeUpdate`: a datanode with two containers registers again with a new version, then with new ports, then with a new IP address, and keeps both containers each time. Without the fix, the list is empty after the first of these registrations. Each of the three changes empties it on its own. - `TestSCMNodeManager`, `TestNodeStateManager`, `TestNodeStateMap`, `TestDeadNodeHandler`, `TestContainerReportHandler`, `TestIncrementalContainerReportHandler`, `TestNodeReportHandler` and `TestSCMNodeMetrics` pass. -- 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]
