HBASE-29850 Add support for dynamic attributes using RequestAttributesFactory

[krconv]

Summary

Adds RequestAttributesFactory to AsyncTableBuilder for generating request attributes dynamically per-request.

Motivation

It is possible to do this for the HBase 2 Table client by overriding the RpcControllerFactory on the connection, but that doesn't work reliably with AsyncTable because retries happen on Netty threads, losing thread-local context. This change provides a hook that is guaranteed to be called on the initiating thread.

Changes

• Add RequestAttributesFactory interface with a single create(Map<String, byte[]>) method
• Add setRequestAttributesFactory() to AsyncTableBuilder
• Factory is invoked at the start of each operation (get, put, scan, batch, etc.)
• getRequestAttributes() returns static attributes; factory is only used for actual requests

Usage

AsyncTable<?> table = conn.getTableBuilder(tableName)
.setRequestAttribute("static.key", value)
.setRequestAttributesFactory(attrs -> {
Map<String, byte[]> newAttrs = newHashMap<>(attrs);
newAttrs.put("dynamic.key", getValueFromThreadLocal());
returnnewAttrs;
})
.build();

[Apache9]

So the requirements here is to use different request attribute for different requests?

I think the design of the request attribute API is for static attributes, if we want to support dynamic attribute, we'd better redesign the APIs, like what you propose here, use a factory.

Out of interest, what is your usage for this feature?

Thanks.

[krconv]

Yes—our requirement is to set a specific request attribute to a value calculated from thread-local context.

We use hbase.quota.user.override.key (which configures throttling to use a request attribute) to throttle based on a logical “upstream caller” rather than the connection user. For example, a high-volume nightly job (EmailJobs-nightlyPurgeJob) calls ObjectsWebService-web, which in this case is the component that actually holds the HBase AsyncTable client and issues deletes to a table objects-1. The service propagates the job identity via thread-local HTTP metadata and sets it as a request attribute so that the job is throttled independently, and can be slowed down without impacting user traffic and other lower-volume jobs. This matters because we share a single AsyncTable per table/JVM—without dynamic attributes, one noisy caller can cause throttling across all traffic handled by that service.

This is exactly what we do with the Table client using the RpcControllerFactory as a workaround, and it works well in our experience. With the AsyncTable though, the RpcControllerFactory is called from various threads during retries, so we don't have a way to propagate any thread-local context reliably.

[Apache9]

OK, got it. Seems reasonable.

So what about this:

1. We add a setRequestAttributeFactory method, as you proposed in this PR, but take a Supplier<Map<String, byte[]>> as parameter.
2. Introduce a FixedRequestAttributeFactory(not a native English speaker, maybe you can have a better name...) class, which always return the same request attribute map, as the default implementation. Also use Builder pattern for creating this factory class.
3. Deprecated the old addRequestAttribute method in AsyncTableBuilder, let users use the new setRequestAttributeFactory method. But when users call the old addRequestAttribute method, we internally use FixedRequestAttributeFactory's Builder to create a FixedRequestAttributeFactory in the end.
4. Do the same for sync client interfaces.

WDYT?

Thanks.

[krconv]

What do you think of also deprecating AsyncTable#getRequestAttributes() without a replacement, if the behavior of it is no longer well defined (i.e. the return value could change based on which thread it is called from)? I like your suggestion to merge the two request attribute setters on AsyncTableBuilder into one, but the complication I see is that now getRequestAttributes() no longer has any static set of attributes to return. I'm not sure why getRequestAttributes() would be useful to anyone, but nevertheless it might be harder for a user to migrate away from using that.

[Apache9]

What do you think of also deprecating AsyncTable#getRequestAttributes() without a replacement, if the behavior of it is no longer well defined (i.e. the return value could change based on which thread it is called from)? I like your suggestion to merge the two request attribute setters on AsyncTableBuilder into one, but the complication I see is that now getRequestAttributes() no longer has any static set of attributes to return. I'm not sure why getRequestAttributes() would be useful to anyone, but nevertheless it might be harder for a user to migrate away from using that.

We can just call request attribute factory's method to get the request attribute for this method? And we can add more javadoc to mention that, since now we allow dynamic request attribute when requesting, the return value may not be the same when you implement a dynamic request attribute factory.

[krconv]

