From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id B54DD26621 for ; Wed, 7 Aug 2019 11:13:21 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id vasyNfwJlwIl for ; Wed, 7 Aug 2019 11:13:21 -0400 (EDT) Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 41DD426612 for ; Wed, 7 Aug 2019 11:13:21 -0400 (EDT) From: Nikita Pettik Subject: [tarantool-patches] [PATCH 2/3] txn: merge struct sql_txn into struct txn Date: Wed, 7 Aug 2019 18:13:13 +0300 Message-Id: <7d398281ec92e03c1d6f5f1bf93970debcfcc684.1565190104.git.korablev@tarantool.org> In-Reply-To: References: In-Reply-To: References: Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: tarantool-patches@freelists.org Cc: v.shpilevoy@tarantool.org, Nikita Pettik This procedure is processed in a several steps. Firstly, we add name to struct txn_savepoint since we should be capable of operating on named savepoints (which in turn is SQL feature). Still, anonymous (in the sense of name absence) savepoints are also valid. Then, we add list (as implementation of stailq) of savepoints to struct txn: it allows us to find savepoint by its name. Finally, we patch rollback to/release savepoint routines: for rollback tail of the list containing savepoints is cut (but subject of rollback routine remains in the list); for release routine we cut tail including node being released. --- src/box/sql/vdbe.c | 52 +++++++++++--------------------------- src/box/sql/vdbe.h | 9 ------- src/box/sql/vdbeaux.c | 32 ----------------------- src/box/txn.c | 70 ++++++++++++++++++++++++++++++++++++++++++--------- src/box/txn.h | 26 +++++++++++++------ 5 files changed, 91 insertions(+), 98 deletions(-) diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index 246654cb7..7d05ce11c 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -2823,13 +2823,9 @@ case OP_Count: { /* out2 */ case OP_Savepoint: { int p1; /* Value of P1 operand */ char *zName; /* Name of savepoint */ - Savepoint *pNew; - Savepoint *pSavepoint; - Savepoint *pTmp; struct txn *txn = in_txn(); - struct sql_txn *psql_txn = txn != NULL ? txn->psql_txn : NULL; - if (psql_txn == NULL) { + if (txn == NULL) { assert(!box_txn()); diag_set(ClientError, ER_NO_TRANSACTION); goto abort_due_to_error; @@ -2840,25 +2836,21 @@ case OP_Savepoint: { /* Assert that the p1 parameter is valid. Also that if there is no open * transaction, then there cannot be any savepoints. */ - assert(psql_txn->pSavepoint == NULL || box_txn()); + assert(stailq_empty(&txn->savepoints) || box_txn()); assert(p1==SAVEPOINT_BEGIN||p1==SAVEPOINT_RELEASE||p1==SAVEPOINT_ROLLBACK); if (p1==SAVEPOINT_BEGIN) { - /* Create a new savepoint structure. */ - pNew = sql_savepoint(p, zName); - /* Link the new savepoint into the database handle's list. */ - pNew->pNext = psql_txn->pSavepoint; - psql_txn->pSavepoint = pNew; + /* + * Savepoint is available by its name so we don't + * care about object itself. + */ + (void) txn_savepoint_new(txn, zName); } else { /* Find the named savepoint. If there is no such savepoint, then an * an error is returned to the user. */ - for( - pSavepoint = psql_txn->pSavepoint; - pSavepoint && sqlStrICmp(pSavepoint->zName, zName); - pSavepoint = pSavepoint->pNext - ); - if (!pSavepoint) { + struct txn_savepoint *sv = txn_savepoint_by_name(txn, zName); + if (sv == NULL) { diag_set(ClientError, ER_NO_SUCH_SAVEPOINT); goto abort_due_to_error; } else { @@ -2867,8 +2859,7 @@ case OP_Savepoint: { * and this is a RELEASE command, then the current transaction * is committed. */ - int isTransaction = pSavepoint->pNext == 0; - if (isTransaction && p1==SAVEPOINT_RELEASE) { + if (p1 == SAVEPOINT_RELEASE) { if ((rc = sqlVdbeCheckFk(p, 1)) != 0) goto vdbe_return; sqlVdbeHalt(p); @@ -2876,19 +2867,8 @@ case OP_Savepoint: { goto abort_due_to_error; } else { if (p1==SAVEPOINT_ROLLBACK) - box_txn_rollback_to_savepoint(pSavepoint->tnt_savepoint); - } - - /* Regardless of whether this is a RELEASE or ROLLBACK, destroy all - * savepoints nested inside of the savepoint being operated on. - */ - while (psql_txn->pSavepoint != pSavepoint) { - pTmp = psql_txn->pSavepoint; - psql_txn->pSavepoint = pTmp->pNext; - /* - * Since savepoints are stored in region, we do not - * have to destroy them - */ + if (box_txn_rollback_to_savepoint(sv) != 0) + goto abort_due_to_error; } /* If it is a RELEASE, then destroy the savepoint being operated on @@ -2897,11 +2877,9 @@ case OP_Savepoint: { * when the savepoint was created. */ if (p1==SAVEPOINT_RELEASE) { - assert(pSavepoint == psql_txn->pSavepoint); - psql_txn->pSavepoint = pSavepoint->pNext; + txn_savepoint_release(sv); } else { - txn->fk_deferred_count = - pSavepoint->tnt_savepoint->fk_deferred_count; + txn->fk_deferred_count = sv->fk_deferred_count; } } } @@ -4843,7 +4821,6 @@ case OP_FkCounter: { if (((p->sql_flags & SQL_DeferFKs) != 0 || pOp->p1 != 0) && !p->auto_commit) { struct txn *txn = in_txn(); - assert(txn != NULL && txn->psql_txn != NULL); txn->fk_deferred_count += pOp->p2; } else { p->nFkConstraint += pOp->p2; @@ -4867,7 +4844,6 @@ case OP_FkIfZero: { /* jump */ if (((p->sql_flags & SQL_DeferFKs) != 0 || pOp->p1 != 0) && !p->auto_commit) { struct txn *txn = in_txn(); - assert(txn != NULL && txn->psql_txn != NULL); if (txn->fk_deferred_count == 0) goto jump_to_p2; } else { diff --git a/src/box/sql/vdbe.h b/src/box/sql/vdbe.h index 8f16202ba..06f258805 100644 --- a/src/box/sql/vdbe.h +++ b/src/box/sql/vdbe.h @@ -172,15 +172,6 @@ struct SubProgram { */ Vdbe *sqlVdbeCreate(Parse *); -/** - * Allocate and initialize SQL-specific struct which completes - * original Tarantool's txn struct using region allocator. - * - * @retval NULL on OOM, new psql_txn struct on success. - **/ -struct sql_txn * -sql_alloc_txn(); - /** * Prepare given VDBE to execution: initialize structs connected * with transaction routine: autocommit mode, deferred foreign diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c index bbbb99c70..3d256f86a 100644 --- a/src/box/sql/vdbeaux.c +++ b/src/box/sql/vdbeaux.c @@ -76,39 +76,12 @@ sqlVdbeCreate(Parse * pParse) return p; } -struct sql_txn * -sql_alloc_txn() -{ - struct sql_txn *txn = region_alloc_object(&fiber()->gc, - struct sql_txn); - if (txn == NULL) { - diag_set(OutOfMemory, sizeof(struct sql_txn), "region", - "struct sql_txn"); - return NULL; - } - txn->pSavepoint = NULL; - return txn; -} - int sql_vdbe_prepare(struct Vdbe *vdbe) { assert(vdbe != NULL); struct txn *txn = in_txn(); vdbe->auto_commit = txn == NULL; - if (txn != NULL) { - /* - * If transaction has been started in Lua, then - * sql_txn is NULL. On the other hand, it is not - * critical, since in Lua it is impossible to - * check FK violations, at least now. - */ - if (txn->psql_txn == NULL) { - txn->psql_txn = sql_alloc_txn(); - if (txn->psql_txn == NULL) - return -1; - } - } return 0; } @@ -2008,11 +1981,6 @@ sql_txn_begin() struct txn *ptxn = txn_begin(false); if (ptxn == NULL) return -1; - ptxn->psql_txn = sql_alloc_txn(); - if (ptxn->psql_txn == NULL) { - box_txn_rollback(); - return -1; - } return 0; } diff --git a/src/box/txn.c b/src/box/txn.c index b57240846..451cc9eb1 100644 --- a/src/box/txn.c +++ b/src/box/txn.c @@ -221,8 +221,8 @@ txn_begin() txn->signature = -1; txn->engine = NULL; txn->engine_tx = NULL; - txn->psql_txn = NULL; txn->fk_deferred_count = 0; + stailq_create(&txn->savepoints); txn->fiber = NULL; fiber_set_txn(fiber(), txn); /* fiber_on_yield is initialized by engine on demand */ @@ -734,28 +734,56 @@ box_txn_alloc(size_t size) alignof(union natural_align)); } -box_txn_savepoint_t * -box_txn_savepoint() +struct txn_savepoint * +txn_savepoint_new(struct txn *txn, const char *name) { - struct txn *txn = in_txn(); - if (txn == NULL) { - diag_set(ClientError, ER_NO_TRANSACTION); - return NULL; - } + assert(txn == in_txn()); + size_t svp_sz = sizeof(struct txn_savepoint); + svp_sz += name != NULL ? strlen(name) + 1 : 0; struct txn_savepoint *svp = - (struct txn_savepoint *) region_alloc_object(&txn->region, - struct txn_savepoint); + (struct txn_savepoint *) region_alloc(&txn->region, svp_sz); if (svp == NULL) { - diag_set(OutOfMemory, sizeof(*svp), - "region", "struct txn_savepoint"); + diag_set(OutOfMemory, svp_sz, "region", "svp"); return NULL; } svp->stmt = stailq_last(&txn->stmts); svp->in_sub_stmt = txn->in_sub_stmt; svp->fk_deferred_count = txn->fk_deferred_count; + if (name != NULL) { + svp->name = (char *) svp + sizeof(struct txn_savepoint); + memcpy(svp->name, name, strlen(name) + 1); + } else { + svp->name = NULL; + } + stailq_add_tail_entry(&txn->savepoints, svp, next); return svp; } +struct txn_savepoint * +txn_savepoint_by_name(struct txn *txn, const char *name) +{ + assert(txn == in_txn()); + struct txn_savepoint *sv = NULL; + stailq_foreach_entry(sv, &txn->savepoints, next) { + if (sv->name == NULL) + continue; + if (strcmp(sv->name, name) == 0) + return sv; + } + return NULL; +} + +box_txn_savepoint_t * +box_txn_savepoint() +{ + struct txn *txn = in_txn(); + if (txn == NULL) { + diag_set(ClientError, ER_NO_TRANSACTION); + return NULL; + } + return txn_savepoint_new(txn, NULL); +} + int box_txn_rollback_to_savepoint(box_txn_savepoint_t *svp) { @@ -779,10 +807,28 @@ box_txn_rollback_to_savepoint(box_txn_savepoint_t *svp) return -1; } txn_rollback_to_svp(txn, svp->stmt); + /* Discard from list all prior savepoints. */ + struct stailq discard; + stailq_cut_tail(&txn->savepoints, &svp->next, &discard); txn->fk_deferred_count = svp->fk_deferred_count; return 0; } +void +txn_savepoint_release(struct txn_savepoint *svp) +{ + struct txn *txn = in_txn(); + assert(txn != NULL); + /* + * Discard current savepoint alongside with all + * created after it savepoints. Note that operation + * is currently available only for SQL. + */ + struct stailq discard; + stailq_cut_tail(&txn->savepoints, &svp->next, &discard); + stailq_cut_tail(&txn->savepoints, *txn->savepoints.last, &discard); +} + static void txn_on_stop(struct trigger *trigger, void *event) { diff --git a/src/box/txn.h b/src/box/txn.h index 87e880a6a..a14f3871a 100644 --- a/src/box/txn.h +++ b/src/box/txn.h @@ -43,7 +43,6 @@ extern "C" { /** box statistics */ extern struct rmean *rmean_box; -struct Savepoint; struct engine; struct space; @@ -128,15 +127,12 @@ struct txn_savepoint { * state violating any number of deferred FK constraints. */ uint32_t fk_deferred_count; + struct stailq_entry next; + char *name; }; extern double too_long_threshold; -struct sql_txn { - /** List of active SQL savepoints. */ - struct Savepoint *pSavepoint; -}; - /** * An element of list of autogenerated ids, being returned as SQL * response metadata. @@ -209,7 +205,7 @@ struct txn { /** Commit and rollback triggers. */ struct rlist on_commit, on_rollback; uint32_t fk_deferred_count; - struct sql_txn *psql_txn; + struct stailq savepoints; }; static inline bool @@ -455,6 +451,22 @@ txn_current_stmt(struct txn *txn) return stailq_entry(stmt, struct txn_stmt, next); } +/** + * Allocate new savepoint object using region allocator. + * Savepoint is allowed to be anonymous (i.e. without + * name). + */ +struct txn_savepoint * +txn_savepoint_new(struct txn *txn, const char *name); + +/** Find savepoint by its name in savepoint list. */ +struct txn_savepoint * +txn_savepoint_by_name(struct txn *txn, const char *name); + +/** Remove given and all prior entries from savepoint list. */ +void +txn_savepoint_release(struct txn_savepoint *svp); + /** * FFI bindings: do not throw exceptions, do not accept extra * arguments -- 2.15.1