IGNITE-28806 Fix dynamic cache group affinity init during first local join by oleg-vlsk · Pull Request #13263 · apache/ignite

oleg-vlsk · 2026-06-22T07:37:46Z

… join

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 tab PR Check at TC.Bot - Instance 1 or TC.Bot - Instance 2)

Notes

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

chesnokoff · 2026-06-25T07:40:14Z

+    private boolean skipNotStartedDynamicGroupOnFirstLocalJoin(
+        GridDhtPartitionsExchangeFuture fut,
+        CacheGroupDescriptor desc,
+        @Nullable CacheGroupContext grp,
+        boolean newAff
+    ) {
+        if (grp != null)
+            return false;
+
+        if (newAff)
+            return false;
+
+        if (!firstLocalJoinExchange(fut))
+            return false;
+
+        AffinityTopologyVersion grpStartTopVer = desc.startTopologyVersion();
+
+        if (grpStartTopVer == null)
+            return false;
+
+        return grpStartTopVer.after(fut.initialVersion());
+    }


Can we make smth like

/** */ private boolean skipNotStartedDynamicGroup( GridDhtPartitionsExchangeFuture fut, CacheGroupDescriptor desc, @Nullable CacheGroupContext grp, boolean newAff ) { if (grp != null || newAff) return false; AffinityTopologyVersion grpStartTopVer = desc.startTopologyVersion(); return grpStartTopVer != null && grpStartTopVer.after(fut.initialVersion()); }

To make this check based on the cache group start version instead of the first join?

Yes, makes sense. If we remove the first-join-check, the fix will be broader. Corrections added.

chesnokoff · 2026-06-25T07:44:09Z

+    private static final String ERR_MSG = "Invalid exchange futures state";
+
+    /** */
+    private static final int SRV_NODES = 3;
+
+    /** */
+    private static final int CLIENT_THREADS = 32;
+
+    /** */
+    private static final int RESTART_CNT = 10;
+
+    /** */
+    private static final int FIRST_CLIENT_PORT = 10800;
+
+    /** */
+    private static final int CACHE_CFG_CNT = 30;
+
+    /** */
+    private final AtomicBoolean stopClients = new AtomicBoolean();
+
+    /** */
+    private final AtomicInteger clientCreateSuccesses = new AtomicInteger();
+
+    /** */
+    private final AtomicReference<Throwable> caughtErr = new AtomicReference<>();
+
+    /** */
+    private final CountDownLatch caughtLatch = new CountDownLatch(1);
+
+    /** */
+    private final Map<Integer, ClientCacheConfiguration> cacheConfs = new HashMap<>();
+
+    /** */
+    private Thread.UncaughtExceptionHandler oldUncaughtHnd;
+
+    /** */
+    private IgniteInternalFuture<?> startFut;
+
+    /** */
+    private IgniteInternalFuture<?> clientFut;


I think this test is too heavy for this fix. It looks more like a stress/probabilistic test than a deterministic regression test. Even if the bug appears again, this test can still pass if the race does not happen during the run

I suggest reverting this test in a separate commit, in case another reviewer asks to restore it later. Instead, we can add a smaller happy-path test to check that we do not skip extra cache groups by mistake

chesnokoff · 2026-06-30T06:12:19Z