I've updated the AsyncTable codepath to reflect the latest suggestions. If this looks good, I'll fix tests and extend to the Table codepath as well. Thank you!

[krconv]

I think master is 4.0.0; should it get deprecated for a later version or should I remove it from master?

[krconv]

The underlying Protobuf ByteString will throw a NPE if value is null, so instead this factory removes the entry if it's null.

[krconv]

The other ideas I had were ImmutableRequestAttributesFactory or StaticRequestAttributesFactory, but I think FixedRequestAttributesFactory is most clear because Immutable- might be interpreted as "the returned collection is immutable" instead of that it will return the same collection every time; and Static- clashes with the reserved word in my opinion

[krconv]

I think it still makes sense to have this factory, mainly for documenting it's behavior; open to replacing with a Supplier<String, byte[]> though

[Apache-HBase]

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 38s		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.
			_ master Compile Tests _
+0 🆗	mvndep	0m 11s		Maven dependency ordering for branch
+1 💚	mvninstall	2m 34s		master passed
+1 💚	compile	3m 15s		master passed
+1 💚	checkstyle	1m 0s		master passed
+1 💚	spotbugs	1m 46s		master passed
+1 💚	spotless	0m 39s		branch has no errors when running spotless:check.
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 11s		Maven dependency ordering for patch
+1 💚	mvninstall	2m 17s		the patch passed
+1 💚	compile	3m 12s		the patch passed
+1 💚	javac	3m 12s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	0m 59s		the patch passed
+1 💚	spotbugs	1m 55s		the patch passed
+1 💚	hadoopcheck	8m 35s		Patch does not cause any errors with Hadoop 3.3.6 3.4.1.
+1 💚	spotless	0m 34s		patch has no errors when running spotless:check.
			_ Other Tests _
+1 💚	asflicense	0m 15s		The patch does not generate ASF License warnings.
		33m 34s

Subsystem	Report/Notes
Docker	ClientAPI=1.53 ServerAPI=1.53 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7665/4/artifact/yetus-general-check/output/Dockerfile
GITHUB PR	#7665
Optional Tests	dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless
uname	Linux b64badd76e4b 6.14.0-1018-aws #18~24.04.1-Ubuntu SMP Mon Nov 24 19:46:27 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	master / fe649a9
Default Java	Eclipse Adoptium-17.0.11+9
Max. process+thread count	86 (vs. ulimit of 30000)
modules	C: hbase-client hbase-server U: .
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7665/4/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]

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 12s		Docker mode activated.
-0 ⚠️	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 _
			_ master Compile Tests _
+0 🆗	mvndep	0m 11s		Maven dependency ordering for branch
+1 💚	mvninstall	2m 33s		master passed
+1 💚	compile	1m 2s		master passed
+1 💚	javadoc	0m 36s		master passed
+1 💚	shadedjars	4m 23s		branch has no errors when building our shaded downstream artifacts.
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 13s		Maven dependency ordering for patch
+1 💚	mvninstall	2m 16s		the patch passed
+1 💚	compile	1m 1s		the patch passed
+1 💚	javac	1m 1s		the patch passed
+1 💚	javadoc	0m 34s		the patch passed
+1 💚	shadedjars	4m 20s		patch has no errors when building our shaded downstream artifacts.
			_ Other Tests _
+1 💚	unit	1m 24s		hbase-client in the patch passed.
-1 ❌	unit	215m 20s	/patch-unit-hbase-server.txt	hbase-server in the patch failed.
		238m 12s

Subsystem	Report/Notes
Docker	ClientAPI=1.53 ServerAPI=1.53 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7665/4/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile
GITHUB PR	#7665
Optional Tests	javac javadoc unit compile shadedjars
uname	Linux 25334e861f0c 6.14.0-1018-aws #18~24.04.1-Ubuntu SMP Mon Nov 24 19:46:27 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	master / fe649a9
Default Java	Eclipse Adoptium-17.0.11+9
Test Results	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7665/4/testReport/
Max. process+thread count	4776 (vs. ulimit of 30000)
modules	C: hbase-client hbase-server U: .
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7665/4/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.

[krconv]

I've fixed the unit test failure, but I believe the new GitHub-based workflows will need to be approved for this PR to re-run them

[krconv]

@Apache9 Can you take another look at this? I believe it is done, and a good addition to the client. Failing tests are unrelated