Re: [Tarantool-patches] [PATCH v4 05/12] box: make clear_synchro_queue() write a PROMOTE entry instead of CONFIRM + ROLLBACK

[Serge Petrenko via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

17.04.2021 01:12, Vladislav Shpilevoy пишет:
> Good job on the patch!
>
> See 2 comments below.
>
>> diff --git a/src/box/txn_limbo.c b/src/box/txn_limbo.c
>> index c96e497c6..2346331c7 100644
>> --- a/src/box/txn_limbo.c
>> +++ b/src/box/txn_limbo.c
>> @@ -317,19 +317,21 @@ txn_limbo_write_cb(struct journal_entry *entry)
>>   }
>>   
>>   static void
>> -txn_limbo_write_synchro(struct txn_limbo *limbo, uint16_t type, int64_t lsn)
>> +txn_limbo_write_synchro(struct txn_limbo *limbo, uint16_t type, int64_t lsn,
>> +			uint64_t term)
>>   {
>> -	assert(lsn > 0);
>> +	assert(lsn >= 0);
>>   
>>   	struct synchro_request req = {
>>   		.type		= type,
>>   		.replica_id	= limbo->owner_id,
>>   		.lsn		= lsn,
>> +		.term		= term,
>>   	};
>>   
>>   	/*
>> -	 * This is a synchronous commit so we can
>> -	 * allocate everything on a stack.
>> +	 * This is a synchronous commit so we can allocate everything on a
>> +	 * stack. Note, that promote body includes synchro body.
> 1. I think this might be discarded now. They have the same encoder
> in this version. Up to you.

Sure, reverted to the original version.

>
>>   	 */
>>   	char body[XROW_SYNCHRO_BODY_LEN_MAX];
>>   	struct xrow_header row;
>> @@ -464,6 +466,37 @@ txn_limbo_read_rollback(struct txn_limbo *limbo, int64_t lsn)
>>   		box_update_ro_summary();
>>   }
>>   
>> +void
>> +txn_limbo_write_promote(struct txn_limbo *limbo, int64_t lsn, uint64_t term)
>> +{
>> +	limbo->confirmed_lsn = lsn;
>> +	limbo->is_in_rollback = true;
>> +	/*
>> +	 * We make sure that promote is only written once everything this
>> +	 * instance has may be confirmed.
>> +	 */
>> +	struct txn_limbo_entry *e = txn_limbo_last_synchro_entry(limbo);
>> +	assert(e == NULL || e->lsn <= lsn);
>> +	(void) e;
>> +	txn_limbo_write_synchro(limbo, IPROTO_PROMOTE, lsn, term);
>> +	limbo->is_in_rollback = false;
>> +}
>> +
>> +/**
>> + * Process a PROMOTE request, i.e. confirm all entries <= @req.lsn and rollback all
>> + * entries > @req.lsn.
> 2. For referencing parameters in doxygen style you need to use
> '@a <name>'. So it would be '@a req.lsn'.

Thanks! Fixed. The incremental diff's below.

>
>> + */
>> +static void
>> +txn_limbo_read_promote(struct txn_limbo *limbo,
>> +		       const struct synchro_request *req)
>> +{
>> +	txn_limbo_read_confirm(limbo, req->lsn);
>> +	txn_limbo_read_rollback(limbo, req->lsn + 1);
>> +	assert(txn_limbo_is_empty(&txn_limbo));
>> +	limbo->owner_id = req->origin_id;
>> +	limbo->confirmed_lsn = 0;
>> +}

===========================================================

diff --git a/src/box/txn_limbo.c b/src/box/txn_limbo.c
index 2346331c7..0d2d274f6 100644
--- a/src/box/txn_limbo.c
+++ b/src/box/txn_limbo.c
@@ -330,8 +330,8 @@ txn_limbo_write_synchro(struct txn_limbo *limbo, 
uint16_t type, int64_t lsn,
         };

         /*
-        * This is a synchronous commit so we can allocate everything on a
-        * stack. Note, that promote body includes synchro body.
+        * This is a synchronous commit so we can
+        * allocate everything on a stack.
          */
         char body[XROW_SYNCHRO_BODY_LEN_MAX];
         struct xrow_header row;
@@ -483,8 +483,8 @@ txn_limbo_write_promote(struct txn_limbo *limbo, 
int64_t lsn, uint64_t term)
  }

  /**
- * Process a PROMOTE request, i.e. confirm all entries <= @req.lsn and 
rollback all
- * entries > @req.lsn.
+ * Process a PROMOTE request, i.e. confirm all entries <= @a req.lsn and
+ * rollback all entries > @a req.lsn.
   */
  static void
  txn_limbo_read_promote(struct txn_limbo *limbo,

-- 
Serge Petrenko