Re: [Tarantool-patches] [PATCH 8/8] replication: write and read CONFIRM entries

[Serge Petrenko <sergepetrenko@tarantool.org>]

19.06.2020 20:50, Serge Petrenko пишет:
> 09.06.2020 15:20, Serge Petrenko пишет:
>> Make txn_limbo write a CONFIRM entry as soon as a batch of entries
>> receive their acks. CONFIRM entry is written to WAL and later replicated
>> to all the replicas.
>>
>> Now replicas put synchronous transactions into txn_limbo and wait for
>> corresponding confirmation entries to arrive and end up in their WAL
>> before committing the transactions.
>>
> Added a new commit:
>
> commit 88884031e6ea8ca836228432b400ca1557afafa6
> Author: Serge Petrenko <sergepetrenko@tarantool.org>
> Date:   Fri Jun 19 18:21:11 2020 +0300
>
>     txn: rework synchronous tx on_rollback trigger
>
>     Instead of allocating the trigger on heap, which would lead to use 
> after
>     free, allocate the trigger together with the txn.
>     Also, clear the trigger when asynchronous journal write succeeds.
>
>     [TO BE SQUASHED INTO THE PREVIOUS COMMIT]
>
> diff --git a/src/box/txn.c b/src/box/txn.c
> index 4f787db79..16856da0d 100644
> --- a/src/box/txn.c
> +++ b/src/box/txn.c
> @@ -238,6 +238,7 @@ txn_begin(void)
>      /* fiber_on_yield is initialized by engine on demand */
>      trigger_create(&txn->fiber_on_stop, txn_on_stop, NULL, NULL);
>      trigger_add(&fiber()->on_stop, &txn->fiber_on_stop);
> +    trigger_create(&txn->on_write_failure, NULL, NULL, NULL);
>      /*
>       * By default all transactions may yield.
>       * It's a responsibility of an engine to disable yields
> @@ -458,6 +459,12 @@ txn_complete(struct txn *txn)
>                           stop_tm - txn->start_tm);
>          }
>      } else {
> +        /*
> +         * Async write succeeded. Clear the trigger which
> +         * would remove the corresponding txn_limbo entry
> +         * in case of failure.
> +         */
> +        trigger_clear(&txn->on_write_failure);
>          /*
>           * Complete is called on every WAL operation
>           * authored by this transaction. And it not always
> @@ -681,10 +688,10 @@ txn_commit_async(struct txn *txn)
>       * Set a trigger to abort waiting for confirm on WAL write
>       * failure.
>       */
> +    trigger_create(&txn->on_write_failure, txn_limbo_on_rollback,
> +               limbo_entry, NULL);
>      if (is_sync) {
> -        struct trigger trig;
> -        trigger_create(&trig, txn_limbo_on_rollback, limbo_entry, NULL);
> -        txn_on_rollback(txn, &trig);
> +        txn_on_rollback(txn, &txn->on_write_failure);
>      }
>      return 0;
>  }
> diff --git a/src/box/txn.h b/src/box/txn.h
> index e7705bb48..9efd6fd0d 100644
> --- a/src/box/txn.h
> +++ b/src/box/txn.h
> @@ -228,6 +228,14 @@ struct txn {
>       * in case a fiber stops (all engines).
>       */
>      struct trigger fiber_on_stop;
> +    /**
> +     * An on_rollback trigger for synchronous transactions
> +     * removing the txn_limbo entry which would wait for
> +     * confirmation otherwise.
> +     * Is issued on asynchronous write failure and is cleared
> +     * on write success.
> +     */
> +    struct trigger on_write_failure;
>      /** Commit and rollback triggers. */
>      struct rlist on_commit, on_rollback;
>      /**
>

Changed this commit:

commit d3bd829759dd3cf3d49170da701d410af111cb60
Author: Serge Petrenko <sergepetrenko@tarantool.org>
Date:   Fri Jun 19 18:21:11 2020 +0300

     txn: rework synchronous tx on_rollback trigger

     Instead of allocating the trigger on heap, which would lead to use 
after
     free, allocate the trigger on the txn region.

     [TO BE SQUASHED INTO THE PREVIOUS COMMIT]

diff --git a/src/box/txn.c b/src/box/txn.c
index bf17fd749..d960c3888 100644
--- a/src/box/txn.c
+++ b/src/box/txn.c
@@ -670,6 +670,22 @@ txn_commit_async(struct txn *txn)
                 txn_limbo_assign_lsn(&txn_limbo, limbo_entry, lsn);
         }

+       /*
+        * We'll need this trigger for sync transactions later,
+        * but allocation failure is inappropriate after the entry
+        * is sent to journal, so allocate early.
+        */
+       struct trigger *trig;
+       if (is_sync) {
+               size_t size;
+               trig = region_alloc_object(&txn->region, typeof(*trig), 
&size);
+               if (trig == NULL) {
+                       diag_set(OutOfMemory, size, "region_alloc_object",
+                                "trig");
+                       return -1;
+               }
+       }
+
         fiber_set_txn(fiber(), NULL);
         if (journal_write_async(req) != 0) {
                 fiber_set_txn(fiber(), txn);
@@ -682,14 +698,14 @@ txn_commit_async(struct txn *txn)
                 return -1;
         }

-       /*
-        * Set a trigger to abort waiting for confirm on WAL write
-        * failure.
-        */
         if (is_sync) {
-               struct trigger trig;
-               trigger_create(&trig, txn_limbo_on_rollback, 
limbo_entry, NULL);
-               txn_on_rollback(txn, &trig);
+               /*
+                * Set a trigger to abort waiting for confirm on
+                * WAL write failure.
+                */
+               trigger_create(trig, txn_limbo_on_rollback,
+                              limbo_entry, NULL);
+               txn_on_rollback(txn, trig);
         }
         return 0;

-- 
Serge Petrenko