jpisaac commented on code in PR #2224:
URL: https://github.com/apache/phoenix/pull/2224#discussion_r2208865755


##########
phoenix-core-client/src/main/java/org/apache/phoenix/jdbc/HAGroupStoreManager.java:
##########
@@ -18,63 +18,78 @@
 package org.apache.phoenix.jdbc;
 
 import org.apache.hadoop.conf.Configuration;
+import org.apache.phoenix.exception.InvalidClusterRoleTransitionException;
+import org.apache.phoenix.exception.StaleHAGroupStoreRecordVersionException;
+
 import java.io.IOException;
-import static 
org.apache.phoenix.query.QueryServices.CLUSTER_ROLE_BASED_MUTATION_BLOCK_ENABLED;
-import static 
org.apache.phoenix.query.QueryServicesOptions.DEFAULT_CLUSTER_ROLE_BASED_MUTATION_BLOCK_ENABLED;
+import java.util.Optional;
 
-public class HAGroupStoreManager {
-    private static volatile HAGroupStoreManager haGroupStoreManagerInstance;
-    private final boolean mutationBlockEnabled;
-    private final Configuration conf;
+/**
+ * Interface for managing HA group store operations including mutation 
blocking checks
+ * and client invalidation.
+ */
+public interface HAGroupStoreManager {
 
     /**
-     * Creates/gets an instance of HAGroupStoreManager.
+     * Checks whether mutation is blocked or not across all HA groups.
+     * If any HAGroupStoreClient instance is not created, it will be created.
+     * If any HAGroup mutation is blocked, it will return true.
+     * @throws IOException when HAGroupStoreClient is not healthy.
+     */
+    boolean isMutationBlocked(Configuration conf) throws IOException;

Review Comment:
   "conf" can be a implementation property/attribute passed in via the 
constructor. The api would be cleaner without needing the conf parameter 
everytime. Ideally the conf is inspected once in the constructor and be done 
with it.



-- 
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]

Reply via email to