IGNITE-27678 Same partitions on different nodes can hold different updates if writeThrough is enabled by zstan · Pull Request #12666 · apache/ignite

zstan · 2026-01-28T08:47:31Z

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.

…dates if writeThrough is enabled

zstan · 2026-01-29T05:12:25Z

...c/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/GridDhtTxRemote.java

-        res.add(nodeId);
-
-        return res;
+        return nearNodeId != nodeId ? List.of(nearNodeId, nodeId) : List.of(nearNodeId);


not relates to an issue, just a bit improvement

Are we sure we need no equals()?

Vladsz83 · 2026-02-01T05:30:14Z

...ontrol-utility/src/test/java/org/apache/ignite/util/IdleVerifyCheckWithWriteThroughTest.java

+     * <ul>
+     *   <li>Start 3 node [node0, node1, node2].</li>
+     *   <li>Initialize put operation into transactional cache where [node1] holds primary partition for such insertion.</li>
+     *   <li>Kill [node1] right after tx PREPARE stage is completed (it triggers tx recovery procedure.</li>


Minor javadoc fix: procedure. -> procedure).

Vladsz83 · 2026-02-01T05:30:39Z

...ontrol-utility/src/test/java/org/apache/ignite/util/IdleVerifyCheckWithWriteThroughTest.java

+
+    /** Test scenario:
+     * <ul>
+     *   <li>Start 3 node [node0, node1, node2].</li>


Minor javadoc fix: 3 node -> 3 nodes

Vladsz83 · 2026-02-01T05:31:34Z

...ontrol-utility/src/test/java/org/apache/ignite/util/IdleVerifyCheckWithWriteThroughTest.java

+
+/** */
+public class IdleVerifyCheckWithWriteThroughTest extends GridCommandHandlerClusterPerMethodAbstractTest {
+    /**  */


/** */ -> /** */

Vladsz83 · 2026-02-01T05:32:17Z

...ontrol-utility/src/test/java/org/apache/ignite/util/IdleVerifyCheckWithWriteThroughTest.java

+        if (withPersistence)
+            cleanPersistenceDir();
+
+        err = new AtomicReference<>();


Can we initialize all there fields with their declarations.

no we can`t because they are static and need to reinitialize before each test call

Vladsz83 · 2026-02-01T05:40:10Z

...ontrol-utility/src/test/java/org/apache/ignite/util/IdleVerifyCheckWithWriteThroughTest.java

+        });
+
+        MapCacheStoreStrategy strategy = new MapCacheStoreStrategy();
+        strategy.resetStore();


Do we need to reset at start?

Vladsz83 · 2026-02-01T06:28:49Z

...ontrol-utility/src/test/java/org/apache/ignite/util/IdleVerifyCheckWithWriteThroughTest.java

+    public static Boolean failOnSessionStart;
+
+    /** */
+    @Parameterized.Parameters(name = "cmdHnd={0}, withPersistence={1}")


and , failOnSessionStart={2} ?

done, thanks

Vladsz83 · 2026-02-01T06:33:29Z

...c/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/GridDhtTxRemote.java

-        res.add(nodeId);
-
-        return res;
+        return nearNodeId != nodeId ? List.of(nearNodeId, nodeId) : List.of(nearNodeId);


Are we sure we need no equals()?

Vladsz83 · 2026-02-01T06:39:45Z

...ain/java/org/apache/ignite/internal/processors/cache/store/GridCacheStoreManagerAdapter.java

            }
        }
+        catch (RuntimeException e) {
+            U.error(log, "Exception raised during notify SessionListeners: ", e);


Suggestion: notify SessionListeners: -> notifying cache store session listeners.

Vladsz83 · 2026-02-01T06:41:12Z

...ain/java/org/apache/ignite/internal/processors/cache/store/GridCacheStoreManagerAdapter.java

    protected boolean alwaysKeepBinary;

+    /** Failure handler reaction. */
+    private Consumer<Throwable> failureHandlerAction;


Lets make an abbreviation.

why do we need abbr if it correctly passed through check style ? Upper i see no abbr, also i think that abbr make it less readable.

Vladsz83 · 2026-02-01T06:47:45Z

...ain/java/org/apache/ignite/internal/processors/cache/store/GridCacheStoreManagerAdapter.java

        GridKernalContext ctx = igniteContext();
        CacheConfiguration cfg = cacheConfiguration();

+        failureHandlerAction = e -> ctx.failure().process(new FailureContext(FailureType.CRITICAL_ERROR, e));


Maybe we should check if(sesLsnrs!=null). No need to worry user if there are no cache store session listeners.

zstan · 2026-02-02T08:12:25Z

...ontrol-utility/src/test/java/org/apache/ignite/util/IdleVerifyCheckWithWriteThroughTest.java

+    private static CountDownLatch nodeKill;
+
+    /** Tx message flag. */
+    private static volatile boolean finalTxMsgPassed;


it used in static class

zstan · 2026-02-02T08:12:57Z

...ontrol-utility/src/test/java/org/apache/ignite/util/IdleVerifyCheckWithWriteThroughTest.java

+    private static volatile boolean finalTxMsgPassed;
+
+    /** Session method flag. */
+    private static AtomicBoolean sessionTriggered = new AtomicBoolean();


I prefer to store as is.

...ontrol-utility/src/test/java/org/apache/ignite/util/IdleVerifyCheckWithWriteThroughTest.java

Vladsz83 · 2026-02-01T17:55:06Z

...ain/java/org/apache/ignite/internal/processors/cache/store/GridCacheStoreManagerAdapter.java

            }
        }
+        catch (RuntimeException e) {
+            U.error(log, "Exception raised during sessionEnd: ", e);


Let's make the massage more user-friendly. What does it mean, what are the issues?

...ontrol-utility/src/test/java/org/apache/ignite/util/IdleVerifyCheckWithWriteThroughTest.java

sonarqubecloud · 2026-02-02T09:52:23Z

Quality Gate failed

Failed conditions
4 New Code Smells (required ≤ 1)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

zstan force-pushed the ignite-27678 branch from fd3cc2e to e1d7d49 Compare January 28, 2026 10:54

zstan marked this pull request as ready for review January 28, 2026 10:54

IGNITE-27678 Same partitions on different nodes can hold different up…

595c395

…dates if writeThrough is enabled

zstan force-pushed the ignite-27678 branch from e1d7d49 to 595c395 Compare January 28, 2026 12:29

zstan commented Jan 29, 2026

View reviewed changes

zstan added 2 commits January 29, 2026 10:28

exception logic as for main stream

355f365

minor

9b4edae

Vladsz83 reviewed Feb 1, 2026

View reviewed changes

zstan commented Feb 2, 2026

View reviewed changes

...ontrol-utility/src/test/java/org/apache/ignite/util/IdleVerifyCheckWithWriteThroughTest.java Show resolved Hide resolved

fix after review

767e485

Vladsz83 reviewed Feb 2, 2026

View reviewed changes

err msg changed

4f20b0b

Conversation

zstan commented Jan 28, 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!

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!

Vladsz83 Feb 1, 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!

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sonarqubecloud bot commented Feb 2, 2026

Quality Gate failed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Vladsz83 Feb 1, 2026 •

edited

Loading