You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "kgeisz (via GitHub)" <gi...@apache.org> on 2025/11/20 03:32:26 UTC

[PR] HBASE-29715: AssignmentManager is trying to pick up the active cluster's tables before refreshing meta and hfiles [hbase]

kgeisz opened a new pull request, #7474:
URL: https://github.com/apache/hbase/pull/7474

   https://issues.apache.org/jira/browse/HBASE-29715
   
   In a read-replica setup, two or more clusters share the same filesystem.  This means the replica cluster can pick up tables created by the active cluster during metrics collection, even when the replica cluster has not had its metadata and HFiles refreshed yet.  PR https://github.com/apache/hbase/pull/7304 introduced some logic to the `TABLE_TO_REGIONS_COUNT` case in `HMaster.getClusterMetricsWithoutCoprocessor()` that skips metrics collection for foreign meta tables.
   
   This pull request broadens this logic to skip all foreign tables.  This change will prevent the replica cluster from logging something like the following:
   
   ```
   2025-11-14T03:22:08,098 ERROR [master/hbase-docker-2:16000.Chore.1] master.TableStateManager: Unable to get table testTable1 state
   org.apache.hadoop.hbase.TableNotFoundException: No state found for testTable1
    at org.apache.hadoop.hbase.master.TableStateManager.getTableState(TableStateManager.java:141) ~[hbase-server-4.0.0-alpha-1-SNAPSHOT.jar:4.0.0-alpha-1-SNAPSHOT]
    at org.apache.hadoop.hbase.master.TableStateManager.isTableState(TableStateManager.java:79) ~[hbase-server-4.0.0-alpha-1-SNAPSHOT.jar:4.0.0-alpha-1-SNAPSHOT]
    at org.apache.hadoop.hbase.master.assignment.AssignmentManager.isTableDisabled(AssignmentManager.java:549) ~[hbase-server-4.0.0-alpha-1-SNAPSHOT.jar:4.0.0-alpha-1-SNAPSHOT]
    at org.apache.hadoop.hbase.master.assignment.AssignmentManager.getRegionStatesCount(AssignmentManager.java:2637) ~[hbase-server-4.0.0-alpha-1-SNAPSHOT.jar:4.0.0-alpha-1-SNAPSHOT]
    at org.apache.hadoop.hbase.master.HMaster.getClusterMetricsWithoutCoprocessor(HMaster.java:3130) ~[hbase-server-4.0.0-alpha-1-SNAPSHOT.jar:4.0.0-alpha-1-SNAPSHOT]
    at org.apache.hadoop.hbase.master.HMaster.getClusterMetricsWithoutCoprocessor(HMaster.java:3017) ~[hbase-server-4.0.0-alpha-1-SNAPSHOT.jar:4.0.0-alpha-1-SNAPSHOT]
    at org.apache.hadoop.hbase.master.balancer.ClusterStatusChore.chore(ClusterStatusChore.java:47) ~[hbase-server-4.0.0-alpha-1-SNAPSHOT.jar:4.0.0-alpha-1-SNAPSHOT]
    at org.apache.hadoop.hbase.ScheduledChore.run(ScheduledChore.java:161) ~[hbase-common-4.0.0-alpha-1-SNAPSHOT.jar:4.0.0-alpha-1-SNAPSHOT]
    at org.apache.hadoop.hbase.trace.TraceUtil.lambda$tracedRunnable$2(TraceUtil.java:155) ~[hbase-common-4.0.0-alpha-1-SNAPSHOT.jar:4.0.0-alpha-1-SNAPSHOT]
    at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:539) ~[?:?]
    at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:305) ~[?:?]
    at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:305) ~[?:?]
    at org.apache.hadoop.hbase.JitterScheduledThreadPoolExecutorImpl$JitteredRunnableScheduledFuture.run(JitterScheduledThreadPoolExecutorImpl.java:107) ~[hbase-common-4.0.0-alpha-1-SNAPSHOT.jar:4.0.0-alpha-1-SNAPSHOT]
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136) ~[?:?]
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635) ~[?:?]
    at java.lang.Thread.run(Thread.java:840) ~[?:?] 
   ```


-- 
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@hbase.apache.org

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


Re: [PR] HBASE-29715: AssignmentManager is trying to pick up the active cluster's tables before refreshing meta and hfiles [hbase]

Posted by "kgeisz (via GitHub)" <gi...@apache.org>.
kgeisz commented on code in PR #7474:
URL: https://github.com/apache/hbase/pull/7474#discussion_r2608877965


##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterMetrics.java:
##########
@@ -225,4 +228,70 @@ public void testClusterMetricsMetaTableSkipping() throws Exception {
       master.getTableDescriptors().remove(replicaMetaTable);
     }
   }
