IGNITE-26608 Added a unified mechanism for propagating Context Attributes by petrov-mg · Pull Request #12429 · apache/ignite

petrov-mg · 2025-10-16T09:56:56Z

Thank you for submitting the pull request to the Apache Ignite.

In order to streamline the review of the contribution
we ask you to ensure the following steps have been taken:

The Contribution Checklist

There is a single JIRA ticket related to the pull request.
The web-link to the pull request is attached to the JIRA ticket.
The JIRA ticket has the Patch Available state.
The pull request body describes changes that have been made.
The description explains WHAT and WHY was made instead of HOW.
The pull request title is treated as the final commit message.
The following pattern must be used: IGNITE-XXXX Change summary where XXXX - number of JIRA issue.
A reviewer has been mentioned through the JIRA comments
(see the Maintainers list)
The pull request has been checked by the Teamcity Bot and
the green visa attached to the JIRA ticket (see TC.Bot: Check PR)

Notes

If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com #ignite channel.

NSAmelchev · 2025-10-16T17:49:56Z

modules/core/src/main/java/org/apache/ignite/internal/processors/security/IgniteSecurity.java

-     * @return Security context holder.
+     * @return Thread Context Scope.
+     *
+     * @see #withContext(Scope, SecurityContext


Suggested change

* @see #withContext(Scope, SecurityContext

* @see #withContext(Scope, SecurityContext)

timoninmaxim · 2025-10-31T09:35:44Z

modules/commons/src/main/java/org/apache/ignite/internal/thread/context/ThreadContextData.java

