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/11/04 02:26:29 UTC

[PR] PHOENIX-7719 Prewarm HAGroupStore Client [phoenix]

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

   (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-7719 Prewarm HAGroupStore Client [phoenix]

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


##########
phoenix-core-server/src/main/java/org/apache/phoenix/coprocessor/PhoenixRegionServerEndpoint.java:
##########
@@ -195,4 +212,56 @@ public ServerMetadataCache getServerMetadataCache() {
         return ServerMetadataCacheImpl.getInstance(conf);
     }
 
+    /**
+     * Prewarms HAGroupStoreClients in background thread with retry.
+     * Initializes all HA group clients asynchronously at startup.
+     */
+    private void startHAGroupStoreClientPrewarming() {
+        prewarmExecutor = Executors.newSingleThreadExecutor(r -> {
+            Thread t = new Thread(r, "HAGroupStoreClient-Prewarm");
+            t.setDaemon(true);
+            return t;
+        });
+
+        prewarmExecutor.submit(() -> {
+            try {
+                HAGroupStoreManager manager = HAGroupStoreManager.getInstance(conf);
+                if (manager == null) {
+                    LOGGER.warn("HAGroupStoreManager is null, skipping prewarming");
+                    return;
+                }
+
+                List<String> pending = new ArrayList<>(manager.getHAGroupNames());

Review Comment:
   Updated the prewarm method to handle this condition. Also updated the javadoc



-- 
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-7719 Prewarm HAGroupStore Client [phoenix]

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


-- 
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-7719 Prewarm HAGroupStore Client [phoenix]

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


##########
phoenix-core-server/src/main/java/org/apache/phoenix/coprocessor/PhoenixRegionServerEndpoint.java:
##########
@@ -224,17 +236,44 @@ private void startHAGroupStoreClientPrewarming() {
         });
 
         prewarmExecutor.submit(() -> {
-            try {
-                HAGroupStoreManager manager = HAGroupStoreManager.getInstance(conf);
-                if (manager == null) {
-                    LOGGER.warn("HAGroupStoreManager is null, skipping prewarming");
+            HAGroupStoreManager manager = null;
+            List<String> pending = null;
+            // Phase 1: Retry indefinitely until HAGroupStoreManager is initialized
+            // and HAGroupNames are retrieved.
+            while (pending == null) {
+                try {
+                    manager = HAGroupStoreManager.getInstance(conf);
+                    if (manager != null) {
+                        pending = new ArrayList<>(manager.getHAGroupNames());
+                        LOGGER.info("Starting prewarming for {} HAGroupStoreClients",
+                                pending.size());
+                    } else {
+                        LOGGER.debug("HAGroupStoreManager is null, retrying in 2s...");
+                    }
+
+                    if (pending == null) {

Review Comment:
   This should not be necessary, pending cannot be null here. manager.getHAGroupNames() either returns a list possibly empty or an exception, right?



-- 
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-7719 Prewarm HAGroupStore Client [phoenix]

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


##########
phoenix-core-server/src/main/java/org/apache/phoenix/coprocessor/PhoenixRegionServerEndpoint.java:
##########
@@ -195,4 +212,56 @@ public ServerMetadataCache getServerMetadataCache() {
         return ServerMetadataCacheImpl.getInstance(conf);
     }
 
+    /**
+     * Prewarms HAGroupStoreClients in background thread with retry.
+     * Initializes all HA group clients asynchronously at startup.
+     */
+    private void startHAGroupStoreClientPrewarming() {
+        prewarmExecutor = Executors.newSingleThreadExecutor(r -> {
+            Thread t = new Thread(r, "HAGroupStoreClient-Prewarm");
+            t.setDaemon(true);
+            return t;
+        });
+
+        prewarmExecutor.submit(() -> {
+            try {
+                HAGroupStoreManager manager = HAGroupStoreManager.getInstance(conf);
+                if (manager == null) {
+                    LOGGER.warn("HAGroupStoreManager is null, skipping prewarming");
+                    return;
+                }
+
+                List<String> pending = new ArrayList<>(manager.getHAGroupNames());

Review Comment:
   If the HA table region is not ready, manager.getHAGroupNames() would return an exception. We should catch the exception and retry. 



-- 
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-7719 Prewarm HAGroupStore Client [phoenix]

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


##########
phoenix-core-server/src/main/java/org/apache/phoenix/coprocessor/PhoenixRegionServerEndpoint.java:
##########
@@ -224,17 +236,44 @@ private void startHAGroupStoreClientPrewarming() {
         });
 
         prewarmExecutor.submit(() -> {
-            try {
-                HAGroupStoreManager manager = HAGroupStoreManager.getInstance(conf);
-                if (manager == null) {
-                    LOGGER.warn("HAGroupStoreManager is null, skipping prewarming");
+            HAGroupStoreManager manager = null;
+            List<String> pending = null;
+            // Phase 1: Retry indefinitely until HAGroupStoreManager is initialized
+            // and HAGroupNames are retrieved.
+            while (pending == null) {
+                try {
+                    manager = HAGroupStoreManager.getInstance(conf);
+                    if (manager != null) {
+                        pending = new ArrayList<>(manager.getHAGroupNames());
+                        LOGGER.info("Starting prewarming for {} HAGroupStoreClients",
+                                pending.size());
+                    } else {
+                        LOGGER.debug("HAGroupStoreManager is null, retrying in 2s...");
+                    }
+
+                    if (pending == null) {

Review Comment:
   Removed, it also handled case when manager is null, added that explicitly now



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