IGNITE-26608 WIP #12429

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

timoninmaxim · 2025-11-18T15:34:16Z

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

+import java.util.concurrent.atomic.AtomicInteger;
+
+/** */
+public class ContextAttribute<T> {


I suppose you publicate the "Scope" term. Let's use at least public terms if possible. ScopeAttribute looks better.

Nope. The Scope has nothing to do with the Attribute. Context does.

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

sonarqubecloud · 2026-01-21T09:56:48Z

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 2 times, most recently from 3e0bd2a to ba5ad27 Compare January 20, 2026 14:46

IGNITE-26608

e4be167

petrov-mg force-pushed the IGNITE-26608 branch from ba5ad27 to e4be167 Compare January 21, 2026 09:48

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

IGNITE-26608 WIP #12429

Are you sure you want to change the base?

IGNITE-26608 WIP #12429

Uh oh!

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!

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!

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!

sonarqubecloud bot commented Jan 21, 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