+
+    /** */
+    <T> T get(ThreadContextAttribute<T> attr) {
+        if (attr.id() >= attrs.length)


Shoulud we throw assert instead? Is it correct to access to the attribute we actually didn't set?

Could it lead to an error, when user forget to set the var and received default value instead?

No, we shouldn't. The initial value of an attribute literally means - the value returned by the ThreadContext#get method if the attribute's value is not explicitly set.

It can be used to skip explicitly setting attributes for some internal operations/workers and still get some value for attribute.

timoninmaxim · 2025-10-31T09:46:38Z

modules/commons/src/main/java/org/apache/ignite/internal/thread/context/Scope.java

-    }
+public interface Scope extends AutoCloseable {
+    /** Binds attribute with specified value to the current scope. */
+    public <T> Scope withAttribute(ThreadContextAttribute<T> attr, T val);


Broken incapsulation. ThreadContextScope inherits Scope, then ThreadContextAttribute must be part of ThreadContextScrope.

It’s more of a naming problem than an “encapsulation.” Anyway, I’ve updated the PR and removed the Scope interface entirely — on second thought, it’s redundant for the current patch. So now only ThreadContextScope remains.

modules/commons/src/main/java/org/apache/ignite/internal/thread/context/Scope.java

modules/commons/src/main/java/org/apache/ignite/internal/thread/context/ScopedContext.java

modules/commons/src/main/java/org/apache/ignite/internal/thread/context/ContextAttribute.java

timoninmaxim · 2025-11-18T15:40:43Z

modules/commons/src/main/java/org/apache/ignite/internal/thread/context/ContextAttribute.java

+    /** */
+    private ContextAttribute(byte id, T initVal) {
+        this.id = id;
+        this.bitmask = 1 << id;


Looks that storing ID is enough. The bitmask is a parameter of ContextDataChain only.

Nope. The bitmask is used in many places to check for the presence of an attribute value. Therefore, it's better to calculate it once and store it in the attribute instance.

modules/commons/src/main/java/org/apache/ignite/internal/thread/context/ContextAttribute.java

timoninmaxim · 2025-11-18T15:56:38Z

modules/commons/src/main/java/org/apache/ignite/internal/thread/context/ScopedContext.java

+    }
+
+    /** */
+    public <T> ScopedContext with(ContextAttribute<T> attr, T val) {


Looks weird, an object that stores an attribute value accepts another attribute (name + value). AFAIU, it's a responsibility of Scope to store all attributes within single scope.

it's a responsibility of Scope to store all attributes within single scope

Nope. See comments above.

I changed class naming a bit so it may be less confusing now.

Looks weird, an object that stores an attribute value accepts another attribute (name + value)
Also keep in mind that the current patch is based on intrusive lists and it is unlikely to be possible to avoid the "weirdness" you mentioned.

sonarqubecloud · 2025-12-01T07:40:36Z

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

modules/commons/src/main/java/org/apache/ignite/internal/thread/context/Context.java

timoninmaxim · 2026-02-03T13:15:55Z

modules/commons/src/main/java/org/apache/ignite/internal/thread/context/Context.java

+     * try-with-resource block to close the returned Scope. Note, updates must be undone in the same order they were applied.
+     */
+    public static <T> Scope set(ContextAttribute<T> attr, T val) {
+        if (get(attr) == val)


WDYT, can we move this optimization into applyUpdates? Currently, there are 2 calls of INSTANCE.get(), that the most expensive part.

timoninmaxim · 2026-02-03T13:23:42Z

modules/commons/src/main/java/org/apache/ignite/internal/thread/context/Context.java

+
+        /** */  
+        <T> ContextUpdater set(ContextAttribute<T> attr, T val) {
+            if (get(attr) == val)


Same here, can we check it later, within Context?

modules/commons/src/main/java/org/apache/ignite/internal/thread/context/Context.java

timoninmaxim · 2026-02-03T14:57:18Z

modules/core/src/main/java/org/apache/ignite/internal/processors/pool/PoolProcessor.java

                            "already used: " + id);

-                    extPools[id] = ctx.security().enabled() ? new SecurityAwareIoPool(ctx.security(), ex) : ex;
+                    extPools[id] = ctx.security().enabled() ? ContextAwareIoPool.wrap(ex) : ex;


Here and below: looks like we should not check security anymore and use ContextAwareIoPool everywhere?

We will definitely do this, but in a separate ticket.

...s/src/main/java/org/apache/ignite/internal/thread/context/function/ContextAwareCallable.java

timoninmaxim · 2026-02-04T09:13:38Z

modules/commons/src/main/java/org/apache/ignite/internal/thread/context/Context.java

+    }
+
+    /** Updates the current context with the specified attributes and their corresponding values. */
+    private Scope applyAttributeUpdates(AttributeValueHolder<?>... atrVals) {


Misprint, atrVals -> attrVals

modules/commons/src/main/java/org/apache/ignite/internal/thread/context/Context.java

timoninmaxim · 2026-02-04T09:19:46Z

modules/commons/src/main/java/org/apache/ignite/internal/thread/context/Context.java

+
+        changeState(prevSnp, newSnp);
+
+        return () -> INSTANCE.get().changeState(newSnp, prevSnp);


It looks it should work with () -> changeState(newSnp, prevSnp). Because we close Scope within the same thread that restores snapshot.

…dating Context.

sonarqubecloud · 2026-02-07T15:56:52Z

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

NSAmelchev reviewed Oct 16, 2025

View reviewed changes

petrov-mg force-pushed the IGNITE-26608 branch 2 times, most recently from 9bf30e8 to 324206b Compare October 17, 2025 11:30

NSAmelchev approved these changes Oct 17, 2025

View reviewed changes

petrov-mg force-pushed the IGNITE-26608 branch 4 times, most recently from da14337 to 8795941 Compare October 28, 2025 21:44

timoninmaxim reviewed Oct 31, 2025

View reviewed changes

petrov-mg force-pushed the IGNITE-26608 branch 6 times, most recently from da9cc26 to a5347c7 Compare November 17, 2025 08:08

timoninmaxim requested changes Nov 18, 2025

View reviewed changes

petrov-mg force-pushed the IGNITE-26608 branch 3 times, most recently from 0930d07 to fcbcc57 Compare November 28, 2025 23:03

petrov-mg force-pushed the IGNITE-26608 branch from fcbcc57 to df78984 Compare December 1, 2025 07:33

petrov-mg force-pushed the IGNITE-26608 branch 4 times, most recently from 31be217 to 98f14f7 Compare January 25, 2026 14:30

timoninmaxim requested changes Feb 3, 2026

View reviewed changes

petrov-mg changed the title ~~IGNITE-26608 WIP~~ IGNITE-26608 Added a unified mechanism for propagating Context Attributes Feb 3, 2026

timoninmaxim requested changes Feb 4, 2026

View reviewed changes

petrov-mg force-pushed the IGNITE-26608 branch from 4244313 to 9eeac8d Compare February 4, 2026 11:13

timoninmaxim approved these changes Feb 4, 2026

View reviewed changes

petrov-mg force-pushed the IGNITE-26608 branch from 9eeac8d to 9b8f71f Compare February 5, 2026 19:55

petrov-mg added 6 commits February 7, 2026 18:35

IGNITE-26608

eb7f0c0

IGNITE-26608

cca187a

IGNITE-26608 Fixed multiple access to a thread local variable when up…

1386201

…dating Context.

IGNITE-26608 Fixed typo in attribute arguments.

80abadc

IGNITE-26608 Removed ThreadLocal access when closing scope.

a399f31

IGNITE-26608 Replace OperationSecurity Context with Scope in tests.

5df3c84

petrov-mg force-pushed the IGNITE-26608 branch from 9b8f71f to 5df3c84 Compare February 7, 2026 15:49

	* @see #withContext(Scope, SecurityContext
	* @see #withContext(Scope, SecurityContext)


		changeState(prevSnp, newSnp);

		return () -> INSTANCE.get().changeState(newSnp, prevSnp);

Conversation

petrov-mg commented Oct 16, 2025

The Contribution Checklist

Notes

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!

petrov-mg Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petrov-mg Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Dec 1, 2025

Quality Gate passed

Uh oh!

Uh oh!

Uh oh!

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!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Feb 7, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

petrov-mg Oct 31, 2025 •

edited

Loading

petrov-mg Oct 31, 2025 •

edited

Loading