+
+  @Test
+  public void testClusterMetricsForeignTableSkipping() throws Exception {
+    List<TableName> allTables = new ArrayList<>();
+
+    // These tables, including the cluster's meta table, should not be foreign to the cluster.
+    // The cluster should be able to find their state.
+    allTables.add(TableName.META_TABLE_NAME);
+    List<TableName> familiarTables = new ArrayList<>();
+    familiarTables.add(TableName.valueOf(null, "familiarTable1"));
+    familiarTables.add(TableName.valueOf("", "familiarTable2"));
+    familiarTables.add(TableName.valueOf("default", "familiarTable3"));
+    familiarTables.add(TableName.valueOf("familiarNamespace", "familiarTable4"));
+    familiarTables.add(TableName.valueOf("familiarNamespace", "familiarTable5"));
+
+    // Create these "familiar" tables so their state can be found
+    TEST_UTIL.getAdmin().createNamespace(NamespaceDescriptor.create("familiarNamespace").build());
+    for (TableName familiarTable : familiarTables) {
+      TEST_UTIL.createTable(familiarTable, "cf");
+      allTables.add(familiarTable);
+    }
+
+    // These tables should be foreign to the cluster.
+    // The cluster should not be able to find their state.
+    allTables.add(TableName.valueOf("hbase", "meta_replica"));
+    allTables.add(TableName.valueOf(null, "defaultNamespaceTable1"));
+    allTables.add(TableName.valueOf("", "defaultNamespaceTable2"));
+    allTables.add(TableName.valueOf("default", "defaultNamespaceTable3"));
+    allTables.add(TableName.valueOf("customNamespace", "customNamespaceTable1"));
+    allTables.add(TableName.valueOf("customNamespace", "customNamespaceTable2"));
+    allTables.add(TableName.valueOf("anotherNamespace", "anotherNamespaceTable"));
+    allTables.add(TableName.valueOf("sharedNamespace", "sharedNamespaceTable1"));
+    allTables.add(TableName.valueOf("sharedNamespace", "sharedNamespaceTable2"));
+
+    // Update master's table descriptors to have all tables
+    TableDescriptor foreignTableDescriptor;
+    for (TableName tableName : allTables) {
+      foreignTableDescriptor = TableDescriptorBuilder.newBuilder(tableName)
+        .setColumnFamily(ColumnFamilyDescriptorBuilder.of("cf")).build();
+      master.getTableDescriptors().update(foreignTableDescriptor, true);
+    }

Review Comment:
   This line in HMaster:
   
   ```
   Map<String, TableDescriptor> tableDescriptorMap = getTableDescriptors().getAll();
   ```
   
   was using [HBaseServerBase.getTableDescriptors()](https://github.com/apache/hbase/blob/master/hbase-server/src/main/java/org/apache/hadoop/hbase/HBaseServerBase.java#L523) rather than using [HMaster.getTableDescriptors()](https://github.com/apache/hbase/blob/master/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java#L3821).
   
   @Kota-SH Suggested I use `HMaster.listTableDescriptors()` (which uses `HMaster.getTableDescriptors()`) so the filtering is done automatically.  I have pushed that change recently.



-- 
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@hbase.apache.org

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


Re: [PR] HBASE-29715: AssignmentManager is trying to pick up the active cluster's tables before refreshing meta and hfiles [hbase]

Posted by "taklwu (via GitHub)" <gi...@apache.org>.
taklwu commented on code in PR #7474:
URL: https://github.com/apache/hbase/pull/7474#discussion_r2612097480


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java:
##########
@@ -3116,16 +3116,9 @@ public ClusterMetrics getClusterMetricsWithoutCoprocessor(EnumSet<Option> option
           if (isActiveMaster() && isInitialized() && assignmentManager != null) {
             try {
               Map<TableName, RegionStatesCount> tableRegionStatesCountMap = new HashMap<>();
-              Map<String, TableDescriptor> tableDescriptorMap = getTableDescriptors().getAll();
-              for (TableDescriptor tableDescriptor : tableDescriptorMap.values()) {
+              List<TableDescriptor> tableDescriptors = listTableDescriptors(null, null, null, true);
+              for (TableDescriptor tableDescriptor : tableDescriptors) {

Review Comment:
   you probably clean my concern that we're comparing the meta state in `tableStateManager` vs the table descriptor found on the filesystem. 
   
   just again the unit test is doing slightly different, it called `master.getTableDescriptors().update(foreignTableDescriptor, true)` to update the `cache` of the `tableDescriptors`/`FSTableDescriptors` (not writing the TableDescriptors on actual file system) with the additional tables, and then compare with the `tableStateManager` when `TABLE_TO_REGIONS_COUNT` hook is being called. if this is loaded from the `FSTableDescriptors.getAll()` before the first time is loaded from the filesystem, the test isn't 100% correct but the reading is right that the neither the `tableStateManager` of meta or `FSTableDescriptors.getAll()` from the view of filesystem does not have those unsynced table
   
   I should have used debug mode to clear the confusion that the  `FSTableDescriptors.getAll()` in the unit test are reading from cache or filesystem, thanks again



-- 
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@hbase.apache.org

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


Re: [PR] HBASE-29715: AssignmentManager is trying to pick up the active cluster's tables before refreshing meta and hfiles [hbase]

Posted by "taklwu (via GitHub)" <gi...@apache.org>.
taklwu commented on code in PR #7474:
URL: https://github.com/apache/hbase/pull/7474#discussion_r2612097480


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java:
##########
@@ -3116,16 +3116,9 @@ public ClusterMetrics getClusterMetricsWithoutCoprocessor(EnumSet<Option> option
           if (isActiveMaster() && isInitialized() && assignmentManager != null) {
             try {
               Map<TableName, RegionStatesCount> tableRegionStatesCountMap = new HashMap<>();
-              Map<String, TableDescriptor> tableDescriptorMap = getTableDescriptors().getAll();
-              for (TableDescriptor tableDescriptor : tableDescriptorMap.values()) {
+              List<TableDescriptor> tableDescriptors = listTableDescriptors(null, null, null, true);
+              for (TableDescriptor tableDescriptor : tableDescriptors) {

Review Comment:
   you probably clean my concern that we're comparing the meta state in `tableStateManager` vs the table descriptor found on the filesystem. 
   
   just again the unit test is doing slightly different, it called `master.getTableDescriptors().update(foreignTableDescriptor, true)` to update the `cache` of the `tableDescriptors`/`FSTableDescriptors` (not writing the TableDescriptors on actual file system) with the additional tables, and then compare with the `tableStateManager` when `TABLE_TO_REGIONS_COUNT` hook is being called. 
   
   since `FSTableDescriptors.getAll()` is always using the cache after the master is initialized (not reloading from the filesystem), the test is partially correct that neither the `tableStateManager` of meta or `FSTableDescriptors.getAll()` from the view of filesystem does not have those unsynced table, or `tableStateManager` does not have those additional loaded table in `cache` of `FSTableDescriptors`
   
   I should have used debug mode to clear the confusion that the  `FSTableDescriptors.getAll()` in the unit test are always from cache, thanks again.



-- 
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@hbase.apache.org

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


Re: [PR] HBASE-29715: AssignmentManager is trying to pick up the active cluster's tables before refreshing meta and hfiles [hbase]

Posted by "Kota-SH (via GitHub)" <gi...@apache.org>.
Kota-SH commented on code in PR #7474:
URL: https://github.com/apache/hbase/pull/7474#discussion_r2608492375


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java:
##########
@@ -3117,13 +3117,11 @@ public ClusterMetrics getClusterMetricsWithoutCoprocessor(EnumSet<Option> option
             try {
               Map<TableName, RegionStatesCount> tableRegionStatesCountMap = new HashMap<>();
               Map<String, TableDescriptor> tableDescriptorMap = getTableDescriptors().getAll();

Review Comment:
   Can we do a single call to HMaster.listTableDescriptors() instead of calculating the tablenames twice? 
   It has internal filter to check if the tablename is present, so we wouldn't need to manually filter again.
   
   `List<TableDescriptor> tableDescriptors = listTableDescriptors(null, null, null, true);`



-- 
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@hbase.apache.org

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


Re: [PR] HBASE-29715: AssignmentManager is trying to pick up the active cluster's tables before refreshing meta and hfiles [hbase]

Posted by "taklwu (via GitHub)" <gi...@apache.org>.
taklwu commented on code in PR #7474:
URL: https://github.com/apache/hbase/pull/7474#discussion_r2609132851


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java:
##########
@@ -3116,16 +3116,9 @@ public ClusterMetrics getClusterMetricsWithoutCoprocessor(EnumSet<Option> option
           if (isActiveMaster() && isInitialized() && assignmentManager != null) {
             try {
               Map<TableName, RegionStatesCount> tableRegionStatesCountMap = new HashMap<>();
-              Map<String, TableDescriptor> tableDescriptorMap = getTableDescriptors().getAll();
-              for (TableDescriptor tableDescriptor : tableDescriptorMap.values()) {
+              List<TableDescriptor> tableDescriptors = listTableDescriptors(null, null, null, true);
+              for (TableDescriptor tableDescriptor : tableDescriptors) {

Review Comment:
   can we have manual test for this change? I'm wondered if this would filter any table (non-metatable that filter by `FSUtils.java#isLocalMetaTable`) that should be considered as foreign to the primary/writer cluster.
   
   `listTableDescriptors` should be loading all the tables from the filesystem via `FSTableDescriptors.java#getAll()`, where this change does not have a way to filter the user/foreign table that should not be unsynced. (should the chore be always refreshing from filesystem when the meta sync was not triggered on the read-only replica?) 
   
   or are we just trying to remove the redundant logic of rechecking the foreign meta table ? 
   
   but anyhow, the unit test flow does not match the title and please try to come up a manual test or change the unit test to act as the primary and the read-only replica. 



-- 
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@hbase.apache.org

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


Re: [PR] HBASE-29715: AssignmentManager is trying to pick up the active cluster's tables before refreshing meta and hfiles [hbase]

Posted by "kgeisz (via GitHub)" <gi...@apache.org>.
kgeisz commented on code in PR #7474:
URL: https://github.com/apache/hbase/pull/7474#discussion_r2608845245


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java:
##########
@@ -3117,13 +3117,11 @@ public ClusterMetrics getClusterMetricsWithoutCoprocessor(EnumSet<Option> option
             try {
               Map<TableName, RegionStatesCount> tableRegionStatesCountMap = new HashMap<>();
               Map<String, TableDescriptor> tableDescriptorMap = getTableDescriptors().getAll();

Review Comment:
   I have made changes based on your suggestion.  Let me know if it's what you had in mind.



-- 
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@hbase.apache.org

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


Re: [PR] HBASE-29715: AssignmentManager is trying to pick up the active cluster's tables before refreshing meta and hfiles [hbase]

Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar commented on code in PR #7474:
URL: https://github.com/apache/hbase/pull/7474#discussion_r2612568627


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java:
##########
@@ -3116,16 +3116,9 @@ public ClusterMetrics getClusterMetricsWithoutCoprocessor(EnumSet<Option> option
           if (isActiveMaster() && isInitialized() && assignmentManager != null) {
             try {
               Map<TableName, RegionStatesCount> tableRegionStatesCountMap = new HashMap<>();
-              Map<String, TableDescriptor> tableDescriptorMap = getTableDescriptors().getAll();
-              for (TableDescriptor tableDescriptor : tableDescriptorMap.values()) {
+              List<TableDescriptor> tableDescriptors = listTableDescriptors(null, null, null, true);
+              for (TableDescriptor tableDescriptor : tableDescriptors) {

Review Comment:
   > I should have used debug mode to clear the confusion that the FSTableDescriptors.getAll() in the unit test are always from cache, thanks again.
   
   Sounds like a reasonable improvement on the testing side that we should do in this patch @kgeisz .



-- 
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@hbase.apache.org

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


Re: [PR] HBASE-29715: AssignmentManager is trying to pick up the active cluster's tables before refreshing meta and hfiles [hbase]

Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar commented on code in PR #7474:
URL: https://github.com/apache/hbase/pull/7474#discussion_r2611855209


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java:
##########
@@ -3116,16 +3116,9 @@ public ClusterMetrics getClusterMetricsWithoutCoprocessor(EnumSet<Option> option
           if (isActiveMaster() && isInitialized() && assignmentManager != null) {
             try {
               Map<TableName, RegionStatesCount> tableRegionStatesCountMap = new HashMap<>();
-              Map<String, TableDescriptor> tableDescriptorMap = getTableDescriptors().getAll();
-              for (TableDescriptor tableDescriptor : tableDescriptorMap.values()) {
+              List<TableDescriptor> tableDescriptors = listTableDescriptors(null, null, null, true);
+              for (TableDescriptor tableDescriptor : tableDescriptors) {

Review Comment:
   To my understanding this logic has been changed to work only with tables which are present in replica cluster's meta table. 
   
   Previously it used `HBaseServerBase.getTableDescriptors().getAll()` which eventually calls `FSTableDescriptors.getAll()` method which loads all table from the filesystem. (only once, because it caches the result)
   
   Now, in this patch, we call `HMaster.listTableDescriptors()` which ends up calling the same `FSTableDescriptors.getAll()`, but before returning the list it, filters for the tablenames which are present in replica cluster's meta table.
   
   ```java
         for (TableDescriptor desc : allHtds) {
           if (
             tableStateManager.isTablePresent(desc.getTableName())
               && (includeSysTables || !desc.getTableName().isSystemTable())
           ) {
             htds.add(desc);
           }
         }
   ```
   
   I wonder why don't we just get the list directly from meta, but either way the patch looks good to me.



-- 
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@hbase.apache.org

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


Re: [PR] HBASE-29715: AssignmentManager is trying to pick up the active cluster's tables before refreshing meta and hfiles [hbase]

Posted by "Kota-SH (via GitHub)" <gi...@apache.org>.
Kota-SH commented on code in PR #7474:
URL: https://github.com/apache/hbase/pull/7474#discussion_r2609054845


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java:
##########
@@ -3117,13 +3117,11 @@ public ClusterMetrics getClusterMetricsWithoutCoprocessor(EnumSet<Option> option
             try {
               Map<TableName, RegionStatesCount> tableRegionStatesCountMap = new HashMap<>();
               Map<String, TableDescriptor> tableDescriptorMap = getTableDescriptors().getAll();

Review Comment:
   lgtm @kgeisz, thanks. 



-- 
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@hbase.apache.org

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


Re: [PR] HBASE-29715: AssignmentManager is trying to pick up the active cluster's tables before refreshing meta and hfiles [hbase]

Posted by "taklwu (via GitHub)" <gi...@apache.org>.
taklwu commented on code in PR #7474:
URL: https://github.com/apache/hbase/pull/7474#discussion_r2608778749


##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterMetrics.java:
##########
@@ -225,4 +228,70 @@ public void testClusterMetricsMetaTableSkipping() throws Exception {
       master.getTableDescriptors().remove(replicaMetaTable);
     }
   }
+
+  @Test
+  public void testClusterMetricsForeignTableSkipping() throws Exception {
+    List<TableName> allTables = new ArrayList<>();
+
+    // These tables, including the cluster's meta table, should not be foreign to the cluster.
+    // The cluster should be able to find their state.
+    allTables.add(TableName.META_TABLE_NAME);
+    List<TableName> familiarTables = new ArrayList<>();
+    familiarTables.add(TableName.valueOf(null, "familiarTable1"));
+    familiarTables.add(TableName.valueOf("", "familiarTable2"));
+    familiarTables.add(TableName.valueOf("default", "familiarTable3"));
+    familiarTables.add(TableName.valueOf("familiarNamespace", "familiarTable4"));
+    familiarTables.add(TableName.valueOf("familiarNamespace", "familiarTable5"));
+
+    // Create these "familiar" tables so their state can be found
+    TEST_UTIL.getAdmin().createNamespace(NamespaceDescriptor.create("familiarNamespace").build());
+    for (TableName familiarTable : familiarTables) {
+      TEST_UTIL.createTable(familiarTable, "cf");
+      allTables.add(familiarTable);
+    }
+
+    // These tables should be foreign to the cluster.
+    // The cluster should not be able to find their state.
+    allTables.add(TableName.valueOf("hbase", "meta_replica"));
+    allTables.add(TableName.valueOf(null, "defaultNamespaceTable1"));
+    allTables.add(TableName.valueOf("", "defaultNamespaceTable2"));
+    allTables.add(TableName.valueOf("default", "defaultNamespaceTable3"));
+    allTables.add(TableName.valueOf("customNamespace", "customNamespaceTable1"));
+    allTables.add(TableName.valueOf("customNamespace", "customNamespaceTable2"));
+    allTables.add(TableName.valueOf("anotherNamespace", "anotherNamespaceTable"));
+    allTables.add(TableName.valueOf("sharedNamespace", "sharedNamespaceTable1"));
+    allTables.add(TableName.valueOf("sharedNamespace", "sharedNamespaceTable2"));
+
+    // Update master's table descriptors to have all tables
+    TableDescriptor foreignTableDescriptor;
+    for (TableName tableName : allTables) {
+      foreignTableDescriptor = TableDescriptorBuilder.newBuilder(tableName)
+        .setColumnFamily(ColumnFamilyDescriptorBuilder.of("cf")).build();
+      master.getTableDescriptors().update(foreignTableDescriptor, true);
+    }

Review Comment:
   nit: I have a question about this `TABLE_TO_REGIONS_COUNT` and `master.getTableDescriptors().update(foreignTableDescriptor, true);
       }` in this test.
       
   in the read replica cluster, if the newly added table is not synced by meta and hfiles operation (not the meta table only), how does load into the `tableDescriptors` ?  the way you have here is implying the HMaster of read replica cluster is using `master.getTableDescriptors().update(foreignTableDescriptor, true);` or somehow read from the filesystem directly。
    
    once those table not in the meta of replica cluster loaded and if `tableStateManager` (the current meta of the replica cluster) does not have this state of this foreignable table , the change in this PR would work.  



-- 
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@hbase.apache.org

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


Re: [PR] HBASE-29715: AssignmentManager is trying to pick up the active cluster's tables before refreshing meta and hfiles [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7474:
URL: https://github.com/apache/hbase/pull/7474#issuecomment-3639813523

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 34s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  codespell  |   0m  0s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  0s |  |  detect-secrets was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  |  Patch does not have any anti-patterns.  |
   |||| _ HBASE-29081 Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 39s |  |  HBASE-29081 passed  |
   | +1 :green_heart: |  compile  |   3m 38s |  |  HBASE-29081 passed  |
   | -0 :warning: |  checkstyle  |   0m 15s | [/buildtool-branch-checkstyle-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7474/2/artifact/yetus-general-check/output/buildtool-branch-checkstyle-hbase-server.txt) |  The patch fails to run checkstyle in hbase-server  |
   | +1 :green_heart: |  spotbugs  |   1m 40s |  |  HBASE-29081 passed  |
   | +1 :green_heart: |  spotless  |   0m 52s |  |  branch has no errors when running spotless:check.  |
   |||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 16s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   3m 31s |  |  the patch passed  |
   | +1 :green_heart: |  javac  |   3m 31s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks issues.  |
   | -0 :warning: |  checkstyle  |   0m 13s | [/buildtool-patch-checkstyle-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7474/2/artifact/yetus-general-check/output/buildtool-patch-checkstyle-hbase-server.txt) |  The patch fails to run checkstyle in hbase-server  |
   | +1 :green_heart: |  spotbugs  |   1m 42s |  |  the patch passed  |
   | +1 :green_heart: |  hadoopcheck  |  12m 22s |  |  Patch does not cause any errors with Hadoop 3.3.6 3.4.0.  |
   | -1 :x: |  spotless  |   0m 39s |  |  patch has 26 errors when running spotless:check, run spotless:apply to fix.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 12s |  |  The patch does not generate ASF License warnings.  |
   |  |   |  40m 17s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7474/2/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/7474 |
   | Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless |
   | uname | Linux 8e872dc59110 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | HBASE-29081 / 35e5cbac228b0842d46ec8436bee41d7f100f34e |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   | spotless | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7474/2/artifact/yetus-general-check/output/patch-spotless.txt |
   | Max. process+thread count | 85 (vs. ulimit of 30000) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7474/2/console |
   | versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 |
   | Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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@hbase.apache.org

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


Re: [PR] HBASE-29715: AssignmentManager is trying to pick up the active cluster's tables before refreshing meta and hfiles [hbase]

Posted by "taklwu (via GitHub)" <gi...@apache.org>.
taklwu commented on code in PR #7474:
URL: https://github.com/apache/hbase/pull/7474#discussion_r2609132851


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java:
##########
@@ -3116,16 +3116,9 @@ public ClusterMetrics getClusterMetricsWithoutCoprocessor(EnumSet<Option> option
           if (isActiveMaster() && isInitialized() && assignmentManager != null) {
             try {
               Map<TableName, RegionStatesCount> tableRegionStatesCountMap = new HashMap<>();
-              Map<String, TableDescriptor> tableDescriptorMap = getTableDescriptors().getAll();
-              for (TableDescriptor tableDescriptor : tableDescriptorMap.values()) {
+              List<TableDescriptor> tableDescriptors = listTableDescriptors(null, null, null, true);
+              for (TableDescriptor tableDescriptor : tableDescriptors) {

Review Comment:
   can we have manual test for this change? I'm wondered if this would filter any table (non-metatable that filter by `FSUtils.java#isLocalMetaTable`) that should be considered as foreign to the primary/writer cluster.
   
   `listTableDescriptors` should be loading all the tables from the filesystem via `FSTableDescriptors.java#getAll()`, where this change does not have a way to filter the user/foreign table that should not be synced. (should the chore be always refreshing from filesystem when the meta sync was not triggered on the read-only replica?) 
   
   or are we just trying to remove the redundant logic of rechecking the foreign meta table ? 
   
   but anyhow, the unit test flow does not match the title and please try to come up a manual test or change the unit test to act as the primary and the read-only replica. 



-- 
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@hbase.apache.org

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


Re: [PR] HBASE-29715: AssignmentManager is trying to pick up the active cluster's tables before refreshing meta and hfiles [hbase]

Posted by "kgeisz (via GitHub)" <gi...@apache.org>.
kgeisz commented on PR #7474:
URL: https://github.com/apache/hbase/pull/7474#issuecomment-3555642538

   I noticed PR #7304 also made changes to `FSTableDescriptors.java` and `HbckChore.java` via [FSUtils.isLocalMetaTable()](https://github.com/apache/hbase/pull/7304/files#diff-240b798d795405da1571cf446135d43346f17c09849773fd2508e8d1b6d1dd4eR1056).  Currently, this PR does not make any changes to those files.


-- 
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@hbase.apache.org

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


Re: [PR] HBASE-29715: AssignmentManager is trying to pick up the active cluster's tables before refreshing meta and hfiles [hbase]

Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar commented on code in PR #7474:
URL: https://github.com/apache/hbase/pull/7474#discussion_r2611893097


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java:
##########
@@ -3116,16 +3116,9 @@ public ClusterMetrics getClusterMetricsWithoutCoprocessor(EnumSet<Option> option
           if (isActiveMaster() && isInitialized() && assignmentManager != null) {
             try {
               Map<TableName, RegionStatesCount> tableRegionStatesCountMap = new HashMap<>();
-              Map<String, TableDescriptor> tableDescriptorMap = getTableDescriptors().getAll();
-              for (TableDescriptor tableDescriptor : tableDescriptorMap.values()) {
+              List<TableDescriptor> tableDescriptors = listTableDescriptors(null, null, null, true);
+              for (TableDescriptor tableDescriptor : tableDescriptors) {

Review Comment:
   > I wonder why don't we just get the list directly from meta, but either way the patch looks good to me.
   
   It would be a full scan on the meta table VS currently we do a GET on meta for every table that we find on the filesystem.
   
   I'm not sure which one is more efficient.



-- 
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@hbase.apache.org

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


Re: [PR] HBASE-29715: AssignmentManager is trying to pick up the active cluster's tables before refreshing meta and hfiles [hbase]

Posted by "taklwu (via GitHub)" <gi...@apache.org>.
taklwu commented on code in PR #7474:
URL: https://github.com/apache/hbase/pull/7474#discussion_r2612097480


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java:
##########
@@ -3116,16 +3116,9 @@ public ClusterMetrics getClusterMetricsWithoutCoprocessor(EnumSet<Option> option
           if (isActiveMaster() && isInitialized() && assignmentManager != null) {
             try {
               Map<TableName, RegionStatesCount> tableRegionStatesCountMap = new HashMap<>();
-              Map<String, TableDescriptor> tableDescriptorMap = getTableDescriptors().getAll();
-              for (TableDescriptor tableDescriptor : tableDescriptorMap.values()) {
+              List<TableDescriptor> tableDescriptors = listTableDescriptors(null, null, null, true);
+              for (TableDescriptor tableDescriptor : tableDescriptors) {

Review Comment:
   you probably clean my concern that we're comparing the meta state in `tableStateManager` vs the table descriptor found on the filesystem. 
   
   just again the unit test is doing slightly different, it called `master.getTableDescriptors().update(foreignTableDescriptor, true)` to update the `cache` of the `tableDescriptors`/`FSTableDescriptors` (not the TableDescriptors on actual file system) with the additional tables, and then compare with the `tableStateManager` when `TABLE_TO_REGIONS_COUNT` hook is being called. I should have used debug mode to clear the confusion that the  `FSTableDescriptors.getAll()` in the unit test are reading from cache or filesystem.



-- 
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@hbase.apache.org

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


Re: [PR] HBASE-29715: AssignmentManager is trying to pick up the active cluster's tables before refreshing meta and hfiles [hbase]

Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar commented on PR #7474:
URL: https://github.com/apache/hbase/pull/7474#issuecomment-3634413122

   @Kota-SH @kgeisz Are the unit test failures related? Can we move on with merging this PR?


-- 
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@hbase.apache.org

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


Re: [PR] HBASE-29715: AssignmentManager is trying to pick up the active cluster's tables before refreshing meta and hfiles [hbase]

Posted by "taklwu (via GitHub)" <gi...@apache.org>.
taklwu commented on code in PR #7474:
URL: https://github.com/apache/hbase/pull/7474#discussion_r2609132851


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java:
##########
@@ -3116,16 +3116,9 @@ public ClusterMetrics getClusterMetricsWithoutCoprocessor(EnumSet<Option> option
           if (isActiveMaster() && isInitialized() && assignmentManager != null) {
             try {
               Map<TableName, RegionStatesCount> tableRegionStatesCountMap = new HashMap<>();
-              Map<String, TableDescriptor> tableDescriptorMap = getTableDescriptors().getAll();
-              for (TableDescriptor tableDescriptor : tableDescriptorMap.values()) {
+              List<TableDescriptor> tableDescriptors = listTableDescriptors(null, null, null, true);
+              for (TableDescriptor tableDescriptor : tableDescriptors) {

Review Comment:
   can we have manual test for this change? I'm wondered if this would filter any table (non-metatable that filter by `FSUtils.java#isLocalMetaTable`) that should be considered as foreign to the primary/writer cluster.
   
   `listTableDescriptors` should be loading all the tables from the filesystem via `FSTableDescriptors.java#getAll()`, where this change does not have a way to filter the user/foreign table that should not be synced. (should the chore be always refreshing from filesystem when the meta sync does not trigger from the read-only replica?) 
   
   or are we just trying to remove the redundant logic of rechecking the foreign meta table ? 
   
   but anyhow, the unit test flow does not match the title and please try to come up a manual test or change the unit test to act as the primary and the read-only replica. 



-- 
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@hbase.apache.org

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


Re: [PR] HBASE-29715: AssignmentManager is trying to pick up the active cluster's tables before refreshing meta and hfiles [hbase]

Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar commented on code in PR #7474:
URL: https://github.com/apache/hbase/pull/7474#discussion_r2611855209


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java:
##########
@@ -3116,16 +3116,9 @@ public ClusterMetrics getClusterMetricsWithoutCoprocessor(EnumSet<Option> option
           if (isActiveMaster() && isInitialized() && assignmentManager != null) {
             try {
               Map<TableName, RegionStatesCount> tableRegionStatesCountMap = new HashMap<>();
-              Map<String, TableDescriptor> tableDescriptorMap = getTableDescriptors().getAll();
-              for (TableDescriptor tableDescriptor : tableDescriptorMap.values()) {
+              List<TableDescriptor> tableDescriptors = listTableDescriptors(null, null, null, true);
+              for (TableDescriptor tableDescriptor : tableDescriptors) {

Review Comment:
   To my understanding this logic has been changed to work only with tables which are present in current cluster's meta table. 
   
   Previously it used `HBaseServerBase.getTableDescriptors().getAll()` which eventually calls `FSTableDescriptors.getAll()` method which loads all table from the filesystem. (only once, because it caches the result)
   
   Now, in this patch, we call `HMaster.listTableDescriptors()` which ends up calling the same `FSTableDescriptors.getAll()`, but before returning the list it, filters for the tablenames which are present in current cluster's meta table.
   
   ```java
         for (TableDescriptor desc : allHtds) {
           if (
             tableStateManager.isTablePresent(desc.getTableName())
               && (includeSysTables || !desc.getTableName().isSystemTable())
           ) {
             htds.add(desc);
           }
         }
   ```
   
   I wonder why don't we just get the list directly from meta, but either way the patch looks good to me.



-- 
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@hbase.apache.org

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


Re: [PR] HBASE-29715: AssignmentManager is trying to pick up the active cluster's tables before refreshing meta and hfiles [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7474:
URL: https://github.com/apache/hbase/pull/7474#issuecomment-3555700013

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 35s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  codespell  |   0m  0s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  0s |  |  detect-secrets was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  |  Patch does not have any anti-patterns.  |
   |||| _ HBASE-29081 Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 58s |  |  HBASE-29081 passed  |
   | +1 :green_heart: |  compile  |   3m 41s |  |  HBASE-29081 passed  |
   | -0 :warning: |  checkstyle  |   0m 18s | [/buildtool-branch-checkstyle-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7474/1/artifact/yetus-general-check/output/buildtool-branch-checkstyle-hbase-server.txt) |  The patch fails to run checkstyle in hbase-server  |
   | +1 :green_heart: |  spotbugs  |   1m 46s |  |  HBASE-29081 passed  |
   | +1 :green_heart: |  spotless  |   0m 55s |  |  branch has no errors when running spotless:check.  |
   |||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 27s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   3m 37s |  |  the patch passed  |
   | +1 :green_heart: |  javac  |   3m 37s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks issues.  |
   | -0 :warning: |  checkstyle  |   0m 15s | [/buildtool-patch-checkstyle-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7474/1/artifact/yetus-general-check/output/buildtool-patch-checkstyle-hbase-server.txt) |  The patch fails to run checkstyle in hbase-server  |
   | +1 :green_heart: |  spotbugs  |   1m 51s |  |  the patch passed  |
   | +1 :green_heart: |  hadoopcheck  |  12m 43s |  |  Patch does not cause any errors with Hadoop 3.3.6 3.4.0.  |
   | +1 :green_heart: |  spotless  |   0m 50s |  |  patch has no errors when running spotless:check.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 12s |  |  The patch does not generate ASF License warnings.  |
   |  |   |  42m  1s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7474/1/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/7474 |
   | Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless |
   | uname | Linux 40eab400b95b 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | HBASE-29081 / 7c9c85a93cb07b487050ce0bca2eba67a58d6bd9 |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   | Max. process+thread count | 83 (vs. ulimit of 30000) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7474/1/console |
   | versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 |
   | Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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@hbase.apache.org

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


Re: [PR] HBASE-29715: AssignmentManager is trying to pick up the active cluster's tables before refreshing meta and hfiles [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7474:
URL: https://github.com/apache/hbase/pull/7474#issuecomment-3640407970

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   1m 22s |  |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck  |
   |||| _ Prechecks _ |
   |||| _ HBASE-29081 Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   2m 47s |  |  HBASE-29081 passed  |
   | +1 :green_heart: |  compile  |   0m 43s |  |  HBASE-29081 passed  |
   | +1 :green_heart: |  javadoc  |   0m 23s |  |  HBASE-29081 passed  |
   | +1 :green_heart: |  shadedjars  |   4m 29s |  |  branch has no errors when building our shaded downstream artifacts.  |
   |||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   2m 15s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 44s |  |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 44s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   0m 20s |  |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   4m 26s |  |  patch has no errors when building our shaded downstream artifacts.  |
   |||| _ Other Tests _ |
   | -1 :x: |  unit  | 229m 47s | [/patch-unit-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7474/2/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-server.txt) |  hbase-server in the patch failed.  |
   |  |   | 250m 58s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.48 ServerAPI=1.48 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7474/2/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/7474 |
   | Optional Tests | javac javadoc unit compile shadedjars |
   | uname | Linux fee7bb96f70d 6.8.0-1024-aws #26~22.04.1-Ubuntu SMP Wed Feb 19 06:54:57 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | HBASE-29081 / 35e5cbac228b0842d46ec8436bee41d7f100f34e |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   |  Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7474/2/testReport/ |
   | Max. process+thread count | 6150 (vs. ulimit of 30000) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7474/2/console |
   | versions | git=2.34.1 maven=3.9.8 |
   | Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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@hbase.apache.org

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


Re: [PR] HBASE-29715: AssignmentManager is trying to pick up the active cluster's tables before refreshing meta and hfiles [hbase]

Posted by "taklwu (via GitHub)" <gi...@apache.org>.
taklwu commented on code in PR #7474:
URL: https://github.com/apache/hbase/pull/7474#discussion_r2612162163


##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterMetrics.java:
##########
@@ -225,4 +228,70 @@ public void testClusterMetricsMetaTableSkipping() throws Exception {
       master.getTableDescriptors().remove(replicaMetaTable);
     }
   }
+
+  @Test
+  public void testClusterMetricsForeignTableSkipping() throws Exception {
+    List<TableName> allTables = new ArrayList<>();
+
+    // These tables, including the cluster's meta table, should not be foreign to the cluster.
+    // The cluster should be able to find their state.
+    allTables.add(TableName.META_TABLE_NAME);
+    List<TableName> familiarTables = new ArrayList<>();
+    familiarTables.add(TableName.valueOf(null, "familiarTable1"));
+    familiarTables.add(TableName.valueOf("", "familiarTable2"));
+    familiarTables.add(TableName.valueOf("default", "familiarTable3"));
+    familiarTables.add(TableName.valueOf("familiarNamespace", "familiarTable4"));
+    familiarTables.add(TableName.valueOf("familiarNamespace", "familiarTable5"));
+
+    // Create these "familiar" tables so their state can be found
+    TEST_UTIL.getAdmin().createNamespace(NamespaceDescriptor.create("familiarNamespace").build());
+    for (TableName familiarTable : familiarTables) {
+      TEST_UTIL.createTable(familiarTable, "cf");
+      allTables.add(familiarTable);
+    }
+
+    // These tables should be foreign to the cluster.
+    // The cluster should not be able to find their state.
+    allTables.add(TableName.valueOf("hbase", "meta_replica"));
+    allTables.add(TableName.valueOf(null, "defaultNamespaceTable1"));
+    allTables.add(TableName.valueOf("", "defaultNamespaceTable2"));
+    allTables.add(TableName.valueOf("default", "defaultNamespaceTable3"));
+    allTables.add(TableName.valueOf("customNamespace", "customNamespaceTable1"));
+    allTables.add(TableName.valueOf("customNamespace", "customNamespaceTable2"));
+    allTables.add(TableName.valueOf("anotherNamespace", "anotherNamespaceTable"));
+    allTables.add(TableName.valueOf("sharedNamespace", "sharedNamespaceTable1"));
+    allTables.add(TableName.valueOf("sharedNamespace", "sharedNamespaceTable2"));
+
+    // Update master's table descriptors to have all tables
+    TableDescriptor foreignTableDescriptor;
+    for (TableName tableName : allTables) {
+      foreignTableDescriptor = TableDescriptorBuilder.newBuilder(tableName)
+        .setColumnFamily(ColumnFamilyDescriptorBuilder.of("cf")).build();
+      master.getTableDescriptors().update(foreignTableDescriptor, true);
+    }

Review Comment:
   so, I use debug mode to check the unit test, I found that `listTableDescriptors` is always using the `cache` of `FSTableDescriptors` instead of loading from the filesystem, such that we're relying on the `tableStateManager` via meta to see if the additional loaded tables should be included. 
   
   
   the test itself is right, just I'm wondered practically how newly added table on the primary/write cluster being added to the `cache` of `FSTableDescriptors` after master has been initialized ? or does it add from the filesystem after restarting the master (meta is yet synced)? 



-- 
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@hbase.apache.org

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


Re: [PR] HBASE-29715: AssignmentManager is trying to pick up the active cluster's tables before refreshing meta and hfiles [hbase]

Posted by "taklwu (via GitHub)" <gi...@apache.org>.
taklwu commented on PR #7474:
URL: https://github.com/apache/hbase/pull/7474#issuecomment-3656730159

   @kgeisz please create a follow up to add a test for fresh replica setup that loads from filesystem (instead of the currently one that is loaded from cache.)


-- 
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@hbase.apache.org

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


Re: [PR] HBASE-29715: AssignmentManager is trying to pick up the active cluster's tables before refreshing meta and hfiles [hbase]

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


-- 
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@hbase.apache.org

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


Re: [PR] HBASE-29715: AssignmentManager is trying to pick up the active cluster's tables before refreshing meta and hfiles [hbase]

Posted by "kgeisz (via GitHub)" <gi...@apache.org>.
kgeisz commented on code in PR #7474:
URL: https://github.com/apache/hbase/pull/7474#discussion_r2608770585


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java:
##########
@@ -3117,13 +3117,11 @@ public ClusterMetrics getClusterMetricsWithoutCoprocessor(EnumSet<Option> option
             try {
               Map<TableName, RegionStatesCount> tableRegionStatesCountMap = new HashMap<>();
               Map<String, TableDescriptor> tableDescriptorMap = getTableDescriptors().getAll();

Review Comment:
   Nice find! This is much cleaner.



-- 
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@hbase.apache.org

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


Re: [PR] HBASE-29715: AssignmentManager is trying to pick up the active cluster's tables before refreshing meta and hfiles [hbase]

Posted by "kgeisz (via GitHub)" <gi...@apache.org>.
kgeisz commented on code in PR #7474:
URL: https://github.com/apache/hbase/pull/7474#discussion_r2608877965


##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterMetrics.java:
##########
@@ -225,4 +228,70 @@ public void testClusterMetricsMetaTableSkipping() throws Exception {
       master.getTableDescriptors().remove(replicaMetaTable);
     }
   }
+
+  @Test
+  public void testClusterMetricsForeignTableSkipping() throws Exception {
+    List<TableName> allTables = new ArrayList<>();
+
+    // These tables, including the cluster's meta table, should not be foreign to the cluster.
+    // The cluster should be able to find their state.
+    allTables.add(TableName.META_TABLE_NAME);
+    List<TableName> familiarTables = new ArrayList<>();
+    familiarTables.add(TableName.valueOf(null, "familiarTable1"));
+    familiarTables.add(TableName.valueOf("", "familiarTable2"));
+    familiarTables.add(TableName.valueOf("default", "familiarTable3"));
+    familiarTables.add(TableName.valueOf("familiarNamespace", "familiarTable4"));
+    familiarTables.add(TableName.valueOf("familiarNamespace", "familiarTable5"));
+
+    // Create these "familiar" tables so their state can be found
+    TEST_UTIL.getAdmin().createNamespace(NamespaceDescriptor.create("familiarNamespace").build());
+    for (TableName familiarTable : familiarTables) {
+      TEST_UTIL.createTable(familiarTable, "cf");
+      allTables.add(familiarTable);
+    }
+
+    // These tables should be foreign to the cluster.
+    // The cluster should not be able to find their state.
+    allTables.add(TableName.valueOf("hbase", "meta_replica"));
+    allTables.add(TableName.valueOf(null, "defaultNamespaceTable1"));
+    allTables.add(TableName.valueOf("", "defaultNamespaceTable2"));
+    allTables.add(TableName.valueOf("default", "defaultNamespaceTable3"));
+    allTables.add(TableName.valueOf("customNamespace", "customNamespaceTable1"));
+    allTables.add(TableName.valueOf("customNamespace", "customNamespaceTable2"));
+    allTables.add(TableName.valueOf("anotherNamespace", "anotherNamespaceTable"));
+    allTables.add(TableName.valueOf("sharedNamespace", "sharedNamespaceTable1"));
+    allTables.add(TableName.valueOf("sharedNamespace", "sharedNamespaceTable2"));
+
+    // Update master's table descriptors to have all tables
+    TableDescriptor foreignTableDescriptor;
+    for (TableName tableName : allTables) {
+      foreignTableDescriptor = TableDescriptorBuilder.newBuilder(tableName)
+        .setColumnFamily(ColumnFamilyDescriptorBuilder.of("cf")).build();
+      master.getTableDescriptors().update(foreignTableDescriptor, true);
+    }

Review Comment:
   This line in HMaster:
   
   ```
   Map<String, TableDescriptor> tableDescriptorMap = getTableDescriptors().getAll();
   ```
   
   was using [HBaseServerBase.getTableDescriptors()](https://github.com/apache/hbase/blob/master/hbase-server/src/main/java/org/apache/hadoop/hbase/HBaseServerBase.java#L523) rather than using [HMaster.getTableDescriptors()](https://github.com/apache/hbase/blob/master/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java#L3821).
   
   @Kota-SH Suggested I use `HMaster. listTableDescriptors()` (which uses `HMaster.getTableDescriptors()`) so the filtering is done automatically.  I have pushed that change recently.



-- 
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@hbase.apache.org

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


Re: [PR] HBASE-29715: AssignmentManager is trying to pick up the active cluster's tables before refreshing meta and hfiles [hbase]

Posted by "kgeisz (via GitHub)" <gi...@apache.org>.
kgeisz commented on PR #7474:
URL: https://github.com/apache/hbase/pull/7474#issuecomment-3639706878

   @anmolnar The failed unit tests are passing for me as well when I run them locally.


-- 
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@hbase.apache.org

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


Re: [PR] HBASE-29715: AssignmentManager is trying to pick up the active cluster's tables before refreshing meta and hfiles [hbase]

Posted by "kgeisz (via GitHub)" <gi...@apache.org>.
kgeisz commented on code in PR #7474:
URL: https://github.com/apache/hbase/pull/7474#discussion_r2608864186


##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterMetrics.java:
##########
@@ -225,4 +228,70 @@ public void testClusterMetricsMetaTableSkipping() throws Exception {
       master.getTableDescriptors().remove(replicaMetaTable);
     }
   }
+
+  @Test
+  public void testClusterMetricsForeignTableSkipping() throws Exception {
+    List<TableName> allTables = new ArrayList<>();
+
+    // These tables, including the cluster's meta table, should not be foreign to the cluster.
+    // The cluster should be able to find their state.
+    allTables.add(TableName.META_TABLE_NAME);
+    List<TableName> familiarTables = new ArrayList<>();
+    familiarTables.add(TableName.valueOf(null, "familiarTable1"));
+    familiarTables.add(TableName.valueOf("", "familiarTable2"));
+    familiarTables.add(TableName.valueOf("default", "familiarTable3"));
+    familiarTables.add(TableName.valueOf("familiarNamespace", "familiarTable4"));
+    familiarTables.add(TableName.valueOf("familiarNamespace", "familiarTable5"));
+
+    // Create these "familiar" tables so their state can be found
+    TEST_UTIL.getAdmin().createNamespace(NamespaceDescriptor.create("familiarNamespace").build());
+    for (TableName familiarTable : familiarTables) {
+      TEST_UTIL.createTable(familiarTable, "cf");
+      allTables.add(familiarTable);
+    }
+
+    // These tables should be foreign to the cluster.
+    // The cluster should not be able to find their state.
+    allTables.add(TableName.valueOf("hbase", "meta_replica"));
+    allTables.add(TableName.valueOf(null, "defaultNamespaceTable1"));
+    allTables.add(TableName.valueOf("", "defaultNamespaceTable2"));
+    allTables.add(TableName.valueOf("default", "defaultNamespaceTable3"));
+    allTables.add(TableName.valueOf("customNamespace", "customNamespaceTable1"));
+    allTables.add(TableName.valueOf("customNamespace", "customNamespaceTable2"));
+    allTables.add(TableName.valueOf("anotherNamespace", "anotherNamespaceTable"));
+    allTables.add(TableName.valueOf("sharedNamespace", "sharedNamespaceTable1"));
+    allTables.add(TableName.valueOf("sharedNamespace", "sharedNamespaceTable2"));
+
+    // Update master's table descriptors to have all tables
+    TableDescriptor foreignTableDescriptor;
+    for (TableName tableName : allTables) {
+      foreignTableDescriptor = TableDescriptorBuilder.newBuilder(tableName)
+        .setColumnFamily(ColumnFamilyDescriptorBuilder.of("cf")).build();
+      master.getTableDescriptors().update(foreignTableDescriptor, true);
+    }

Review Comment:
   The table descriptors were coming from the filesystem.  The replica cluster was seeing these, and if its meta had not been refreshed, then any foreign table descriptors could not have their state found.



-- 
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@hbase.apache.org

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


Re: [PR] HBASE-29715: AssignmentManager is trying to pick up the active cluster's tables before refreshing meta and hfiles [hbase]

Posted by "taklwu (via GitHub)" <gi...@apache.org>.
taklwu commented on code in PR #7474:
URL: https://github.com/apache/hbase/pull/7474#discussion_r2612162163


##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterMetrics.java:
##########
@@ -225,4 +228,70 @@ public void testClusterMetricsMetaTableSkipping() throws Exception {
       master.getTableDescriptors().remove(replicaMetaTable);
     }
   }
+
+  @Test
+  public void testClusterMetricsForeignTableSkipping() throws Exception {
+    List<TableName> allTables = new ArrayList<>();
+
+    // These tables, including the cluster's meta table, should not be foreign to the cluster.
+    // The cluster should be able to find their state.
+    allTables.add(TableName.META_TABLE_NAME);
+    List<TableName> familiarTables = new ArrayList<>();
+    familiarTables.add(TableName.valueOf(null, "familiarTable1"));
+    familiarTables.add(TableName.valueOf("", "familiarTable2"));
+    familiarTables.add(TableName.valueOf("default", "familiarTable3"));
+    familiarTables.add(TableName.valueOf("familiarNamespace", "familiarTable4"));
+    familiarTables.add(TableName.valueOf("familiarNamespace", "familiarTable5"));
+
+    // Create these "familiar" tables so their state can be found
+    TEST_UTIL.getAdmin().createNamespace(NamespaceDescriptor.create("familiarNamespace").build());
+    for (TableName familiarTable : familiarTables) {
+      TEST_UTIL.createTable(familiarTable, "cf");
+      allTables.add(familiarTable);
+    }
+
+    // These tables should be foreign to the cluster.
+    // The cluster should not be able to find their state.
+    allTables.add(TableName.valueOf("hbase", "meta_replica"));
+    allTables.add(TableName.valueOf(null, "defaultNamespaceTable1"));
+    allTables.add(TableName.valueOf("", "defaultNamespaceTable2"));
+    allTables.add(TableName.valueOf("default", "defaultNamespaceTable3"));
+    allTables.add(TableName.valueOf("customNamespace", "customNamespaceTable1"));
+    allTables.add(TableName.valueOf("customNamespace", "customNamespaceTable2"));
+    allTables.add(TableName.valueOf("anotherNamespace", "anotherNamespaceTable"));
+    allTables.add(TableName.valueOf("sharedNamespace", "sharedNamespaceTable1"));
+    allTables.add(TableName.valueOf("sharedNamespace", "sharedNamespaceTable2"));
+
+    // Update master's table descriptors to have all tables
+    TableDescriptor foreignTableDescriptor;
+    for (TableName tableName : allTables) {
+      foreignTableDescriptor = TableDescriptorBuilder.newBuilder(tableName)
+        .setColumnFamily(ColumnFamilyDescriptorBuilder.of("cf")).build();
+      master.getTableDescriptors().update(foreignTableDescriptor, true);
+    }

Review Comment:
   so, I use debug mode to check the unit test, I found that `listTableDescriptors` is always using the `cache` of `FSTableDescriptors` instead of loading from the filesystem, such that we're relying on the `tableStateManager` via meta to see if the additional loaded tables should be included. 
   
   
   the test itself is right, just I'm wondered practically how newly added table on the primary/write cluster being added to the `cache` of `FSTableDescriptors` after master has been initialized ? or does it add from the filesystem after restarting the master (meta is yet synced)? 
   
   so, please consider to test it manually and see if that's what you expected in [HBASE-29579](https://issues.apache.org/jira/browse/HBASE-29579)



-- 
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@hbase.apache.org

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