Skip to content

HBASE-29715: AssignmentManager is trying to pick up the active cluster's tables before refreshing meta and hfiles - #7474

Merged
taklwu merged 3 commits into
apache:HBASE-29081from
kgeisz:HBASE-29715-assignment-manager-picking-up-foreign-non-meta-tables
Dec 15, 2025
Merged

HBASE-29715: AssignmentManager is trying to pick up the active cluster's tables before refreshing meta and hfiles#7474
taklwu merged 3 commits into
apache:HBASE-29081from
kgeisz:HBASE-29715-assignment-manager-picking-up-foreign-non-meta-tables

Conversation

@kgeisz

@kgeiszkgeisz commented Nov 20, 2025

Copy link
Copy Markdown
Contributor

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 #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) ~[?:?] 

With this PR's changes, the following will be logged instead:

2025-11-20T04:17:29,535 INFO [master/hbase-docker-2:16000.Chore.1] master.HMaster: Skipping foreign table testTable1 in cluster metrics

Change-Id: I976431d363df438c95feaa2c0d5cc23028858947
Change-Id: I8cebc36512c7f59b437d74dfc376fb49d95bf422
@kgeisz

Copy link
Copy Markdown
ContributorAuthor

I noticed PR #7304 also made changes to FSTableDescriptors.java and HbckChore.java via FSUtils.isLocalMetaTable(). Currently, this PR does not make any changes to those files.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

anmolnar
anmolnar previously approved these changes Dec 9, 2025

@anmolnaranmolnar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm.

@anmolnar

Copy link
Copy Markdown
Contributor

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

@Kota-SHKota-SH left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, The unit test failures do not seem related to the change, they pass locally.
However, I have added a minor comment about using listTableDescriptors() to avoid duplication.

