[tarantool-patches] Re: [PATCH] txn: convert txn flags into bit mask
Vladimir Davydov
vdavydov.dev at gmail.com
Tue Jul 30 10:58:57 MSK 2019
On Mon, Jul 29, 2019 at 06:37:54PM +0300, Konstantin Osipov wrote:
> * Vladimir Davydov <vdavydov.dev at gmail.com> [19/07/29 15:33]:
> > On Mon, Jul 29, 2019 at 03:14:11PM +0300, Konstantin Osipov wrote:
> > > * Vladimir Davydov <vdavydov.dev at gmail.com> [19/07/29 13:56]:
> > > > ---
> > > > https://github.com/tarantool/tarantool/commits/dv/txn-flags
> > >
> > > Curious, why not use bit fields?
>
> I will read the article, I do not have a strong opinion, it's just
> checking for bit flags using an enum looks a bit clumsy, how about
> a bit of syntax sugar like
> txn_has_flag(txn, FLAG), tnx_set_flag(txn, FLAG),
> txn_clear_flag(txn, FLAG).
>
> What do you think?
Sounds reasonable. Here you go:
>From 16610dedb84c78a96d720d7bb1b1b881f0fa9d11 Mon Sep 17 00:00:00 2001
From: Vladimir Davydov <vdavydov.dev at gmail.com>
Date: Mon, 29 Jul 2019 13:49:04 +0300
Subject: [PATCH] txn: convert txn flags into bit mask
diff --git a/src/box/txn.c b/src/box/txn.c
index c7d01b2e..05ec6f14 100644
--- a/src/box/txn.c
+++ b/src/box/txn.c
@@ -197,10 +197,7 @@ txn_begin()
txn->n_new_rows = 0;
txn->n_local_rows = 0;
txn->n_applier_rows = 0;
- txn->has_triggers = false;
- txn->is_done = false;
- txn->can_yield = true;
- txn->is_aborted_by_yield = false;
+ txn->flags = 0;
txn->in_sub_stmt = 0;
txn->id = ++tsn;
txn->signature = -1;
@@ -212,6 +209,12 @@ txn_begin()
/* 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);
+ /*
+ * By default all transactions may yield.
+ * It's a responsibility of an engine to disable yields
+ * if they are not supported.
+ */
+ txn_set_flag(txn, TXN_CAN_YIELD);
return txn;
}
@@ -396,13 +399,13 @@ txn_complete(struct txn *txn)
/* Undo the transaction. */
if (txn->engine)
engine_rollback(txn->engine, txn);
- if (txn->has_triggers)
+ if (txn_has_flag(txn, TXN_HAS_TRIGGERS))
txn_run_rollback_triggers(txn);
} else {
/* Commit the transaction. */
if (txn->engine != NULL)
engine_commit(txn->engine, txn);
- if (txn->has_triggers)
+ if (txn_has_flag(txn, TXN_HAS_TRIGGERS))
txn_run_commit_triggers(txn);
double stop_tm = ev_monotonic_now(loop());
@@ -422,7 +425,7 @@ txn_complete(struct txn *txn)
if (txn->fiber == NULL)
txn_free(txn);
else {
- txn->is_done = true;
+ txn_set_flag(txn, TXN_IS_DONE);
if (txn->fiber != fiber())
/* Wake a waiting fiber up. */
fiber_wakeup(txn->fiber);
@@ -491,8 +494,8 @@ txn_write_to_wal(struct txn *txn)
static int
txn_prepare(struct txn *txn)
{
- if (txn->is_aborted_by_yield) {
- assert(!txn->can_yield);
+ if (txn_has_flag(txn, TXN_IS_ABORTED_BY_YIELD)) {
+ assert(!txn_has_flag(txn, TXN_CAN_YIELD));
diag_set(ClientError, ER_TRANSACTION_YIELD);
diag_log();
return -1;
@@ -518,7 +521,7 @@ txn_prepare(struct txn *txn)
return -1;
}
trigger_clear(&txn->fiber_on_stop);
- if (!txn->can_yield)
+ if (!txn_has_flag(txn, TXN_CAN_YIELD))
trigger_clear(&txn->fiber_on_yield);
return 0;
}
@@ -558,7 +561,7 @@ txn_commit(struct txn *txn)
* In case of non-yielding journal the transaction could already
* be done and there is nothing to wait in such cases.
*/
- if (!txn->is_done) {
+ if (!txn_has_flag(txn, TXN_IS_DONE)) {
bool cancellable = fiber_set_cancellable(false);
fiber_yield();
fiber_set_cancellable(cancellable);
@@ -586,7 +589,7 @@ txn_rollback(struct txn *txn)
{
assert(txn == in_txn());
trigger_clear(&txn->fiber_on_stop);
- if (!txn->can_yield)
+ if (!txn_has_flag(txn, TXN_CAN_YIELD))
trigger_clear(&txn->fiber_on_yield);
txn->signature = -1;
txn_complete(txn);
@@ -607,11 +610,13 @@ void
txn_can_yield(struct txn *txn, bool set)
{
assert(txn == in_txn());
- assert(txn->can_yield != set);
- txn->can_yield = set;
if (set) {
+ assert(!txn_has_flag(txn, TXN_CAN_YIELD));
+ txn_set_flag(txn, TXN_CAN_YIELD);
trigger_clear(&txn->fiber_on_yield);
} else {
+ assert(txn_has_flag(txn, TXN_CAN_YIELD));
+ txn_clear_flag(txn, TXN_CAN_YIELD);
trigger_create(&txn->fiber_on_yield, txn_on_yield, NULL, NULL);
trigger_add(&fiber()->on_yield, &txn->fiber_on_yield);
}
@@ -781,11 +786,12 @@ txn_on_yield(struct trigger *trigger, void *event)
(void) trigger;
(void) event;
struct txn *txn = in_txn();
- assert(txn != NULL && !txn->can_yield);
+ assert(txn != NULL);
+ assert(!txn_has_flag(txn, TXN_CAN_YIELD));
txn_rollback_to_svp(txn, NULL);
- if (txn->has_triggers) {
+ if (txn_has_flag(txn, TXN_HAS_TRIGGERS)) {
txn_run_rollback_triggers(txn);
- txn->has_triggers = false;
+ txn_clear_flag(txn, TXN_HAS_TRIGGERS);
}
- txn->is_aborted_by_yield = true;
+ txn_set_flag(txn, TXN_IS_ABORTED_BY_YIELD);
}
diff --git a/src/box/txn.h b/src/box/txn.h
index 2d5dea21..0a5c5169 100644
--- a/src/box/txn.h
+++ b/src/box/txn.h
@@ -51,6 +51,23 @@ struct tuple;
struct xrow_header;
struct Vdbe;
+enum txn_flag {
+ /** Transaction has been processed. */
+ TXN_IS_DONE,
+ /**
+ * Transaction has been aborted by fiber yield so
+ * should be rolled back at commit.
+ */
+ TXN_IS_ABORTED_BY_YIELD,
+ /**
+ * fiber_yield() is allowed inside the transaction.
+ * See txn_can_yield() for more details.
+ */
+ TXN_CAN_YIELD,
+ /** on_commit and/or on_rollback list is not empty. */
+ TXN_HAS_TRIGGERS,
+};
+
enum {
/**
* Maximum recursion depth for on_replace triggers.
@@ -160,20 +177,8 @@ struct txn {
* already assigned LSN.
*/
int n_applier_rows;
- /* True when transaction is processed. */
- bool is_done;
- /**
- * True if the transaction was aborted so should be
- * rolled back at commit.
- */
- bool is_aborted_by_yield;
- /**
- * True if yields are allowed inside a transaction,
- * see txn_can_yield().
- */
- bool can_yield;
- /** True if on_commit and on_rollback lists are non-empty. */
- bool has_triggers;
+ /** Bit mask of transaction flags, see txn_flag. */
+ unsigned flags;
/** The number of active nested statement-level transactions. */
int8_t in_sub_stmt;
/**
@@ -206,6 +211,24 @@ struct txn {
struct sql_txn *psql_txn;
};
+static inline bool
+txn_has_flag(struct txn *txn, enum txn_flag flag)
+{
+ return (txn->flags & (1 << flag)) != 0;
+}
+
+static inline void
+txn_set_flag(struct txn *txn, enum txn_flag flag)
+{
+ txn->flags |= 1 << flag;
+}
+
+static inline void
+txn_clear_flag(struct txn *txn, enum txn_flag flag)
+{
+ txn->flags &= ~(1 << flag);
+}
+
/* Pointer to the current transaction (if any) */
static inline struct txn *
in_txn()
@@ -260,10 +283,10 @@ txn_write(struct txn *txn);
static inline void
txn_init_triggers(struct txn *txn)
{
- if (txn->has_triggers == false) {
+ if (!txn_has_flag(txn, TXN_HAS_TRIGGERS)) {
rlist_create(&txn->on_commit);
rlist_create(&txn->on_rollback);
- txn->has_triggers = true;
+ txn_set_flag(txn, TXN_HAS_TRIGGERS);
}
}
More information about the Tarantool-patches
mailing list