+                    if (log.isDebugEnabled()) {
+                        log.debug("Skip coordinator affinity initialization for cache group started after" +
+                            " current exchange [grp=" + desc.cacheOrGroupName() +
+                            ", grpId=" + desc.groupId() + ", curTopVer=" + topVer +


Suggested change

", grpId=" + desc.groupId() + ", curTopVer=" + topVer +

", grpId=" + grpId + ", curTopVer=" + topVer +

desc.groupId() was already calculated at line

int grpId = desc.groupId();

chesnokoff · 2026-06-30T06:36:52Z

        return null;
    }

+    /** */


Could you add a short doc to skipNotStartedDynamicGroup? Explain that the group has not started yet at this exchange, so it will be initialized later by its own exchange and that is why we can safely skip it here

Javadoc added.

chesnokoff · 2026-06-30T06:40:04Z

+
+    /** */
+    @Test
+    public void testThinClientDynamicCacheStartCreatesCacheGroups() throws Exception {


I'm not sure that this test covers the fix. skipNotStartedDynamicGroup is called only from initCoordinatorCaches, and this method runs only on a local join

Here all 3 servers are started before dynamic caches are created, so the new skip check code is not called

Could you start one more node after cache creation and check groups and caches on this new node too? Then the test will really check that we do not skip valid groups

I thoroughly debugged the current version of the test and found out that it does not cover the skip check path at all:

CacheAffinitySharedManager#initCoordinatorCaches is not called during the dynamic cache creation;

neither is it called during the proposed additional node startup.

It is only called during the initial cluster startup on the coordinator node upon static cache initialization. That’s why it was necessary to repeatedly shut down the cluster and start it up again in the reproducer and the initial stress test.

Based on that, a smaller happy-path test with several static cache groups seems more relevant: it verifies that cache groups which belong to the current coordinator initialization exchange are not skipped and are properly started, even though during their initialization CacheAffinitySharedManager#skipNotStartedDynamicGroup is called. Therefore I changed the happy-path test to CacheAffinityCoordinatorInitTest#testStaticCacheGroupsOnCoordinatorStart.

This still does not cover the exact “future dynamic group descriptor” scenario. Covering that would require either an internal hook, or the initial stress test.

… join

…rtedDynamicGroup and add DynamicCacheStartExchangeTest (as per review)

chesnokoff

LGTM

One note for next reviewer: initial patch had a reproducer test
53bb6f9

But that test was more like a stress test. It was heavy and probabilistic, so it could make TC runs slower and still miss the bug in some runs

I think the current deterministic happy-path test is an acceptable compromise. It checks that we do not skip cache groups that should be started in the current exchange

But the final decision is up to committer

chesnokoff reviewed Jun 25, 2026

View reviewed changes

chesnokoff reviewed Jun 30, 2026

View reviewed changes

Valuyskiy.O.Y added 5 commits July 7, 2026 12:25

IGNITE-28806 Fix dynamic cache group affinity init during first local…

53bb6f9

… join

IGNITE-28806 Remove DynamicCacheStartDuringRestartExchangeTest

482bb48

IGNITE-28806 Add corrections to CacheAffinitySharedManager#skipNotSta…

043282e

…rtedDynamicGroup and add DynamicCacheStartExchangeTest (as per review)

IGNITE-28806 Add corrections to CacheAffinitySharedManager as per review

ab4d203

IGNITE-28806 Refactor DynamicCacheStartExchangeTest

94ec9cb

oleg-vlsk force-pushed the IGNITE-28806 branch from 03097f0 to 94ec9cb Compare July 7, 2026 02:26

chesnokoff approved these changes Jul 7, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

IGNITE-28806 Fix dynamic cache group affinity init during first local join#13263

IGNITE-28806 Fix dynamic cache group affinity init during first local join#13263
oleg-vlsk wants to merge 5 commits into
apache:masterfrom
oleg-vlsk:IGNITE-28806

oleg-vlsk commented Jun 22, 2026

Uh oh!

chesnokoff Jun 25, 2026

Uh oh!

oleg-vlsk Jun 29, 2026

Uh oh!

chesnokoff Jun 25, 2026

Uh oh!

oleg-vlsk Jun 29, 2026

Uh oh!

chesnokoff Jun 30, 2026

Uh oh!

oleg-vlsk Jul 7, 2026

Uh oh!

chesnokoff Jun 30, 2026

Uh oh!

oleg-vlsk Jul 7, 2026

Uh oh!

chesnokoff Jun 30, 2026 •

edited

Loading

Uh oh!

oleg-vlsk Jul 7, 2026 •

edited

Loading

Uh oh!

chesnokoff left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	", grpId=" + desc.groupId() + ", curTopVer=" + topVer +
	", grpId=" + grpId + ", curTopVer=" + topVer +

Uh oh!

Conversation

oleg-vlsk commented Jun 22, 2026

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!

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!

chesnokoff Jun 30, 2026 • 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!

oleg-vlsk Jul 7, 2026 • 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!

chesnokoff left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

chesnokoff Jun 30, 2026 •

edited

Loading

oleg-vlsk Jul 7, 2026 •

edited

Loading