HBASE-29779: Call super coprocessor instead of returning for system table #7555

sharmaar12 · 2025-12-16T14:39:57Z

No description provided.

wchevreuil · 2025-12-17T10:53:23Z

hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/ReadOnlyController.java

  }

+  private boolean
+    isOperationOnNonMetaTable(final ObserverContext<? extends RegionCoprocessorEnvironment> c) {


nit: Simply call it "isOnMeta" and return the check c.getEnvironment().getRegionInfo().getTable().equals(TableName.META_TABLE_NAME), moving the negation check to the callers? eg:
...
if (!isOnMeta(c)) {
internalReadOnlyGuard();
}
...

taklwu

will you change your PR based on wellington's comment ?

taklwu · 2025-12-17T20:50:29Z

hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java

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


[nit] do we need to rebase it? I found kevin has this implementation already 00d9900

Yes will do this.

sharmaar12 · 2025-12-18T05:30:08Z

will you change your PR based on wellington's comment ?

@taklwu Yes will be doing that. So at the same time I will do rebase and resubmit.

…ables

There are some other system tables such as acl or namespace which are shared with active cluster hence allowing operation with them in readonly cluster will make system inconsistent.

This is done do avoid any conflicts after the changes in HBASE-29691: Change TableName.META_TABLE_NAME from being a global static

sharmaar12 · 2025-12-17T08:01:38Z

hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/ReadOnlyController.java

    return Optional.of(this);
  }

-  @Override


Moved the preflush function below to put code for both versions together.

sharmaar12 · 2025-12-17T08:09:45Z

.../test/java/org/apache/hadoop/hbase/security/access/TestReadOnlyControllerRegionObserver.java

  }