@@ -3117,13 +3117,11 @@ public ClusterMetrics getClusterMetricsWithoutCoprocessor(EnumSet<Option> option
try {
Map<TableName, RegionStatesCount> tableRegionStatesCountMap = new HashMap<>();
Map<String, TableDescriptor> tableDescriptorMap = getTableDescriptors().getAll();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice find! This is much cleaner.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm @kgeisz, thanks.

Change-Id: I2001fd4f8289ee6c74621bd6d78f642e77e78129
taklwu
taklwu previously approved these changes Dec 11, 2025
Comment on lines +266 to +271
TableDescriptor foreignTableDescriptor;
for (TableName tableName : allTables) {
foreignTableDescriptor = TableDescriptorBuilder.newBuilder(tableName)
.setColumnFamily(ColumnFamilyDescriptorBuilder.of("cf")).build();
master.getTableDescriptors().update(foreignTableDescriptor, true);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@kgeiszkgeiszDec 11, 2025

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line in HMaster:

Map<String, TableDescriptor> tableDescriptorMap = getTableDescriptors().getAll();

was using HBaseServerBase.getTableDescriptors() rather than using HMaster.getTableDescriptors().

@Kota-SH Suggested I use HMaster.listTableDescriptors() (which uses HMaster.getTableDescriptors()) so the filtering is done automatically. I have pushed that change recently.

@taklwutaklwuDec 11, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@kgeisz

Copy link
Copy Markdown
ContributorAuthor

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

@Apache-HBase

Copy link
Copy Markdown

💔 -1 overall

VoteSubsystemRuntimeLogfileComment
+0 🆗reexec0m 34sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+0 🆗codespell0m 0scodespell was not available.
+0 🆗detsecrets0m 0sdetect-secrets was not available.
+1 💚@author0m 0sThe patch does not contain any @author tags.
+1 💚hbaseanti0m 0sPatch does not have any anti-patterns.
_ HBASE-29081 Compile Tests _
+1 💚mvninstall3m 39sHBASE-29081 passed
+1 💚compile3m 38sHBASE-29081 passed
-0 ⚠️checkstyle0m 15s/buildtool-branch-checkstyle-hbase-server.txtThe patch fails to run checkstyle in hbase-server
+1 💚spotbugs1m 40sHBASE-29081 passed
+1 💚spotless0m 52sbranch has no errors when running spotless:check.
_ Patch Compile Tests _
+1 💚mvninstall3m 16sthe patch passed
+1 💚compile3m 31sthe patch passed
+1 💚javac3m 31sthe patch passed
+1 💚blanks0m 0sThe patch has no blanks issues.
-0 ⚠️checkstyle0m 13s/buildtool-patch-checkstyle-hbase-server.txtThe patch fails to run checkstyle in hbase-server
+1 💚spotbugs1m 42sthe patch passed
+1 💚hadoopcheck12m 22sPatch does not cause any errors with Hadoop 3.3.6 3.4.0.
-1 ❌spotless0m 39spatch has 26 errors when running spotless:check, run spotless:apply to fix.
_ Other Tests _
+1 💚asflicense0m 12sThe patch does not generate ASF License warnings.
40m 17s
SubsystemReport/Notes
DockerClientAPI=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#7474
Optional Testsdupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless
unameLinux 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 toolmaven
Personalitydev-support/hbase-personality.sh
git revisionHBASE-29081 / 35e5cba
Default JavaEclipse Adoptium-17.0.11+9
spotlesshttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7474/2/artifact/yetus-general-check/output/patch-spotless.txt
Max. process+thread count85 (vs. ulimit of 30000)
modulesC: hbase-server U: hbase-server
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7474/2/console
versionsgit=2.34.1 maven=3.9.8 spotbugs=4.7.3
Powered byApache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

@taklwu
taklwu self-requested a review December 11, 2025 03:16

@Kota-SHKota-SH left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment on lines +3119 to +3120
List<TableDescriptor> tableDescriptors = listTableDescriptors(null, null, null, true);
for (TableDescriptor tableDescriptor : tableDescriptors) {

@taklwutaklwuDec 11, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@anmolnaranmolnarDec 11, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

for (TableDescriptordesc : 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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@taklwutaklwuDec 11, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 .

@taklwu
taklwu self-requested a review December 11, 2025 04:58
@taklwu
taklwu dismissed their stale reviewDecember 11, 2025 04:58

asked clarification about the updated patch

@Apache-HBase

Copy link
Copy Markdown

💔 -1 overall

VoteSubsystemRuntimeLogfileComment
+0 🆗reexec1m 22sDocker mode activated.
-0 ⚠️yetus0m 3sUnprocessed 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 💚mvninstall2m 47sHBASE-29081 passed
+1 💚compile0m 43sHBASE-29081 passed
+1 💚javadoc0m 23sHBASE-29081 passed
+1 💚shadedjars4m 29sbranch has no errors when building our shaded downstream artifacts.
_ Patch Compile Tests _
+1 💚mvninstall2m 15sthe patch passed
+1 💚compile0m 44sthe patch passed
+1 💚javac0m 44sthe patch passed
+1 💚javadoc0m 20sthe patch passed
+1 💚shadedjars4m 26spatch has no errors when building our shaded downstream artifacts.
_ Other Tests _
-1 ❌unit229m 47s/patch-unit-hbase-server.txthbase-server in the patch failed.
250m 58s
SubsystemReport/Notes
DockerClientAPI=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#7474
Optional Testsjavac javadoc unit compile shadedjars
unameLinux 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 toolmaven
Personalitydev-support/hbase-personality.sh
git revisionHBASE-29081 / 35e5cba
Default JavaEclipse Adoptium-17.0.11+9
Test Resultshttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7474/2/testReport/
Max. process+thread count6150 (vs. ulimit of 30000)
modulesC: hbase-server U: hbase-server
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7474/2/console
versionsgit=2.34.1 maven=3.9.8
Powered byApache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

@anmolnar
anmolnar self-requested a review December 11, 2025 15:59
@anmolnar
anmolnar dismissed their stale reviewDecember 11, 2025 16:01

Changes have been made.

@anmolnaranmolnar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall lgtm.

@taklwu
taklwu merged commit e08c367 into apache:HBASE-29081Dec 15, 2025
1 check failed
@taklwu

Copy link
Copy Markdown
Contributor

@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.)

anmolnar pushed a commit that referenced this pull request Dec 17, 2025
…r's tables before refreshing meta and hfiles (#7474)
Signed-off-by: Tak Lon (Stephen) Wu <taklwu@apache.org>
Signed-off-by: Andor Molnár <andor@apache.org>
Reviewed by: Kota-SH <shanmukhaharipriya@gmail.com>
anmolnar pushed a commit that referenced this pull request Mar 13, 2026
…r's tables before refreshing meta and hfiles (#7474)
Signed-off-by: Tak Lon (Stephen) Wu <taklwu@apache.org>
Signed-off-by: Andor Molnár <andor@apache.org>
Reviewed by: Kota-SH <shanmukhaharipriya@gmail.com>
anmolnar pushed a commit that referenced this pull request Apr 8, 2026
…r's tables before refreshing meta and hfiles (#7474)
Signed-off-by: Tak Lon (Stephen) Wu <taklwu@apache.org>
Signed-off-by: Andor Molnár <andor@apache.org>
Reviewed by: Kota-SH <shanmukhaharipriya@gmail.com>
anmolnar pushed a commit that referenced this pull request Apr 10, 2026
…r's tables before refreshing meta and hfiles (#7474)
Signed-off-by: Tak Lon (Stephen) Wu <taklwu@apache.org>
Signed-off-by: Andor Molnár <andor@apache.org>
Reviewed by: Kota-SH <shanmukhaharipriya@gmail.com>
kgeisz added a commit to kgeisz/hbase that referenced this pull request Apr 29, 2026
…r's tables before refreshing meta and hfiles (apache#7474)
Signed-off-by: Tak Lon (Stephen) Wu <taklwu@apache.org>
Signed-off-by: Andor Molnár <andor@apache.org>
Reviewed by: Kota-SH <shanmukhaharipriya@gmail.com>
anmolnar pushed a commit that referenced this pull request May 5, 2026
…r's tables before refreshing meta and hfiles (#7474)
Signed-off-by: Tak Lon (Stephen) Wu <taklwu@apache.org>
Signed-off-by: Andor Molnár <andor@apache.org>
Reviewed by: Kota-SH <shanmukhaharipriya@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@kgeisz@Apache-HBase@anmolnar@taklwu@Kota-SH