You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by "ritegarg (via GitHub)" <gi...@apache.org> on 2025/08/14 07:59:26 UTC

[PR] PHOENIX-7566 HAGroupState subscription feature and state management for ReplicationLogReader [phoenix]

ritegarg opened a new pull request, #2274:
URL: https://github.com/apache/phoenix/pull/2274

   (no comment)


-- 
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: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] PHOENIX-7566 HAGroupState subscription feature and state management for ReplicationLogReader [phoenix]

Posted by "kadirozde (via GitHub)" <gi...@apache.org>.
kadirozde commented on code in PR #2274:
URL: https://github.com/apache/phoenix/pull/2274#discussion_r2347792999


##########
phoenix-core-client/src/main/java/org/apache/phoenix/jdbc/HAGroupStoreClient.java:
##########
@@ -293,10 +308,29 @@ public void setHAGroupStatusIfNeeded(HAGroupStoreRecord.HAGroupState haGroupStat
         }
         if (isUpdateNeeded(currentHAGroupStoreRecord.getHAGroupState(),
                 currentHAGroupStoreRecordStat.getMtime(), haGroupState)) {
+                // We maintain last sync time as the last time cluster was in sync state.
+                // If state changes from ACTIVE_IN_SYNC to ACTIVE_NOT_IN_SYNC, record that time
+                // Once state changes back to ACTIVE_IN_SYNC or the role is
+                // NOT ACTIVE or ACTIVE_TO_STANDBY
+                // set the time to null to mark that we are current(or we don't have any reader).
+                // TODO: Verify that for reader this is the correct approach.
+                Long lastSyncTimeInMs = currentHAGroupStoreRecord
+                        .getLastSyncStateTimeInMs();
+                ClusterRole clusterRole = haGroupState.getClusterRole();
+                if (currentHAGroupStoreRecord.getHAGroupState()
+                        == HAGroupStoreRecord.HAGroupState.ACTIVE_IN_SYNC
+                        && haGroupState == HAGroupStoreRecord.HAGroupState.ACTIVE_NOT_IN_SYNC) {
+                    lastSyncTimeInMs = System.currentTimeMillis();

Review Comment:
   This should be a filed in the HA Store record in Zookeeper. This field is updated when the cluster switches from ACTIVE_IN_SYNC to ACTIVE_NOT_IN_SYNC. When a region server of the standby cluster starts and the active cluster is in ACTIVE_NOT_IN_SYNC, the region server needs to know lastSyncTimeInMs for establishing the consistency point for the standby cluster.



-- 
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: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] PHOENIX-7566 HAGroupState subscription feature and state management for ReplicationLogReader [phoenix]

Posted by "kadirozde (via GitHub)" <gi...@apache.org>.
kadirozde merged PR #2274:
URL: https://github.com/apache/phoenix/pull/2274


-- 
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: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] PHOENIX-7566 HAGroupState subscription feature and state management for ReplicationLogReader [phoenix]

Posted by "ritegarg (via GitHub)" <gi...@apache.org>.
ritegarg commented on code in PR #2274:
URL: https://github.com/apache/phoenix/pull/2274#discussion_r2349822075


##########
phoenix-core-client/src/main/java/org/apache/phoenix/jdbc/HAGroupStoreClient.java:
##########
@@ -293,10 +308,29 @@ public void setHAGroupStatusIfNeeded(HAGroupStoreRecord.HAGroupState haGroupStat
         }
         if (isUpdateNeeded(currentHAGroupStoreRecord.getHAGroupState(),
                 currentHAGroupStoreRecordStat.getMtime(), haGroupState)) {
+                // We maintain last sync time as the last time cluster was in sync state.
+                // If state changes from ACTIVE_IN_SYNC to ACTIVE_NOT_IN_SYNC, record that time
+                // Once state changes back to ACTIVE_IN_SYNC or the role is
+                // NOT ACTIVE or ACTIVE_TO_STANDBY
+                // set the time to null to mark that we are current(or we don't have any reader).
+                // TODO: Verify that for reader this is the correct approach.
+                Long lastSyncTimeInMs = currentHAGroupStoreRecord
+                        .getLastSyncStateTimeInMs();
+                ClusterRole clusterRole = haGroupState.getClusterRole();
+                if (currentHAGroupStoreRecord.getHAGroupState()
+                        == HAGroupStoreRecord.HAGroupState.ACTIVE_IN_SYNC
+                        && haGroupState == HAGroupStoreRecord.HAGroupState.ACTIVE_NOT_IN_SYNC) {
+                    lastSyncTimeInMs = System.currentTimeMillis();

Review Comment:
   Yes, I have added that new field 
   https://github.com/apache/phoenix/pull/2274/files#diff-8d6c9a772a5a7947b4f4f3c98f460924658742acf4bbb22d53f45c6d241f3cdaR166
   It is updated here https://github.com/apache/phoenix/pull/2274/files#diff-61fd251b01270c601f6cc2851770e91d36d12eafec0aa709782649be2a286cfbR340



-- 
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: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] PHOENIX-7566 HAGroupState subscription feature and state management for ReplicationLogReader [phoenix]

Posted by "ritegarg (via GitHub)" <gi...@apache.org>.
ritegarg commented on code in PR #2274:
URL: https://github.com/apache/phoenix/pull/2274#discussion_r2353557571


##########
phoenix-core-client/src/main/java/org/apache/phoenix/jdbc/HAGroupStoreClient.java:
##########
@@ -638,4 +692,189 @@ private boolean isUpdateNeeded(HAGroupStoreRecord.HAGroupState currentHAGroupSta
         return ((System.currentTimeMillis() - currentHAGroupStoreRecordMtime) > waitTime);
     }
 
+    // ========== HA Group State Change Subscription Methods ==========
+
+    /**
+     * Subscribe to be notified when a specific state transition occurs.
+     *
+     * @param fromState the state to transition from
+     * @param toState the state to transition to
+     * @param clusterType whether to monitor local or peer cluster
+     * @param listener the listener to notify when the transition occurs
+     */
+    public void subscribeToTransition(HAGroupState fromState, HAGroupState toState,

Review Comment:
   Removed specific transition methods and also updated test 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: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] PHOENIX-7566 HAGroupState subscription feature and state management for ReplicationLogReader [phoenix]

Posted by "kadirozde (via GitHub)" <gi...@apache.org>.
kadirozde commented on code in PR #2274:
URL: https://github.com/apache/phoenix/pull/2274#discussion_r2347803316


##########
phoenix-core-client/src/main/java/org/apache/phoenix/jdbc/HAGroupStoreClient.java:
##########
@@ -638,4 +692,189 @@ private boolean isUpdateNeeded(HAGroupStoreRecord.HAGroupState currentHAGroupSta
         return ((System.currentTimeMillis() - currentHAGroupStoreRecordMtime) > waitTime);
     }
 
+    // ========== HA Group State Change Subscription Methods ==========
+
+    /**
+     * Subscribe to be notified when a specific state transition occurs.
+     *
+     * @param fromState the state to transition from
+     * @param toState the state to transition to
+     * @param clusterType whether to monitor local or peer cluster
+     * @param listener the listener to notify when the transition occurs
+     */
+    public void subscribeToTransition(HAGroupState fromState, HAGroupState toState,

Review Comment:
   I feel that we do not need to complicate matters by having subscriptions at the state transition level. I think subscriptions at <toState, clusterType> should be good enough. A subscriber does not need to know the details of the state transitions as it mostly cares about the target state. Also, subscription at the transition level creates coupling between subscribers and the HA state machine, which is not necessary. 



-- 
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: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org