IGNITE-20127 Implement delayed replication acks #2520

ascherbakoff · 2023-08-30T16:15:04Z

Implement delayed replication acks for tx commit.
limitation: doesn't work for sql - needs a separate ticket.
https://issues.apache.org/jira/browse/IGNITE-20127

…nto ignite-20127 � Conflicts: � modules/table/src/main/java/org/apache/ignite/internal/table/distributed/replicator/PartitionReplicaListener.java

vldpyatkov · 2023-09-26T10:31:48Z

modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaResult.java

+     *
+     * @return The replication future.
+     */
+    public @Nullable CompletableFuture<?> repFuture() {


Just name it as it really is replicationFuture.

vldpyatkov · 2023-09-26T10:34:45Z

modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaManager.java

+
+                        if (sender != null) {
+                            // Using strong send here is important to avoid a reordering with a normal response.
+                            clusterNetSvc.messagingService().send(sender, msg0);


How will we avoid reordering if the message may be in another thread?

Agree. I'd rather add operation id or similar same for both fromPrimary and delayed responses in order to match each other. Such ids should be unique within transaction, so basically it's operation counter or similar. From one point of view that means that it won't be possible to use simple inflights counter, however on the other hand it'll be possible to implement more reliable engine including aforementioned optimizations like single response for single noded groups. WDYT?

The absence of reordering is guaranteed by current IO implementation. Both messages will use the same channel for sending a message, and a delayed message is send strictly after the initial.

vldpyatkov · 2023-09-27T20:16:33Z

...table/src/main/java/org/apache/ignite/internal/table/distributed/raft/PartitionListener.java

@@ -149,6 +149,8 @@ public void onWrite(Iterator<CommandClosure<WriteCommand>> iterator) {
        iterator.forEachRemaining((CommandClosure<? extends WriteCommand> clo) -> {
            Command command = clo.command();

+            // LOG.info("CMD {}", command.getClass().getName());


Debugging commnets.

vldpyatkov · 2023-09-27T20:18:47Z

...n/java/org/apache/ignite/internal/table/distributed/replicator/PartitionReplicaListener.java

@@ -156,6 +159,8 @@

 /** Partition replication listener. */
 public class PartitionReplicaListener implements ReplicaListener {
+    private static final IgniteLogger LOGGER = Loggers.forClass(PartitionReplicaListener.class);


We name it LOG, just because we use the same name everywhere.

vldpyatkov · 2023-09-27T20:23:42Z

...n/java/org/apache/ignite/internal/table/distributed/replicator/PartitionReplicaListener.java


-                return txOps;
-            });
+            if (txOps.state == TxState.ABORTED || txOps.state == TxState.COMMITED) {


Use here TxState#isFinalState

vldpyatkov · 2023-09-27T20:37:02Z

...n/java/org/apache/ignite/internal/table/distributed/replicator/PartitionReplicaListener.java

-                if (txOps == null) {
-                    txOps = new TxCleanupReadyFutureList();
-                }
+        var cleanupReadyFut = new CompletableFuture<Void>();


I was trying to understand how this future connected with a cleanup procedure, and I realized the future does not connect with the cleanup. I would rename this variable to 'replicationReadyFut'.

This future is used to prevent races between inflight ops and commit/abort, as it was before my patch. No need to change anything here.

vldpyatkov · 2023-09-27T20:39:42Z

...n/java/org/apache/ignite/internal/table/distributed/replicator/PartitionReplicaListener.java

+                return cmd.txId();
+            });
+
+            // TODO error handling similar to replication group.


All TODO comments should a ticket number.

No longer actual, removed.

vldpyatkov · 2023-09-27T21:16:55Z

...n/java/org/apache/ignite/internal/table/distributed/replicator/PartitionReplicaListener.java

     */
-    private CompletableFuture<Object> applyUpdateCommand(UpdateCommand cmd) {
+    private CompletableFuture<CompletableFuture<?>> applyUpdateCommand(UpdateCommand cmd) {


I don't see it necessary to use this unclear logic. This method returns either a result or a replication future. A future wrapper is never required.

Agree. Proposed API is overcomplicated.

This signature is required to implement correct chaining both for implicit and explicit tx flows.
The result of a call either the immediate result with a delayed replication future, or immediate replication future.
Actually this is the simplest way to implement it.
Can you propose simpler variant, which is also correct ?

vldpyatkov · 2023-09-27T21:17:47Z

...n/java/org/apache/ignite/internal/table/distributed/replicator/PartitionReplicaListener.java

@@ -1853,36 +1902,68 @@ private CompletableFuture<Object> applyUpdateCommand(UpdateCommand cmd) {
     * @param cmd UpdateAll command.
     * @return Raft future, see {@link #applyCmdWithExceptionHandling(Command)}.
     */
-    private CompletableFuture<Object> applyUpdateAllCommand(UpdateAllCommand cmd) {
+    private CompletableFuture<CompletableFuture<?>> applyUpdateAllCommand(UpdateAllCommand cmd, boolean skipDelayedAck) {


The same is before. The future wrapper is not need here.

vldpyatkov · 2023-09-27T21:37:17Z

...le/src/main/java/org/apache/ignite/internal/table/distributed/storage/InternalTableImpl.java

@@ -106,6 +117,8 @@
 * Storage of table rows.
 */
 public class InternalTableImpl implements InternalTable {
+    private static final IgniteLogger LOGGER = Loggers.forClass(InternalTableImpl.class);


We name it LOG, just because we use the same name everywhere.

vldpyatkov · 2023-09-27T21:48:33Z

modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java

@@ -57,7 +65,12 @@
 *
 * <p>Uses 2PC for atomic commitment and 2PL for concurrency control.
 */
-public class TxManagerImpl implements TxManager {
+public class TxManagerImpl implements TxManager, NetworkMessageHandler {
+    private static final IgniteLogger LOGGER = Loggers.forClass(TxManagerImpl.class);


vldpyatkov · 2023-09-27T21:52:49Z

modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java

-        return replicaService.invoke(recipientNode, req)
-                // TODO: IGNITE-20033 TestOnly code, let's consider using Txn state map instead of states.
-                .thenRun(() -> changeState(txId, PENDING, commit ? COMMITED : ABORTED));
+        Function<Void, CompletableFuture<Void>> clo = ignored -> {


Why do not use Supplier<CompletableFuture<Void>>?

Because Supplier can not be used in completableFuture.thenCompose

vldpyatkov · 2023-09-28T10:58:56Z

modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java

+    @Override
+    public void removeInflight(UUID txId) {
+        TxContext tuple = txCtxMap.compute(txId, (uuid, ctx) -> {
+            assert ctx != null;


Add a check that the counter of the flight operations is greater than 0.

vldpyatkov · 2023-09-28T11:15:12Z

modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java

+                    .thenRun(() -> changeState(txId, PENDING, commit ? COMMITED : ABORTED));
+        };
+
+        if (!commit) {


The logic for rollback and commit operations are duplicated. You can just make this on the commit flag:

// All inflights have been completed before the finish. if (commit && tuple.inflights == 0) { tuple.waitRepFut.complete(null); }

I've removed code duplication.

vldpyatkov · 2023-09-28T11:18:59Z

modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java

@@ -57,7 +65,12 @@
 *
 * <p>Uses 2PC for atomic commitment and 2PL for concurrency control.
 */
-public class TxManagerImpl implements TxManager {
+public class TxManagerImpl implements TxManager, NetworkMessageHandler {


I don't see where TxManager is used as a NetworkMessageHandler. Also, I do not understand why it is necessary to calculate inflight operations for the algorithm.

Here org.apache.ignite.internal.tx.impl.TxManagerImpl#start
Inflight calculation is required for safe committing a transaction, see ticket description.

vldpyatkov · 2023-09-28T11:22:04Z

modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java

            if (v == before) {
                return after;
            } else {
                return v;
            }
        });
-
-        assert computeResult == after : "Unable to change transaction state, expected = [" + before + "],"


I'm not sure if it is right to delete this asset because the redundant state changes invoked are not expected.

Agree, assert seems useful.

This assertion is no longer in the master.

sanpwc · 2023-09-28T13:59:42Z

modules/network-api/src/main/java/org/apache/ignite/network/MessagingService.java

@@ -42,6 +42,7 @@ public interface MessagingService {
     *     <li>Messages send to same receiver will be delivered in the same order as they were sent;</li>
     *     <li>If a message N has been successfully delivered to a member implies that all messages to same receiver
     *     preceding N have also been successfully delivered.</li>
+     *     <li>A message is delivered only once</li>


Full stop is missing in the end.

sanpwc · 2023-09-28T14:00:29Z

modules/network/src/main/java/org/apache/ignite/network/DefaultMessagingService.java

@@ -115,6 +115,8 @@ public DefaultMessagingService(
        this.marshaller = marshaller;

        this.outboundExecutor = Executors.newSingleThreadExecutor(NamedThreadFactory.create(nodeName, "MessagingService-outbound-", LOG));
+        // TODO asch the implementation of delayed acks relies on absence of reordering on subsequent messages delivery.


Because if it is not held the commit can be called with some write operation in progress.
But we need to enforce the following invariant on commit - all inflight operations are finished and all corresponding replication is completed.
Actually, looks like we can relax this invariant to "all corresponding replication is completed", because it implies that all updates on primary replica is also applied.

sanpwc · 2023-09-28T14:12:42Z

modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaManager.java

+                        LOG.debug("Sending delayed response for replica request [request={}]", request);
+
+                        if (ex == null) {
+                            msg0 = prepareReplicaResponse(finalSendTimestamp, res0);


What kind of result you expect in repFut (res0). I believe that it is void in any case because it basically tells us that replication is finished or throws an exception.

It is not always void. In delayed ack response txId is set as a result. See applyUpdateCommand

sanpwc · 2023-09-28T14:24:36Z

modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaManager.java

@@ -261,6 +261,35 @@ private void onReplicaMessageReceived(NetworkMessage message, String senderConsi
                    }
                }

+                if (res.repFuture() != null) {


How do we handle single noded raft groups? I didn't check the code below, maybe it's already implemented. But for now, my questions/expectations are following:

In case of single noded raft group and if primary is collocated with that single node, we should skip primary storage updates, and just send the command for the replication, synchronously await the result and return the future with completed res.repFuture and thus send single response to the client side.

Otherwise use common logic with two responses.

In all cases we should check whether repFuture is completed when we are ready to send response from the primary and send only one response if it is.

BTW if items 1 and 2 aren't implemented yet, I'd rather do it in a separate ticket.

No special handling required, it just works as is. But you are correct - it may be additionally optimized. I suggest a separate ticket.

sanpwc · 2023-09-28T14:27:47Z

modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaManager.java

+                            msg0 = prepareReplicaErrorResponse(finalSendTimestamp, ex);
+                        }
+
+                        ClusterNode sender = clusterNetSvc.topologyService().getByConsistentId(senderConsistentId);


I'd rather add overloaded version of send with recipientConsistentId as we discussed with Ivan and just skip the clusterNodeResolver part.
We already have one with channelType param
CompletableFuture<Void> send(String recipientConsistentId, ChannelType channelType, NetworkMessage msg);

sanpwc · 2023-09-28T15:00:12Z

...ationTest/java/org/apache/ignite/distributed/ItTxDistributedTestThreeNodesThreeReplicas.java

+        CompletableFuture<Void> fut = accounts.recordView().upsertAsync(tx, makeValue(1, 100.));
+        // Update must complete now despite the blocked replication protocol.
+        assertTrue(IgniteTestUtils.waitForCondition(fut::isDone, 5_000), "The update future is not completed within timeout");
+


Do you have an idea of how to check that transaction won't finish until unblocking?

sanpwc · 2023-09-28T15:16:45Z

...a/org/apache/ignite/internal/table/distributed/replication/PartitionReplicaListenerTest.java


-            assertFalse(replicaWriteFut.isDone());
+            assertTrue(replicaWriteFut.isDone());


sanpwc · 2023-09-28T15:21:37Z

modules/table/src/testFixtures/java/org/apache/ignite/internal/table/TxAbstractTest.java

@@ -1541,7 +1580,7 @@ public void run() {
                    }

                    while (!stop.get() && firstErr.get() == null) {
-                        InternalTransaction tx = txManager(accounts).begin();
+                        InternalTransaction tx = clientTxManager().begin();


Is an intended change? I mean that, despite the fact that we still have txManager() we want use client ones, right?

Yes it is. Previously a transaction has been started using server txManager, instead of an client.

sanpwc · 2023-09-28T15:25:21Z

modules/transactions/src/main/java/org/apache/ignite/internal/tx/LockManager.java

@@ -33,7 +33,6 @@ public interface LockManager {
     * @param lockKey The key.
     * @param lockMode Lock mode, for example shared, exclusive, intention-shared etc.
     * @return The future with gained lock that will be completed when a lock is successfully acquired.
-     * @throws LockException When a lock can't be taken due to possible deadlock.


Because this exception is never thrown.

sanpwc · 2023-09-28T15:59:23Z

...n/java/org/apache/ignite/internal/table/distributed/replicator/PartitionReplicaListener.java

+            if (res instanceof ReplicaResult) {
+                return (ReplicaResult) res;
+            } else {
+                return new ReplicaResult(res, null);


I'd rather introduce overloaded version with one param.

This will not work, because ReplicaResult represents two results. See my previous comment about it.

sanpwc · 2023-09-28T16:08:47Z

...n/java/org/apache/ignite/internal/table/distributed/replicator/PartitionReplicaListener.java

@@ -1561,6 +1591,7 @@ private CompletableFuture<Object> processMultiEntryAction(ReadWriteMultiRowRepli

                int i = 0;

+                // TODO sort keys ?


sanpwc · 2023-09-28T16:10:57Z

...n/java/org/apache/ignite/internal/table/distributed/replicator/PartitionReplicaListener.java

                    }

                    return updateAllCommand(request, rowIdsToDelete)
-                            .thenCompose(this::applyUpdateAllCommand)
-                            .thenApply(ignored -> result);
+                            .thenCompose(cmd -> applyUpdateAllCommand(cmd, request.skipDelayedAck()))


Same as above. Feel free to answer in one topic only. Why we skipDelayedAck in case of _ALL?

sanpwc · 2023-09-28T16:22:35Z

...n/java/org/apache/ignite/internal/table/distributed/replicator/PartitionReplicaListener.java

-                    CompletableFuture<Object> raftFut = rowIdsToDelete.isEmpty() ? completedFuture(null)
-                            : updateAllCommand(request, rowIdsToDelete).thenCompose(this::applyUpdateAllCommand);
+                    if (rowIdsToDelete.isEmpty()) {
+                        return completedFuture(new ReplicaResult(result, null));


That's a tricky part. We still may want to update safeTime even if there are no rows to delete, otherwise we will need to await idle safe time propagation. We may cover this with separate optimization ticket, but for now I'd prefer calling applyUpdateAllCommand even for empty rows for the simplicity.

Not sure if this is actual after merging.

sanpwc · 2023-09-28T16:33:39Z

...n/java/org/apache/ignite/internal/table/distributed/replicator/PartitionReplicaListener.java

@@ -1853,36 +1902,68 @@ private CompletableFuture<Object> applyUpdateCommand(UpdateCommand cmd) {
     * @param cmd UpdateAll command.
     * @return Raft future, see {@link #applyCmdWithExceptionHandling(Command)}.
     */
-    private CompletableFuture<Object> applyUpdateAllCommand(UpdateAllCommand cmd) {
+    private CompletableFuture<CompletableFuture<?>> applyUpdateAllCommand(UpdateAllCommand cmd, boolean skipDelayedAck) {


Code seems reasonably complicated. It worth adding comments explaining the flow.

sanpwc · 2023-09-28T16:35:57Z

...n/java/org/apache/ignite/internal/table/distributed/replicator/PartitionReplicaListener.java

+                CompletableFuture<Object> fut = applyCmdWithExceptionHandling(cmd).thenApply(res -> {
+                    // Currently result is always null on a successfull execution of a replication command.
+                    // This check guaranties the result will never be lost.
+                    assert res == null : "Replication result is lost";


Well, from my point of view it will always be null, because basically it's void.

But the method signature says otherwise. The result can appear later, so I've added some protection.

sanpwc · 2023-09-28T18:16:07Z

modules/transactions/src/main/java/org/apache/ignite/internal/tx/TxManager.java

+     * @param tx The transaction id.
+     * @return {@code True} if the inflight was registered. The update must be failed on false.
+     */
+    boolean addInflight(UUID txId);


I'd say that add/removeInflight should be a part of InternalTransaction and not TxManager.
Besides that javadoc requires clarification. And what's more important sql currently totally miss the thing.

I disagree. It makes sense only for RW transactions, so it conflicts with RO implementation.

sanpwc · 2023-09-28T18:17:04Z

modules/transactions/src/main/java/org/apache/ignite/internal/tx/TxManager.java

+    /**
+     * Registers the infligh update for a transaction.
+     *
+     * @param tx The transaction id.


txId not tx, but as mentioned below probably not relevant because I believe we should move given method to InternalTransaction.

sanpwc · 2023-09-28T18:18:57Z

...le/src/main/java/org/apache/ignite/internal/table/distributed/storage/InternalTableImpl.java

    ) {
+        // LOGGER.info("enlistWithRetry {} {}", partId, tx.id().toString());


Please remove.

sanpwc · 2023-09-28T18:19:22Z

...le/src/main/java/org/apache/ignite/internal/table/distributed/storage/InternalTableImpl.java

@@ -486,13 +476,54 @@ private <R> CompletableFuture<R> enlistWithRetry(
        return result;
    }

+    private <R> CompletableFuture<R> trackingInvoke(


Given method deserves javadoc.

sanpwc · 2023-09-28T18:20:35Z

...le/src/main/java/org/apache/ignite/internal/table/distributed/storage/InternalTableImpl.java

+                || request instanceof MultipleRowReplicaRequest && ((MultipleRowReplicaRequest) request).requestType() != RW_GET_ALL;
+
+        if (write && !full) {
+            // Track only write requests from explicit transactions.


Why we track only write requests. AFAIK we should fail reading operations within finished transactions as well as modifications ones. WDYT?

Tracking is required only for wait replication on commit purposes.

sanpwc · 2023-09-28T18:30:24Z

...le/src/main/java/org/apache/ignite/internal/table/distributed/storage/InternalTableImpl.java

+                assert noOpChecker != null;
+
+                // Remove inflight if no replication was scheduled, otherwise inflight will be removed by delayed response.
+                if (noOpChecker.test(res, request)) {


noOpCheck naming seems confusing to me.

Renamed to noWriteCheck

sanpwc · 2023-09-28T18:30:43Z

...le/src/main/java/org/apache/ignite/internal/table/distributed/storage/InternalTableImpl.java

@@ -509,16 +540,16 @@ private <T> CompletableFuture<T> postEnlist(CompletableFuture<T> fut, boolean au
            if (e != null) {
                RuntimeException e0 = wrapReplicationException(e);

+                //LOGGER.info("DBG: before auto rollback {}", tx0.id());


Please remove, here and below.

sanpwc · 2023-09-28T18:39:20Z

modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java

+            AtomicReference<CompletableFuture<Void>> ref = new AtomicReference<>();
+            TxContext tuple = txCtxMap.compute(txId, (uuid, tuple0) -> {
+                if (tuple0 == null) {
+                    tuple0 = new TxContext(); // No writes enlisted.


Let's be consistent with the naming. enlistment and in-flight operations have different meaning. It's minor of course.

The naming is correct here - inflighs are caused by enlisted writes.

sanpwc · 2023-09-28T18:52:34Z

modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java

+                tuple.waitRepFut.complete(null);
+            }
+
+            return tuple.waitRepFut.thenCompose(clo).handle((ignored, err) -> {


In order to be on the same page. We do wait for all in-flights to complete even in case of rollback, right?

No we do not.

…nto ignite-20127 � Conflicts: � modules/client/src/test/java/org/apache/ignite/client/fakes/FakeTxManager.java � modules/replicator/src/main/java/org/apache/ignite/internal/replicator/Replica.java � modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaManager.java � modules/replicator/src/main/java/org/apache/ignite/internal/replicator/listener/ReplicaListener.java � modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItTxDistributedTestSingleNode.java � modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItTxDistributedTestThreeNodesThreeReplicas.java � modules/table/src/integrationTest/java/org/apache/ignite/distributed/ReplicaUnavailableTest.java � modules/table/src/main/java/org/apache/ignite/internal/table/distributed/replication/request/ReadWriteMultiRowReplicaRequest.java � modules/table/src/main/java/org/apache/ignite/internal/table/distributed/replicator/PartitionReplicaListener.java � modules/table/src/main/java/org/apache/ignite/internal/table/distributed/storage/InternalTableImpl.java � modules/table/src/test/java/org/apache/ignite/internal/table/RepeatedFinishReadWriteTransactionTest.java � modules/table/src/test/java/org/apache/ignite/internal/table/TxLocalTest.java � modules/table/src/test/java/org/apache/ignite/internal/table/distributed/replication/PartitionReplicaListenerTest.java � modules/table/src/testFixtures/java/org/apache/ignite/internal/table/TxAbstractTest.java � modules/table/src/testFixtures/java/org/apache/ignite/internal/table/impl/DummyInternalTableImpl.java � modules/transactions/src/main/java/org/apache/ignite/internal/tx/InternalTransaction.java � modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/ReadWriteTransactionImpl.java � modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java

…nto ignite-20127 � Conflicts: � modules/table/src/main/java/org/apache/ignite/internal/table/distributed/storage/InternalTableImpl.java � modules/table/src/test/java/org/apache/ignite/internal/table/RepeatedFinishReadWriteTransactionTest.java � modules/table/src/test/java/org/apache/ignite/internal/table/TxLocalTest.java � modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/ReadWriteTransactionImpl.java � modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java

…nto ignite-20127 � Conflicts: � modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaManager.java � modules/table/src/main/java/org/apache/ignite/internal/table/distributed/storage/InternalTableImpl.java � modules/table/src/test/java/org/apache/ignite/internal/table/RepeatedFinishReadWriteTransactionTest.java

* IGNITE-19425 Use data nodes from DistributionZoneManager instead of BaselineManager#nodes and remove BaselineManager (apache#2605) * IGNITE-20127 Implement delayed replication acks - Fixes apache#2520. Signed-off-by: Alexey Scherbakov <alexey.scherbakoff@gmail.com> * IGNITE-20330 Create an abstraction for building indexes (apache#2631) * IGNITE-20513 Use NativeType as the type of the system view column (apache#2668) * IGNITE-20590 Remove TxManager.beginImplicit() (apache#2669) * IGNITE-20588 Use consistent schema when converting data in KV/record views (apache#2667) * IGNITE-20342 Rollback transaction for SQL execution issues (apache#2611) * IGNITE-20445 Clean up write intents for RW transaction on replication… (apache#2657) * IGNITE-20607 Move NativeType to core (apache#2670) * IGNITE-20484 NPE when some operation occurs when the primary replica … (apache#2672) * IGNITE-20442 Sql. Extend grammar with transaction related statements. (apache#2663) * IGNITE-20620 Add index availability command to catalog (apache#2680) * IGNITE-20521 Split Security API from ignite-security module (apache#2662) * IGNITE-20621 Add a replication group index build completion listener to IndexBuilder (apache#2682) * IGNITE-19226 Fetch table schema by timestamp (apache#2648) * IGNITE-19325 Unmute test MultiActorPlacementDriverTest#prolongAfterActiveActorChanger (apache#2677) * IGNITE-20625 Fix ODBC metadata reading error (apache#2687) * IGNITE-20457 Verify commitTimestamp against enlisted partitions expiration timestamps (apache#2658) * IGNITE-20635 Cleanup code wrt IGNITE-18733 mentions (apache#2686) * IGNITE-20317 Return metastorage invokes for zones changes in handlers, immediately recalculate data nodes when scale up/down is immediate. (apache#2685) * IGNITE-19276 Implement a mechanism to build indices distributively (apache#2676) * IGNITE-20567 Move the 'enabled' flag from the authentication configuration to security (apache#2665) * IGNITE-20578 Implement scan over system view (apache#2678) * IGNITE-20387: Remap most exceptions to SqlExceptions for SQL API (apache#2613) * IGNITE-19217 Implement foreign keys query for ODBC (apache#2692) * IGNITE-20512 Remove port range from HTTP server (apache#2673) * Remove port range from HTTP server configuration * Update junit5 version to avoid jar hell. * IGNITE-20657 At the checkpoint ArrayIndexOutOfBoundsException (apache#2696) * IGNITE-20444 Sql. Add restrictions for execution tx related statements with single statement mode (apache#2683) * IGNITE-20116 Linearize storage updates with safeTime adjustment rules (apache#2689) * IGNITE-20636 Add to the MakeIndexAvailableCommand the ability to use only the indexId (apache#2691) * IGNITE-19219 Implement primary keys query in ODBC (apache#2699) * IGNITE-20366 testBatchReadPutConcurrently failed (apache#2694) * IGNITE-20435 Preserve key order in batch opperations (deleteAll, deleteAllExact, insertAll) (apache#2664) * IGNITE-20385 Sql. Fixed handling of NodeLeftException (apache#2622) * IGNITE-20672 Broke compilation (apache#2705) * IGNITE-20530 Start building indexes for write-only indexes (apache#2704) * IGNITE-20659 Placement driver do not create a lease (apache#2700) * IGNITE-20668 Increase wait after a DDL to account for idle safe-time propagation period (apache#2703) * IGNITE-20522 Create the default user 'Ignite' (apache#2684) * Ignite/ignite default user is added to the configuration if the user did not specify one * security.authentication.enabled -> security.enabled * We didn't have the ability to set dynamic defaults in the current implementation of configuration before this patch. Moreover, we don't have the ability to set a default value for NamedConfigValue at all. Now you can add the code to the configuration module if you want to add some defaults based on user-provided values or other external conditions. * IGNITE-20616 ensureReplicaIsPrimary should use getPrimaryReplica instead of awaitPrimaryReplica (apache#2701) * IGNITE-20671 Sql. Fixed ItSqlApiTest#ddl test (apache#2708) * IGNITE-20677 Sql. Improve error reporting in assertThrowsSqlException (apache#2707) * IGNITE-20600 Sql. Fix a message of an error, which occurs while updating primary key column (apache#2695) * IGNITE-20478 Sql. Get rid of UNSPECIFIED_VALUE_PLACEHOLDER (apache#2671) * IGNITE-20430 Got rid of unused set and fixed replica waiters removal (apache#2604) * IGNITE-20454 Sql. Added a callback that is notified when data prefetching is complete (apache#2674) * IGNITE-20002 Implement durable unlock on primary partition re-election (apache#2697) * IGNITE-20630 Select only available nodes for deployment unit download (apache#2713) * IGNITE-20693 Fixed NPE in placement driver actor on deactivation (apache#2718) * IGNITE-20695 Cleanup resource (apache#2723) * IGNITE-20545 Improve logging in AbstractRpcTest (apache#2717) * IGNITE-20395 Clean up write intents for RW transaction on primary (apache#2679) * IGNITE-20699 Decrease idle safe time propagation period in tests (apache#2730) * IGNTIE-20629 Exclude ODBC build from assemble pipeline by default (apache#2706) * IGNITE-20706 Rename CatalogSchemaManager to SchemaManager (apache#2733) * IGNITE-20702 Fix NPE in ReplicaManager.onReplicaMessageReceived (apache#2731) * IGNITE-20704 Add methods to fsync files and directories (apache#2735) * IGNITE-20703 LeaseUpdaterTest causes a NullPointerException (apache#2732) * IGNITE-20599 Implement 'NOT_TOMBSTONE' operation in the meta storage dsl. (apache#2722) * IGNITE-20042 Check table existence before executing each operation in an RW transaction (apache#2721) * IGNITE-20576 Fix testGetReturningTupleWithUnknownSchemaRequestsNewSchema (apache#2738) * IGNITE-20359 Expose storage profiles as a node attribute (apache#2711) * IGNITE-20439 Sql. Support multiple schemas in CatalogSqlSchemaManager (apache#2719) * IGNITE-20311 Sql. Fix behaviour of ROUND function (apache#2690) * IGNITE-20684 Use proper sync method (apache#2715) * IGNITE-20670 Extract deployment code integration test to related module (apache#2714) * IGNITE-20304 Sql. Documentation for system views module (apache#2726) * IGNITE-20644 Java thin: Fix error detection in doSchemaOutInOpAsync (apache#2739) Fix well-known error detection in `ClientTable` when `sendServerExceptionStackTraceToClient` is enabled: `unwrapRootCause` did not work, the important exception can be on any level. * IGNITE-20498 Fix potential catalog version order violations (apache#2734) * IGNITE-20560 Remove the field out of sync with the map (apache#2660) * IGNITE-20099 Update okhttp version (apache#2746) * IGNITE-20720 Move ClusterPerTestIntegrationTest and ClusterPerClassIntegrationTest to Test Fixtures (apache#2744) * IGNITE-20618 Sql. Degradation of SELECT operations performance over time (apache#2728) * IGNITE-20726: Fix incorrect link that mentions a closed ticket. (apache#2745) * IGNITE-20727 Pass schema version in each read/write ReplicaRequest (apache#2747) * IGNITE-20561 Change condition for DistributionZonesUtil#triggerKeyConditionForZonesChanges to solve inconsistency issues (apache#2743) * IGNITE-20734 Fixed a link to cli doc (apache#2750) * IGNITE-20739 fix compilation after merge of IGNITE-20561 (apache#2753) * IGNITE-20624 Fix race between getting logical topology and mapping fragments (apache#2710) --------- Signed-off-by: Alexey Scherbakov <alexey.scherbakoff@gmail.com> Co-authored-by: Mirza Aliev <alievmirza@gmail.com> Co-authored-by: Alexey Scherbakov <alexey.scherbakoff@gmail.com> Co-authored-by: Kirill Tkalenko <tkalkirill@yandex.ru> Co-authored-by: korlov42 <korlov@gridgain.com> Co-authored-by: Roman Puchkovskiy <roman.puchkovskiy@gmail.com> Co-authored-by: Max Zhuravkov <shhwwa@gmail.com> Co-authored-by: Cyrill <cyrill.sizov@gmail.com> Co-authored-by: Ivan Gagarkin <gagarkin.iiu@gmail.com> Co-authored-by: Vladislav Pyatkov <vldpyatkov@gmail.com> Co-authored-by: Igor Sapego <isapego@apache.org> Co-authored-by: Alexander Lapin <lapin1702@gmail.com> Co-authored-by: ygerzhedovich <41903880+ygerzhedovich@users.noreply.github.com> Co-authored-by: Aleksandr Pakhomov <apkhmv@gmail.com> Co-authored-by: Pavel Pereslegin <xxtern@gmail.com> Co-authored-by: Evgeniy Stanilovskiy <stanilovsky@gmail.com> Co-authored-by: Denis Chudov <moonglloom@gmail.com> Co-authored-by: Mikhail <Pochatkin@users.noreply.github.com> Co-authored-by: Vadim Pakhnushev <8614891+valepakh@users.noreply.github.com> Co-authored-by: Alexander Polovtcev <alex.polovtcev@gmail.com> Co-authored-by: Pavel Tupitsyn <ptupitsyn@apache.org> Co-authored-by: Andrew V. Mashenkov <AMashenkov@users.noreply.github.com> Co-authored-by: IgGusev <igusev@gridgain.com>

ascherbakoff added 16 commits August 24, 2023 18:44

IGNITE-20127 Delayed replication commit ack

8fb3038

IGNITE-20127 Thread safe inflight tracking

4329822

Merge branch 'main' of https://gitbox.apache.org/repos/asf/ignite-3 i…

0cd37bb

…nto ignite-20127 � Conflicts: � modules/table/src/main/java/org/apache/ignite/internal/table/distributed/replicator/PartitionReplicaListener.java

IGNITE-20127 Delayed responses for single key updates

dd86b84

IGNITE-20127 Delayed responses for all update types

5bbab4b

IGNITE-20127 Fix tests

3b2eaae

IGNITE-20127 RC

976a5ec

IGNITE-20127 Fix tests

b6740d6

IGNITE-20127 Fix remaining tests

b92518c

IGNITE-20127 Fix remaining tests 2

c22909a

IGNITE-20127 Fix remaining tests 3

35849c1

IGNITE-20127 Fix remaining tests 4

9f1f7e8

IGNITE-20127 Fix remaining tests 5

7e3612b

IGNITE-20127 Fix styles

a0a4acf

IGNITE-20127 Fix hang

6a4aae9

IGNITE-20127 Debug log

40f2ae0

vldpyatkov reviewed Sep 27, 2023

View reviewed changes

vldpyatkov reviewed Sep 28, 2023

View reviewed changes

sanpwc reviewed Sep 28, 2023

View reviewed changes

ascherbakoff added 17 commits September 29, 2023 17:31

IGNITE-20127 Fix tests again

e6b49d8

IGNITE-20127 Fix tests again

f28cd63

IGNITE-20127 Fix tests again 2

acb15fa

IGNITE-20127 Fix compilation

00d92b1

IGNITE-20127 Fix warnings

ffba8b5

IGNITE-20127 Post-review

ce89912

IGNITE-20127 Post-review

5aa2ce6

IGNITE-20127 Fix compilation

a45961d

IGNITE-20127 Cleanup

df20815

IGNITE-20127 Fixed swap issue

c3a43bd

IGNITE-20127 Fix tests

23cc78e

IGNITE-20127 Fix tests 2

42a4615

IGNITE-20127 Fix style

c0a2042

sanpwc approved these changes Oct 6, 2023

View reviewed changes

vldpyatkov approved these changes Oct 6, 2023

View reviewed changes

IGNITE-20127 Fix imports

8032828

ascherbakoff changed the title ~~IGNITE-20127~~ IGNITE-20127 Implement delayed replication acks Oct 6, 2023

IGNITE-20127 Fix imports again

113b038

asfgit closed this in 82c7459 Oct 7, 2023


		assertFalse(replicaWriteFut.isDone());
		assertTrue(replicaWriteFut.isDone());

		@@ -1561,6 +1591,7 @@ private CompletableFuture<Object> processMultiEntryAction(ReadWriteMultiRowRepli

		int i = 0;

		// TODO sort keys ?

		) {
		// LOGGER.info("enlistWithRetry {} {}", partId, tx.id().toString());

IGNITE-20127 Implement delayed replication acks #2520

IGNITE-20127 Implement delayed replication acks #2520

Conversation

ascherbakoff commented Aug 30, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sanpwc Sep 28, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ascherbakoff commented Aug 30, 2023 •

edited

sanpwc Sep 28, 2023 •

edited