ndimiduk commented on a change in pull request #812: HBASE-23275: Track active master's address in ActiveMasterManager URL: https://github.com/apache/hbase/pull/812#discussion_r344971133
########## File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java ########## @@ -57,12 +56,18 @@ final AtomicBoolean clusterHasActiveMaster = new AtomicBoolean(false); final AtomicBoolean clusterShutDown = new AtomicBoolean(false); + // This server's information. private final ServerName sn; private int infoPort; private final Server master; + // Active master's server name. Invalidated anytime active master changes (based on ZK + // notifications) and lazily fetched on-demand. + // ServerName is immutable, so we don't need heavy synchronization around it. Review comment: Why is the `AtomicReference` necessary at all? If concurrent threads are updating this value, last write wins. The `ServerName` struct is assigned as a complete piece of data, only one field is updated, so there's no worry of seeing half of a write. If two threads find a `null` value, they'll both reach out to ZK to retrieve a value, so you're not preventing duplicate work. If the duplicate work thing isn't a concern, I think a `volatile` field will do the trick. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