-  @Test(expected = IOException.class)
-  public void testPreBatchMutateReadOnlyException() throws IOException {


This test function is moved above to be added near the other tests covering same function.

sharmaar12 · 2025-12-17T08:11:49Z

.../test/java/org/apache/hadoop/hbase/security/access/TestReadOnlyControllerRegionObserver.java

+  @Test
+  public void testPreWALAppendReadOnlyMetaNoException() throws IOException {
+    readOnlyController.onConfigurationChange(readOnlyConf);
+    when(key.getTableName()).thenReturn(TableName.META_TABLE_NAME);


Note that here we will be using key to determine on which table the operation is getting performed on hence we have to mock key.getTableName instead of regionInfo.getTable()

sharmaar12 · 2025-12-18T05:29:44Z

hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java

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


Yes will do this.

sharmaar12 · 2025-12-18T10:49:35Z

hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/ReadOnlyController.java

  }

+  private boolean
+    isOperationOnNonMetaTable(final ObserverContext<? extends RegionCoprocessorEnvironment> c) {


Apache-HBase · 2025-12-19T15:10:46Z

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 37s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 0s		codespell was not available.
+0 🆗	detsecrets	0m 0s		detect-secrets was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
+1 💚	hbaseanti	0m 0s		Patch does not have any anti-patterns.
			_ HBASE-29081 Compile Tests _
+1 💚	mvninstall	3m 32s		HBASE-29081 passed
+1 💚	compile	3m 27s		HBASE-29081 passed
-0 ⚠️	checkstyle	0m 15s	/buildtool-branch-checkstyle-hbase-server.txt	The patch fails to run checkstyle in hbase-server
+1 💚	spotbugs	1m 40s		HBASE-29081 passed
+1 💚	spotless	0m 53s		branch has no errors when running spotless:check.
			_ Patch Compile Tests _
+1 💚	mvninstall	3m 3s		the patch passed
+1 💚	compile	3m 33s		the patch passed
+1 💚	javac	3m 33s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
-0 ⚠️	checkstyle	0m 14s	/buildtool-patch-checkstyle-hbase-server.txt	The patch fails to run checkstyle in hbase-server
+1 💚	spotbugs	1m 42s		the patch passed
+1 💚	hadoopcheck	12m 3s		Patch does not cause any errors with Hadoop 3.3.6 3.4.0.
+1 💚	spotless	0m 46s		patch has no errors when running spotless:check.
			_ Other Tests _
+1 💚	asflicense	0m 12s		The patch does not generate ASF License warnings.
		39m 38s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7555/6/artifact/yetus-general-check/output/Dockerfile
GITHUB PR	#7555
JIRA Issue	HBASE-29779
Optional Tests	dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless
uname	Linux 4d53d96975a8 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 / `2d548d1`
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-7555/6/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.

Apache-HBase · 2025-12-19T18:51:30Z

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 42s		Docker mode activated.
-0 ⚠️	yetus	0m 4s		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 💚	mvninstall	4m 1s		HBASE-29081 passed
+1 💚	compile	0m 59s		HBASE-29081 passed
+1 💚	javadoc	0m 29s		HBASE-29081 passed
+1 💚	shadedjars	6m 17s		branch has no errors when building our shaded downstream artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	3m 20s		the patch passed
+1 💚	compile	1m 13s		the patch passed
+1 💚	javac	1m 13s		the patch passed
+1 💚	javadoc	0m 33s		the patch passed
+1 💚	shadedjars	6m 17s		patch has no errors when building our shaded downstream artifacts.
			_ Other Tests _
+1 💚	unit	231m 8s		hbase-server in the patch passed.
		260m 20s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7555/6/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile
GITHUB PR	#7555
JIRA Issue	HBASE-29779
Optional Tests	javac javadoc unit compile shadedjars
uname	Linux 281a41a039ba 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 / `2d548d1`
Default Java	Eclipse Adoptium-17.0.11+9
Test Results	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7555/6/testReport/
Max. process+thread count	4260 (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-7555/6/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.

taklwu

LGTM

This comment has been minimized.

Sign in to view

sharmaar12 force-pushed the sys_table branch from bba9be4 to a0c20a2 Compare December 17, 2025 08:00

sharmaar12 marked this pull request as ready for review December 17, 2025 08:12

This comment has been minimized.

Sign in to view

wchevreuil reviewed Dec 17, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

anmolnar force-pushed the HBASE-29081 branch from e08c367 to 00d9900 Compare December 17, 2025 18:17

taklwu reviewed Dec 17, 2025

View reviewed changes

sharmaar12 force-pushed the sys_table branch from 2431036 to 2a49b5b Compare December 18, 2025 10:49

sharmaar12 requested review from taklwu and wchevreuil December 18, 2025 10:49

This comment has been minimized.

Sign in to view

sharmaar12 added 4 commits December 19, 2025 19:20

HBASE-29779: Call super coprocessor instead of returning for system t…

52c17b6

…ables

Instead of system table just allow operation for hbase meta only

f9c4940

There are some other system tables such as acl or namespace which are shared with active cluster hence allowing operation with them in readonly cluster will make system inconsistent.

Invert the methods name and add negation to caller

5852cd8

Instead of static variable comparison use the API from TableName class

2d548d1

This is done do avoid any conflicts after the changes in HBASE-29691: Change TableName.META_TABLE_NAME from being a global static

sharmaar12 force-pushed the sys_table branch from eb864ab to 2d548d1 Compare December 19, 2025 14:19

sharmaar12 commented Dec 19, 2025

View reviewed changes

taklwu approved these changes Dec 22, 2025

View reviewed changes

HBASE-29779: Call super coprocessor instead of returning for system table #7555

Are you sure you want to change the base?

HBASE-29779: Call super coprocessor instead of returning for system table #7555

Conversation

sharmaar12 commented Dec 16, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

taklwu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sharmaar12 commented Dec 18, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Apache-HBase commented Dec 19, 2025

Uh oh!

Apache-HBase commented Dec 19, 2025

Uh oh!

taklwu left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants