ss77892 commented on PR #11025: URL: https://github.com/apache/ozone/pull/11025#issuecomment-5297637180
A couple comments from my second brain: (1) with the setCurrentVersion call removed from HddsDatanodeService.initializeDatanodeDetails(), it looks like nothing sets currentVersion before the datanode registers. On a fresh start the builder now defaults to DEFAULT_VERSION, on a restart the value in datanode.id is dropped because the yaml setter is now a no-op, and the only remaining write happens in HeartbeatEndpointTask, which runs after registration. Since RegisterEndpointTask serializes the details object directly, every registration ends up advertising DEFAULT_VERSION to SCM until the first heartbeat corrects it, a short window, but it briefly reports an even older version than the pre-ZDU. (2) A small follow-up on the HeartbeatEndpointTask change: the task now mutates the shared DatanodeDetails via setCurrentVersion on every call(), and since the endpoint executor runs one thread per SCM plus Recon, several tasks can do this concurrently. currentVersion used to be write-once at startup, but with ZDU it's now intentionally updated at runtime, which puts it in the same category as persistedOpState and persistedOpStateExpiryEpochSec, both volatile for exactly that reason. Without volatile there's no visibility guarantee between threads, so one endpoint could keep reporting a stale version after another already updated it. Could we mark currentVersion volatile to match? -- 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]
