Re: [Tarantool-patches] [PATCH v2 1/1] txn: destroy commit and rollback triggers

[Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Thanks for the review!

On 26.04.2021 12:13, Cyrill Gorcunov wrote:
> On Sun, Apr 25, 2021 at 05:42:35PM +0200, Vladislav Shpilevoy wrote:
>>  static int
>>  txn_add_redo(struct txn *txn, struct txn_stmt *stmt, struct request *request)
>>  {
>> @@ -149,8 +146,11 @@ txn_rollback_one_stmt(struct txn *txn, struct txn_stmt *stmt)
>>  {
>>  	if (txn->engine != NULL && stmt->space != NULL)
>>  		engine_rollback_statement(txn->engine, txn, stmt);
>> -	if (stmt->has_triggers)
>> -		txn_run_rollback_triggers(txn, &stmt->on_rollback);
>> +	if (stmt->has_triggers && trigger_run(&stmt->on_rollback, txn) != 0) {
>> +		diag_log();
>> +		unreachable();
>> +		panic("statement rollback trigger failed");
>> +	}
>>  }
> 
> Good catch, Vlad! Can we please eliminate these unreachable() calls while
> you're modifying it? It is simply not needed (because it is rather a hint
> for compiler and in case if the code is executing it leads to unpredicted
> results so panic() is only right thing to do).

Np:

====================
diff --git a/src/box/txn.c b/src/box/txn.c
index 2ae55b4e1..869e43d02 100644
--- a/src/box/txn.c
+++ b/src/box/txn.c
@@ -148,7 +148,6 @@ txn_rollback_one_stmt(struct txn *txn, struct txn_stmt *stmt)
 		engine_rollback_statement(txn->engine, txn, stmt);
 	if (stmt->has_triggers && trigger_run(&stmt->on_rollback, txn) != 0) {
 		diag_log();
-		unreachable();
 		panic("statement rollback trigger failed");
 	}
 }
@@ -486,7 +485,6 @@ txn_complete_fail(struct txn *txn)
 	if (txn_has_flag(txn, TXN_HAS_TRIGGERS)) {
 		if (trigger_run(&txn->on_rollback, txn) != 0) {
 			diag_log();
-			unreachable();
 			panic("transaction rollback trigger failed");
 		}
 		/* Can't rollback more than once. */
@@ -514,7 +512,6 @@ txn_complete_success(struct txn *txn)
 		 */
 		if (trigger_run_reverse(&txn->on_commit, txn) != 0) {
 			diag_log();
-			unreachable();
 			panic("transaction commit trigger failed");
 		}
 		/* Can't commit more than once. */