Himanshu-g81 commented on code in PR #2302:
URL: https://github.com/apache/phoenix/pull/2302#discussion_r2447519933
##########
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:
> This is because the ACTIVE cluster will start in ANIS state. The writer
might not be active so setting this to current timestamp
I think in that case it would be 0? i.e. cluster is not in sync but we don't
have exact timestamp on when it was last in sync, we will not purge anything
during compaction on standby side and as soon as it's in sync, this timestamp
will also be updated.
> Also, setting null means we are current, it's more of a convention that we
adopt.
@ritegarg if the cluster is in sync, can we keep it currentTime (and not
null?) (i.e. last timestamp when cluster was in sync - at the time when API is
called, which is curren time). On reader it's handled with same assumption.
--
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]
