kadirozde commented on code in PR #2302:
URL: https://github.com/apache/phoenix/pull/2302#discussion_r2467024852
##########
phoenix-core-client/src/main/java/org/apache/phoenix/jdbc/HAGroupStoreClient.java:
##########
@@ -374,110 +402,403 @@ public HAGroupStoreRecord
getHAGroupStoreRecordFromPeer() throws IOException {
if (!isHealthy) {
throw new IOException("HAGroupStoreClient is not healthy");
}
- return fetchCacheRecord(this.peerPathChildrenCache,
ClusterType.PEER).getLeft();
+ return
fetchCacheRecordAndPopulateZKIfNeeded(this.peerPathChildrenCache,
ClusterType.PEER).getLeft();
}
- private void initializeZNodeIfNeeded() throws IOException,
- StaleHAGroupStoreRecordVersionException {
+ private void initializeZNodeIfNeeded() throws IOException, SQLException {
// Sometimes the ZNode might not be available in local cache yet, so
better to check
// in ZK directly if we need to initialize
Pair<HAGroupStoreRecord, Stat> cacheRecordFromZK =
phoenixHaAdmin.getHAGroupStoreRecordInZooKeeper(this.haGroupName);
HAGroupStoreRecord haGroupStoreRecord = cacheRecordFromZK.getLeft();
- HAGroupState defaultHAGroupState =
this.clusterRole.getDefaultHAGroupState();
- // Initialize lastSyncTimeInMs only if we start in ACTIVE_NOT_IN_SYNC
state
- // and ZNode is not already present
- Long lastSyncTimeInMs =
defaultHAGroupState.equals(HAGroupState.ACTIVE_NOT_IN_SYNC)
- ? System.currentTimeMillis()
- : null;
- HAGroupStoreRecord newHAGroupStoreRecord = new HAGroupStoreRecord(
- HAGroupStoreRecord.DEFAULT_PROTOCOL_VERSION,
- haGroupName,
- this.clusterRole.getDefaultHAGroupState(),
- lastSyncTimeInMs
- );
- // Only update current ZNode if it doesn't have the same role as
present in System Table.
- // If not exists, then create ZNode
- // TODO: Discuss if this approach is what reader needs.
- if (haGroupStoreRecord != null &&
- !haGroupStoreRecord.getClusterRole().equals(this.clusterRole))
{
- phoenixHaAdmin.updateHAGroupStoreRecordInZooKeeper(haGroupName,
- newHAGroupStoreRecord,
cacheRecordFromZK.getRight().getVersion());
- } else if (haGroupStoreRecord == null) {
+ // Only if the ZNode is not present, we need to create it from System
Table
+ if (haGroupStoreRecord == null) {
+ SystemTableHAGroupRecord systemTableRecord
+ = getSystemTableHAGroupRecord(this.haGroupName);
+ Preconditions.checkNotNull(systemTableRecord,
+ "System Table HAGroupRecord cannot be null");
+ HAGroupStoreRecord.HAGroupState defaultHAGroupState
+ =
systemTableRecord.getClusterRole().getDefaultHAGroupState();
+ Long lastSyncTimeInMs
+ =
defaultHAGroupState.equals(HAGroupStoreRecord.HAGroupState.ACTIVE_NOT_IN_SYNC)
+ ? System.currentTimeMillis()
Review Comment:
I am not sure I understand the value of setting anything other than zero
when we do not know/determine the value for lastSyncTimeInMs. What are the
consumers supposed to do when they see the value is -1 (instead of zero)?
Regardless if it is zero or -1, the behavior will be the same. So, let us set
it to zero in all such cases.
--
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]
