dant3 commented on code in PR #7095:
URL: https://github.com/apache/ignite-3/pull/7095#discussion_r2585245254


##########
modules/table/src/test/java/org/apache/ignite/internal/table/distributed/StorageUpdateHandlerTest.java:
##########
@@ -222,4 +236,54 @@ void testUpdateAllBatchedTryLockFailed() {
         assertEquals(row3, result3.binaryRow());
     }
 
+    /**
+     * Tests that {@link StorageUpdateHandler#handleUpdateAll} respects {@code 
shouldRelease()} from the storage engine.
+     *
+     * <p>This test verifies that the implementation checks {@code 
shouldRelease()} during batch processing
+     * to allow the storage engine to perform critical operations like 
checkpoints.
+     */
+    @Test
+    void testHandleUpdateAllExitsEarlyOnShouldRelease() {
+        UUID txUuid = UUID.randomUUID();
+        HybridTimestamp commitTs = CLOCK.now();
+        TablePartitionId partitionId = new TablePartitionId(333, PARTITION_ID);
+
+        Map<UUID, BinaryRow> rows = Map.of(
+                UUID.randomUUID(), binaryRow(new TestKey(1, "foo1"), new 
TestValue(2, "bar")),
+                UUID.randomUUID(), binaryRow(new TestKey(3, "foo3"), new 
TestValue(4, "baz")),
+                UUID.randomUUID(), binaryRow(new TestKey(5, "foo5"), new 
TestValue(7, "zzu"))
+        );
+
+        Map<UUID, TimedBinaryRow> rowsToUpdate = 
rows.entrySet().stream().collect(Collectors.toMap(
+                Map.Entry::getKey,
+                entry -> new TimedBinaryRow(entry.getValue(), null)
+        ));
+        // Given: shouldRelease returns true at first
+        lenient().doReturn(true, 
false).when(shouldReleaseSupplier).getAsBoolean();
+
+        // When: handleUpdateAll is called with write intents
+        storageUpdateHandler.handleUpdateAll(txUuid, rowsToUpdate, 
partitionId, true, null, null, null);
+
+        // Then: All rows are eventually written across multiple batches
+        verify(storage, times(rows.size())).addWrite(any(), any(), any(), 
anyInt(), anyInt());
+
+        // Commit all write intents
+        storageUpdateHandler.switchWriteIntents(txUuid, true, commitTs, null);
+
+        // Verify all rows were committed
+        verify(storage, times(rows.size())).commitWrite(any(), any(), 
eq(txUuid));
+        // And shouldRelease() was called at least once
+        verify(shouldReleaseSupplier, atLeastOnce()).getAsBoolean();
+
+        // Verify all rows can be read with correct values
+        List<BinaryRow> readResults = new ArrayList<>();
+        for (UUID id : rows.keySet()) {
+            ReadResult readResult = storage.read(new 
RowId(partitionId.partitionId(), id), HybridTimestamp.MAX_VALUE);
+            if (readResult != null) {

Review Comment:
   overall result is already asserted after the for cycle. This assert, while 
possible, will only add redundancy.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to