* [Tarantool-patches] [PATCH 01/14] box/txn: fix void args mess
2020-02-19 18:36 [Tarantool-patches] [PATCH 00/14] rework async and sync transactions Cyrill Gorcunov
@ 2020-02-19 18:37 ` Cyrill Gorcunov
2020-02-20 14:10 ` Nikita Pettik
2020-02-19 18:37 ` [Tarantool-patches] [PATCH 02/14] box/journal: use plain int for return value Cyrill Gorcunov
` (12 subsequent siblings)
13 siblings, 1 reply; 26+ messages in thread
From: Cyrill Gorcunov @ 2020-02-19 18:37 UTC (permalink / raw)
To: tml
Using void explicitly in functions which take
no arguments allows to optimize code a bit and
don't assume if there might be variable args.
Moreover in commit e070cc4d9 we dropped arguments
from txn_begin but didn't update vy_scheduler.c.
The compiler didn't complain because it assumed
there are vargs.
Acked-by: Konstantin Osipov <kostja.osipov@gmail.com>
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
src/box/txn.c | 14 +++++++-------
src/box/txn.h | 4 ++--
src/box/vy_scheduler.c | 2 +-
3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/src/box/txn.c b/src/box/txn.c
index 30ec586ba..5dadf4985 100644
--- a/src/box/txn.c
+++ b/src/box/txn.c
@@ -173,7 +173,7 @@ txn_rollback_to_svp(struct txn *txn, struct stailq_entry *svp)
* Return a txn from cache or create a new one if cache is empty.
*/
inline static struct txn *
-txn_new()
+txn_new(void)
{
if (!stailq_empty(&txn_cache))
return stailq_shift_entry(&txn_cache, struct txn, in_txn_cache);
@@ -209,7 +209,7 @@ txn_free(struct txn *txn)
}
struct txn *
-txn_begin()
+txn_begin(void)
{
static int64_t tsn = 0;
assert(! in_txn());
@@ -670,13 +670,13 @@ box_txn_id(void)
}
bool
-box_txn()
+box_txn(void)
{
return in_txn() != NULL;
}
int
-box_txn_begin()
+box_txn_begin(void)
{
if (in_txn()) {
diag_set(ClientError, ER_ACTIVE_TRANSACTION);
@@ -688,7 +688,7 @@ box_txn_begin()
}
int
-box_txn_commit()
+box_txn_commit(void)
{
struct txn *txn = in_txn();
/**
@@ -709,7 +709,7 @@ box_txn_commit()
}
int
-box_txn_rollback()
+box_txn_rollback(void)
{
struct txn *txn = in_txn();
if (txn == NULL)
@@ -788,7 +788,7 @@ txn_savepoint_by_name(struct txn *txn, const char *name)
}
box_txn_savepoint_t *
-box_txn_savepoint()
+box_txn_savepoint(void)
{
struct txn *txn = in_txn();
if (txn == NULL) {
diff --git a/src/box/txn.h b/src/box/txn.h
index 97d7b5de5..ae2c3a58f 100644
--- a/src/box/txn.h
+++ b/src/box/txn.h
@@ -251,7 +251,7 @@ txn_clear_flag(struct txn *txn, enum txn_flag flag)
/* Pointer to the current transaction (if any) */
static inline struct txn *
-in_txn()
+in_txn(void)
{
return fiber()->storage.txn;
}
@@ -261,7 +261,7 @@ in_txn()
* @pre no transaction is active
*/
struct txn *
-txn_begin();
+txn_begin(void);
/**
* Commit a transaction.
diff --git a/src/box/vy_scheduler.c b/src/box/vy_scheduler.c
index 86bed8013..acc909d09 100644
--- a/src/box/vy_scheduler.c
+++ b/src/box/vy_scheduler.c
@@ -895,7 +895,7 @@ vy_deferred_delete_batch_process_f(struct cmsg *cmsg)
deferred_delete_space = space_by_id(BOX_VINYL_DEFERRED_DELETE_ID);
assert(deferred_delete_space != NULL);
- struct txn *txn = txn_begin(false);
+ struct txn *txn = txn_begin();
if (txn == NULL)
goto fail;
--
2.20.1
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Tarantool-patches] [PATCH 01/14] box/txn: fix void args mess
2020-02-19 18:37 ` [Tarantool-patches] [PATCH 01/14] box/txn: fix void args mess Cyrill Gorcunov
@ 2020-02-20 14:10 ` Nikita Pettik
2020-02-20 14:16 ` Cyrill Gorcunov
0 siblings, 1 reply; 26+ messages in thread
From: Nikita Pettik @ 2020-02-20 14:10 UTC (permalink / raw)
To: Cyrill Gorcunov; +Cc: tml
On 19 Feb 21:37, Cyrill Gorcunov wrote:
> Using void explicitly in functions which take
> no arguments allows to optimize code a bit and
> don't assume if there might be variable args.
>
> Moreover in commit e070cc4d9 we dropped arguments
> from txn_begin but didn't update vy_scheduler.c.
> The compiler didn't complain because it assumed
> there are vargs.
>
> Acked-by: Konstantin Osipov <kostja.osipov@gmail.com>
> Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
LGTM. I assume you can push such patches aimed solely at refactoring
(taking into account second ack).
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Tarantool-patches] [PATCH 01/14] box/txn: fix void args mess
2020-02-20 14:10 ` Nikita Pettik
@ 2020-02-20 14:16 ` Cyrill Gorcunov
2020-02-20 14:34 ` Nikita Pettik
0 siblings, 1 reply; 26+ messages in thread
From: Cyrill Gorcunov @ 2020-02-20 14:16 UTC (permalink / raw)
To: Nikita Pettik; +Cc: tml
On Thu, Feb 20, 2020 at 05:10:48PM +0300, Nikita Pettik wrote:
> On 19 Feb 21:37, Cyrill Gorcunov wrote:
> > Using void explicitly in functions which take
> > no arguments allows to optimize code a bit and
> > don't assume if there might be variable args.
> >
> > Moreover in commit e070cc4d9 we dropped arguments
> > from txn_begin but didn't update vy_scheduler.c.
> > The compiler didn't complain because it assumed
> > there are vargs.
> >
> > Acked-by: Konstantin Osipov <kostja.osipov@gmail.com>
> > Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
>
> LGTM. I assume you can push such patches aimed solely at refactoring
> (taking into account second ack).
Thanks, the first 4 patches are just namings and ect, no
functional changes. Actually I've prepared the branch
for this and gave it to Kirill. Letme talk to him,
I think I'll push myself and add your Acks as well.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Tarantool-patches] [PATCH 01/14] box/txn: fix void args mess
2020-02-20 14:16 ` Cyrill Gorcunov
@ 2020-02-20 14:34 ` Nikita Pettik
0 siblings, 0 replies; 26+ messages in thread
From: Nikita Pettik @ 2020-02-20 14:34 UTC (permalink / raw)
To: Cyrill Gorcunov; +Cc: tml
On 20 Feb 17:16, Cyrill Gorcunov wrote:
> On Thu, Feb 20, 2020 at 05:10:48PM +0300, Nikita Pettik wrote:
> > On 19 Feb 21:37, Cyrill Gorcunov wrote:
> > > Using void explicitly in functions which take
> > > no arguments allows to optimize code a bit and
> > > don't assume if there might be variable args.
> > >
> > > Moreover in commit e070cc4d9 we dropped arguments
> > > from txn_begin but didn't update vy_scheduler.c.
> > > The compiler didn't complain because it assumed
> > > there are vargs.
> > >
> > > Acked-by: Konstantin Osipov <kostja.osipov@gmail.com>
> > > Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
> >
> > LGTM. I assume you can push such patches aimed solely at refactoring
> > (taking into account second ack).
>
> Thanks, the first 4 patches are just namings and ect, no
> functional changes. Actually I've prepared the branch
> for this and gave it to Kirill. Letme talk to him,
> I think I'll push myself and add your Acks as well.
Pushed to master (first 4 patches).
^ permalink raw reply [flat|nested] 26+ messages in thread
* [Tarantool-patches] [PATCH 02/14] box/journal: use plain int for return value
2020-02-19 18:36 [Tarantool-patches] [PATCH 00/14] rework async and sync transactions Cyrill Gorcunov
2020-02-19 18:37 ` [Tarantool-patches] [PATCH 01/14] box/txn: fix void args mess Cyrill Gorcunov
@ 2020-02-19 18:37 ` Cyrill Gorcunov
2020-02-20 14:11 ` Nikita Pettik
2020-02-19 18:37 ` [Tarantool-patches] [PATCH 03/14] box/journal: sanitize completion naming Cyrill Gorcunov
` (11 subsequent siblings)
13 siblings, 1 reply; 26+ messages in thread
From: Cyrill Gorcunov @ 2020-02-19 18:37 UTC (permalink / raw)
To: tml
We're returning int64_t with values 0 or -1 by now,
there is no need for such big return type, plain
integer is enough.
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
src/box/box.cc | 2 +-
src/box/journal.c | 2 +-
src/box/journal.h | 8 ++++----
src/box/wal.c | 8 ++++----
4 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/src/box/box.cc b/src/box/box.cc
index 1b2b27d61..9e8311d1e 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -310,7 +310,7 @@ struct recovery_journal {
* exact same signature during local recovery to properly mark
* min/max LSN of created LSM levels.
*/
-static int64_t
+static int
recovery_journal_write(struct journal *base,
struct journal_entry *entry)
{
diff --git a/src/box/journal.c b/src/box/journal.c
index d762613dd..cf6485642 100644
--- a/src/box/journal.c
+++ b/src/box/journal.c
@@ -36,7 +36,7 @@
* Used to load from a memtx snapshot. LSN is not used,
* but txn_commit() must work.
*/
-static int64_t
+static int
dummy_journal_write(struct journal *journal, struct journal_entry *entry)
{
(void) journal;
diff --git a/src/box/journal.h b/src/box/journal.h
index 236058bb1..55900d8e5 100644
--- a/src/box/journal.h
+++ b/src/box/journal.h
@@ -112,8 +112,8 @@ journal_entry_complete(struct journal_entry *entry)
* synchronous replication.
*/
struct journal {
- int64_t (*write)(struct journal *journal,
- struct journal_entry *req);
+ int (*write)(struct journal *journal,
+ struct journal_entry *req);
void (*destroy)(struct journal *journal);
};
@@ -128,7 +128,7 @@ extern struct journal *current_journal;
*
* @return 0 if write was scheduled or -1 in case of an error.
*/
-static inline int64_t
+static inline int
journal_write(struct journal_entry *entry)
{
return current_journal->write(current_journal, entry);
@@ -165,7 +165,7 @@ journal_set(struct journal *new_journal)
static inline void
journal_create(struct journal *journal,
- int64_t (*write)(struct journal *, struct journal_entry *),
+ int (*write)(struct journal *, struct journal_entry *),
void (*destroy)(struct journal *))
{
journal->write = write;
diff --git a/src/box/wal.c b/src/box/wal.c
index 0ae66ff32..ac977c16e 100644
--- a/src/box/wal.c
+++ b/src/box/wal.c
@@ -60,10 +60,10 @@ const char *wal_mode_STRS[] = { "none", "write", "fsync", NULL };
int wal_dir_lock = -1;
-static int64_t
+static int
wal_write(struct journal *, struct journal_entry *);
-static int64_t
+static int
wal_write_in_wal_mode_none(struct journal *, struct journal_entry *);
/*
@@ -1157,7 +1157,7 @@ wal_writer_f(va_list ap)
* WAL writer main entry point: queue a single request
* to be written to disk.
*/
-static int64_t
+static int
wal_write(struct journal *journal, struct journal_entry *entry)
{
struct wal_writer *writer = (struct wal_writer *) journal;
@@ -1213,7 +1213,7 @@ wal_write(struct journal *journal, struct journal_entry *entry)
return -1;
}
-static int64_t
+static int
wal_write_in_wal_mode_none(struct journal *journal,
struct journal_entry *entry)
{
--
2.20.1
^ permalink raw reply [flat|nested] 26+ messages in thread
* [Tarantool-patches] [PATCH 03/14] box/journal: sanitize completion naming
2020-02-19 18:36 [Tarantool-patches] [PATCH 00/14] rework async and sync transactions Cyrill Gorcunov
2020-02-19 18:37 ` [Tarantool-patches] [PATCH 01/14] box/txn: fix void args mess Cyrill Gorcunov
2020-02-19 18:37 ` [Tarantool-patches] [PATCH 02/14] box/journal: use plain int for return value Cyrill Gorcunov
@ 2020-02-19 18:37 ` Cyrill Gorcunov
2020-02-20 14:12 ` Nikita Pettik
2020-02-19 18:37 ` [Tarantool-patches] [PATCH 04/14] box/txn: rename txn_entry_done_cb to txn_entry_complete_cb Cyrill Gorcunov
` (10 subsequent siblings)
13 siblings, 1 reply; 26+ messages in thread
From: Cyrill Gorcunov @ 2020-02-19 18:37 UTC (permalink / raw)
To: tml
Instead of on_done use on_complete prefix
since done it too general while we're trying
to complete write procedue. Also it is more
consistent with txn_complete name.
Acked-by: Konstantin Osipov <kostja.osipov@gmail.com>
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
src/box/journal.c | 8 ++++----
src/box/journal.h | 12 ++++++------
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/src/box/journal.c b/src/box/journal.c
index cf6485642..11e78990d 100644
--- a/src/box/journal.c
+++ b/src/box/journal.c
@@ -54,8 +54,8 @@ struct journal *current_journal = &dummy_journal;
struct journal_entry *
journal_entry_new(size_t n_rows, struct region *region,
- journal_entry_done_cb on_done_cb,
- void *on_done_cb_data)
+ journal_entry_complete_cb on_complete_cb,
+ void *on_complete_cb_data)
{
struct journal_entry *entry;
@@ -71,8 +71,8 @@ journal_entry_new(size_t n_rows, struct region *region,
entry->approx_len = 0;
entry->n_rows = n_rows;
entry->res = -1;
- entry->on_done_cb = on_done_cb;
- entry->on_done_cb_data = on_done_cb_data;
+ entry->on_complete_cb = on_complete_cb;
+ entry->on_complete_cb_data = on_complete_cb_data;
return entry;
}
diff --git a/src/box/journal.h b/src/box/journal.h
index 55900d8e5..64f167c6f 100644
--- a/src/box/journal.h
+++ b/src/box/journal.h
@@ -43,7 +43,7 @@ struct xrow_header;
struct journal_entry;
/** Journal entry finalization callback typedef. */
-typedef void (*journal_entry_done_cb)(struct journal_entry *entry, void *data);
+typedef void (*journal_entry_complete_cb)(struct journal_entry *entry, void *data);
/**
* An entry for an abstract journal.
@@ -66,11 +66,11 @@ struct journal_entry {
* or fail. Entry->res is set to a result value before the callback
* is fired.
*/
- journal_entry_done_cb on_done_cb;
+ journal_entry_complete_cb on_complete_cb;
/**
* A journal entry completion callback argument.
*/
- void *on_done_cb_data;
+ void *on_complete_cb_data;
/**
* Approximate size of this request when encoded.
*/
@@ -94,8 +94,8 @@ struct region;
*/
struct journal_entry *
journal_entry_new(size_t n_rows, struct region *region,
- journal_entry_done_cb on_done_cb,
- void *on_done_cb_data);
+ journal_entry_complete_cb on_complete_cb,
+ void *on_complete_cb_data);
/**
* Finalize a single entry.
@@ -103,7 +103,7 @@ journal_entry_new(size_t n_rows, struct region *region,
static inline void
journal_entry_complete(struct journal_entry *entry)
{
- entry->on_done_cb(entry, entry->on_done_cb_data);
+ entry->on_complete_cb(entry, entry->on_complete_cb_data);
}
/**
--
2.20.1
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Tarantool-patches] [PATCH 03/14] box/journal: sanitize completion naming
2020-02-19 18:37 ` [Tarantool-patches] [PATCH 03/14] box/journal: sanitize completion naming Cyrill Gorcunov
@ 2020-02-20 14:12 ` Nikita Pettik
2020-02-20 14:15 ` Nikita Pettik
0 siblings, 1 reply; 26+ messages in thread
From: Nikita Pettik @ 2020-02-20 14:12 UTC (permalink / raw)
To: Cyrill Gorcunov; +Cc: tml
On 19 Feb 21:37, Cyrill Gorcunov wrote:
> Instead of on_done use on_complete prefix
> since done it too general while we're trying
> to complete write procedue. Also it is more
> consistent with txn_complete name.
>
> Acked-by: Konstantin Osipov <kostja.osipov@gmail.com>
> Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
> ---
> src/box/journal.c | 8 ++++----
> src/box/journal.h | 12 ++++++------
> 2 files changed, 10 insertions(+), 10 deletions(-)
>
LGTM
Btw why not to do the same with txn_entry_done_cb ?
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Tarantool-patches] [PATCH 03/14] box/journal: sanitize completion naming
2020-02-20 14:12 ` Nikita Pettik
@ 2020-02-20 14:15 ` Nikita Pettik
0 siblings, 0 replies; 26+ messages in thread
From: Nikita Pettik @ 2020-02-20 14:15 UTC (permalink / raw)
To: Cyrill Gorcunov; +Cc: tml
On 20 Feb 17:12, Nikita Pettik wrote:
> On 19 Feb 21:37, Cyrill Gorcunov wrote:
> > Instead of on_done use on_complete prefix
> > since done it too general while we're trying
> > to complete write procedue. Also it is more
> > consistent with txn_complete name.
> >
> > Acked-by: Konstantin Osipov <kostja.osipov@gmail.com>
> > Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
> > ---
> > src/box/journal.c | 8 ++++----
> > src/box/journal.h | 12 ++++++------
> > 2 files changed, 10 insertions(+), 10 deletions(-)
> >
>
> LGTM
>
> Btw why not to do the same with txn_entry_done_cb ?
>
Nevermind, it is done in the next patch (I've accidentally skipped it).
^ permalink raw reply [flat|nested] 26+ messages in thread
* [Tarantool-patches] [PATCH 04/14] box/txn: rename txn_entry_done_cb to txn_entry_complete_cb
2020-02-19 18:36 [Tarantool-patches] [PATCH 00/14] rework async and sync transactions Cyrill Gorcunov
` (2 preceding siblings ...)
2020-02-19 18:37 ` [Tarantool-patches] [PATCH 03/14] box/journal: sanitize completion naming Cyrill Gorcunov
@ 2020-02-19 18:37 ` Cyrill Gorcunov
2020-02-20 14:15 ` Nikita Pettik
2020-02-19 18:37 ` [Tarantool-patches] [PATCH 05/14] box/txn: rename txn_write_to_wal to txn_write_to_wal_async Cyrill Gorcunov
` (9 subsequent siblings)
13 siblings, 1 reply; 26+ messages in thread
From: Cyrill Gorcunov @ 2020-02-19 18:37 UTC (permalink / raw)
To: tml
To look similar to txn_complete.
Acked-by: Konstantin Osipov <kostja.osipov@gmail.com>
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
src/box/txn.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/box/txn.c b/src/box/txn.c
index 5dadf4985..a4ca48224 100644
--- a/src/box/txn.c
+++ b/src/box/txn.c
@@ -469,7 +469,7 @@ txn_complete(struct txn *txn)
}
static void
-txn_entry_done_cb(struct journal_entry *entry, void *data)
+txn_entry_complete_cb(struct journal_entry *entry, void *data)
{
struct txn *txn = data;
txn->signature = entry->res;
@@ -493,7 +493,7 @@ txn_write_to_wal(struct txn *txn)
struct journal_entry *req = journal_entry_new(txn->n_new_rows +
txn->n_applier_rows,
&txn->region,
- txn_entry_done_cb,
+ txn_entry_complete_cb,
txn);
if (req == NULL) {
txn_rollback(txn);
--
2.20.1
^ permalink raw reply [flat|nested] 26+ messages in thread
* [Tarantool-patches] [PATCH 05/14] box/txn: rename txn_write_to_wal to txn_write_to_wal_async
2020-02-19 18:36 [Tarantool-patches] [PATCH 00/14] rework async and sync transactions Cyrill Gorcunov
` (3 preceding siblings ...)
2020-02-19 18:37 ` [Tarantool-patches] [PATCH 04/14] box/txn: rename txn_entry_done_cb to txn_entry_complete_cb Cyrill Gorcunov
@ 2020-02-19 18:37 ` Cyrill Gorcunov
2020-02-19 18:37 ` [Tarantool-patches] [PATCH 06/14] box/journal: supersede journal_write with journal_write_async Cyrill Gorcunov
` (8 subsequent siblings)
13 siblings, 0 replies; 26+ messages in thread
From: Cyrill Gorcunov @ 2020-02-19 18:37 UTC (permalink / raw)
To: tml
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
src/box/txn.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/box/txn.c b/src/box/txn.c
index a4ca48224..682480c16 100644
--- a/src/box/txn.c
+++ b/src/box/txn.c
@@ -485,7 +485,7 @@ txn_entry_complete_cb(struct journal_entry *entry, void *data)
}
static int64_t
-txn_write_to_wal(struct txn *txn)
+txn_write_to_wal_async(struct txn *txn)
{
assert(txn->n_new_rows + txn->n_applier_rows > 0);
@@ -584,7 +584,7 @@ txn_write(struct txn *txn)
return 0;
}
fiber_set_txn(fiber(), NULL);
- return txn_write_to_wal(txn);
+ return txn_write_to_wal_async(txn);
}
int
--
2.20.1
^ permalink raw reply [flat|nested] 26+ messages in thread
* [Tarantool-patches] [PATCH 06/14] box/journal: supersede journal_write with journal_write_async
2020-02-19 18:36 [Tarantool-patches] [PATCH 00/14] rework async and sync transactions Cyrill Gorcunov
` (4 preceding siblings ...)
2020-02-19 18:37 ` [Tarantool-patches] [PATCH 05/14] box/txn: rename txn_write_to_wal to txn_write_to_wal_async Cyrill Gorcunov
@ 2020-02-19 18:37 ` Cyrill Gorcunov
2020-02-19 18:37 ` [Tarantool-patches] [PATCH 07/14] box/txn: rename txn_write to txn_commit_async Cyrill Gorcunov
` (7 subsequent siblings)
13 siblings, 0 replies; 26+ messages in thread
From: Cyrill Gorcunov @ 2020-02-19 18:37 UTC (permalink / raw)
To: tml
1) name it journal_write_async, since it operates in async
mode where completion handlers are deferred upon journal
engine does a real write;
2) require a caller to ship a fiber with active transaction
bound so we could restore it back if write operation failed
thus rollback will obtain consistent active transaction;
3) move fiber_set_txn to the header file for reuse in
journal_write_async.
Suggested-by: Konstantin Osipov <kostja.osipov@gmail.com>
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
src/box/journal.c | 22 ++++++++++++++++++++++
src/box/journal.h | 9 +++------
src/box/txn.c | 9 +--------
src/box/txn.h | 9 +++++++++
4 files changed, 35 insertions(+), 14 deletions(-)
diff --git a/src/box/journal.c b/src/box/journal.c
index 11e78990d..238860165 100644
--- a/src/box/journal.c
+++ b/src/box/journal.c
@@ -29,6 +29,8 @@
* SUCH DAMAGE.
*/
#include "journal.h"
+#include "fiber.h"
+#include "txn.h"
#include <small/region.h>
#include <diag.h>
@@ -76,3 +78,23 @@ journal_entry_new(size_t n_rows, struct region *region,
return entry;
}
+int
+journal_write_async(struct journal_entry *entry)
+{
+ struct txn *txn = in_txn();
+ assert(txn != NULL);
+
+ /*
+ * The transaction should be unbound
+ * from current fiber, once operation
+ * is complete the journal engine will
+ * bound it back. For rollback sake we
+ * bind it by self on error path.
+ */
+ fiber_set_txn(fiber(), NULL);
+ int ret = current_journal->write(current_journal, entry);
+ if (ret != 0)
+ fiber_set_txn(fiber(), txn);
+
+ return ret;
+}
diff --git a/src/box/journal.h b/src/box/journal.h
index 64f167c6f..efb6ac8e1 100644
--- a/src/box/journal.h
+++ b/src/box/journal.h
@@ -124,15 +124,12 @@ struct journal {
extern struct journal *current_journal;
/**
- * Send a single entry to write.
+ * Queue a single entry to write.
*
* @return 0 if write was scheduled or -1 in case of an error.
*/
-static inline int
-journal_write(struct journal_entry *entry)
-{
- return current_journal->write(current_journal, entry);
-}
+extern int
+journal_write_async(struct journal_entry *entry);
/**
* Change the current implementation of the journaling API.
diff --git a/src/box/txn.c b/src/box/txn.c
index 682480c16..05321e97d 100644
--- a/src/box/txn.c
+++ b/src/box/txn.c
@@ -49,12 +49,6 @@ txn_on_yield(struct trigger *trigger, void *event);
static void
txn_run_rollback_triggers(struct txn *txn, struct rlist *triggers);
-static inline void
-fiber_set_txn(struct fiber *fiber, struct txn *txn)
-{
- fiber->storage.txn = txn;
-}
-
static int
txn_add_redo(struct txn *txn, struct txn_stmt *stmt, struct request *request)
{
@@ -520,7 +514,7 @@ txn_write_to_wal_async(struct txn *txn)
assert(local_row == remote_row + txn->n_new_rows);
/* Send the entry to the journal. */
- if (journal_write(req) < 0) {
+ if (journal_write_async(req) < 0) {
diag_set(ClientError, ER_WAL_IO);
diag_log();
return -1;
@@ -583,7 +577,6 @@ txn_write(struct txn *txn)
fiber_set_txn(fiber(), NULL);
return 0;
}
- fiber_set_txn(fiber(), NULL);
return txn_write_to_wal_async(txn);
}
diff --git a/src/box/txn.h b/src/box/txn.h
index ae2c3a58f..fe13ef5f8 100644
--- a/src/box/txn.h
+++ b/src/box/txn.h
@@ -256,6 +256,15 @@ in_txn(void)
return fiber()->storage.txn;
}
+/**
+ * Set pointer to the current transaction (if any).
+ */
+static inline void
+fiber_set_txn(struct fiber *fiber, struct txn *txn)
+{
+ fiber->storage.txn = txn;
+}
+
/**
* Start a transaction explicitly.
* @pre no transaction is active
--
2.20.1
^ permalink raw reply [flat|nested] 26+ messages in thread
* [Tarantool-patches] [PATCH 07/14] box/txn: rename txn_write to txn_commit_async
2020-02-19 18:36 [Tarantool-patches] [PATCH 00/14] rework async and sync transactions Cyrill Gorcunov
` (5 preceding siblings ...)
2020-02-19 18:37 ` [Tarantool-patches] [PATCH 06/14] box/journal: supersede journal_write with journal_write_async Cyrill Gorcunov
@ 2020-02-19 18:37 ` Cyrill Gorcunov
2020-02-22 20:28 ` Georgy Kirichenko
2020-02-19 18:37 ` [Tarantool-patches] [PATCH 08/14] box/txn: move setup of transaction start time to txn_prepare Cyrill Gorcunov
` (6 subsequent siblings)
13 siblings, 1 reply; 26+ messages in thread
From: Cyrill Gorcunov @ 2020-02-19 18:37 UTC (permalink / raw)
To: tml
Plain txn_write is too general.
Suggested-by: Konstantin Osipov <kostja.osipov@gmail.com>
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
src/box/applier.cc | 2 +-
src/box/txn.c | 4 ++--
src/box/txn.h | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/box/applier.cc b/src/box/applier.cc
index ae3d281a5..10859a835 100644
--- a/src/box/applier.cc
+++ b/src/box/applier.cc
@@ -805,7 +805,7 @@ applier_apply_tx(struct stailq *rows)
trigger_create(on_commit, applier_txn_commit_cb, NULL, NULL);
txn_on_commit(txn, on_commit);
- if (txn_write(txn) < 0)
+ if (txn_commit_async(txn) < 0)
goto fail;
/* Transaction was sent to journal so promote vclock. */
diff --git a/src/box/txn.c b/src/box/txn.c
index 05321e97d..0ad596c1e 100644
--- a/src/box/txn.c
+++ b/src/box/txn.c
@@ -558,7 +558,7 @@ txn_prepare(struct txn *txn)
}
int
-txn_write(struct txn *txn)
+txn_commit_async(struct txn *txn)
{
if (txn_prepare(txn) != 0) {
txn_rollback(txn);
@@ -585,7 +585,7 @@ txn_commit(struct txn *txn)
{
txn->fiber = fiber();
- if (txn_write(txn) != 0)
+ if (txn_commit_async(txn) != 0)
return -1;
/*
* In case of non-yielding journal the transaction could already
diff --git a/src/box/txn.h b/src/box/txn.h
index fe13ef5f8..f2e0fa9e7 100644
--- a/src/box/txn.h
+++ b/src/box/txn.h
@@ -302,7 +302,7 @@ txn_rollback(struct txn *txn);
* freed.
*/
int
-txn_write(struct txn *txn);
+txn_commit_async(struct txn *txn);
/**
* Most txns don't have triggers, and txn objects
--
2.20.1
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Tarantool-patches] [PATCH 07/14] box/txn: rename txn_write to txn_commit_async
2020-02-19 18:37 ` [Tarantool-patches] [PATCH 07/14] box/txn: rename txn_write to txn_commit_async Cyrill Gorcunov
@ 2020-02-22 20:28 ` Georgy Kirichenko
2020-02-22 21:00 ` Cyrill Gorcunov
0 siblings, 1 reply; 26+ messages in thread
From: Georgy Kirichenko @ 2020-02-22 20:28 UTC (permalink / raw)
To: tml
[-- Attachment #1: Type: text/plain, Size: 2196 bytes --]
Hi!
Well done but I have a minor remark about some renames you suggest.
So if we would like to talk about synchronous replication then renaming
txn_write to txn_commit seems quite incorrect as there would be two stages
of transaction commit - transaction was written to journal and then
transaction was completed(with commit or rollback). And the stages should be
distinguishable.
Obviously it depends on further implementation so I let the final resolution up
to you.
On Wednesday, February 19, 2020 9:37:06 PM MSK Cyrill Gorcunov wrote:
> Plain txn_write is too general.
>
> Suggested-by: Konstantin Osipov <kostja.osipov@gmail.com>
> Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
> ---
> src/box/applier.cc | 2 +-
> src/box/txn.c | 4 ++--
> src/box/txn.h | 2 +-
> 3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/src/box/applier.cc b/src/box/applier.cc
> index ae3d281a5..10859a835 100644
> --- a/src/box/applier.cc
> +++ b/src/box/applier.cc
> @@ -805,7 +805,7 @@ applier_apply_tx(struct stailq *rows)
> trigger_create(on_commit, applier_txn_commit_cb, NULL, NULL);
> txn_on_commit(txn, on_commit);
>
> - if (txn_write(txn) < 0)
> + if (txn_commit_async(txn) < 0)
> goto fail;
>
> /* Transaction was sent to journal so promote vclock. */
> diff --git a/src/box/txn.c b/src/box/txn.c
> index 05321e97d..0ad596c1e 100644
> --- a/src/box/txn.c
> +++ b/src/box/txn.c
> @@ -558,7 +558,7 @@ txn_prepare(struct txn *txn)
> }
>
> int
> -txn_write(struct txn *txn)
> +txn_commit_async(struct txn *txn)
> {
> if (txn_prepare(txn) != 0) {
> txn_rollback(txn);
> @@ -585,7 +585,7 @@ txn_commit(struct txn *txn)
> {
> txn->fiber = fiber();
>
> - if (txn_write(txn) != 0)
> + if (txn_commit_async(txn) != 0)
> return -1;
> /*
> * In case of non-yielding journal the transaction could already
> diff --git a/src/box/txn.h b/src/box/txn.h
> index fe13ef5f8..f2e0fa9e7 100644
> --- a/src/box/txn.h
> +++ b/src/box/txn.h
> @@ -302,7 +302,7 @@ txn_rollback(struct txn *txn);
> * freed.
> */
> int
> -txn_write(struct txn *txn);
> +txn_commit_async(struct txn *txn);
>
> /**
> * Most txns don't have triggers, and txn objects
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Tarantool-patches] [PATCH 07/14] box/txn: rename txn_write to txn_commit_async
2020-02-22 20:28 ` Georgy Kirichenko
@ 2020-02-22 21:00 ` Cyrill Gorcunov
0 siblings, 0 replies; 26+ messages in thread
From: Cyrill Gorcunov @ 2020-02-22 21:00 UTC (permalink / raw)
To: Georgy Kirichenko; +Cc: tml
On Sat, Feb 22, 2020 at 11:28:41PM +0300, Georgy Kirichenko wrote:
> Hi!
>
> Well done but I have a minor remark about some renames you suggest.
> So if we would like to talk about synchronous replication then renaming
> txn_write to txn_commit seems quite incorrect as there would be two stages
> of transaction commit - transaction was written to journal and then
> transaction was completed(with commit or rollback). And the stages should be
> distinguishable.
> Obviously it depends on further implementation so I let the final resolution up
> to you.
Thanks! You know at first I need to finish the series and have
everything working and passing tests, so I'll resend the updated
series and then we will discuss the better naming, sounds good?
^ permalink raw reply [flat|nested] 26+ messages in thread
* [Tarantool-patches] [PATCH 08/14] box/txn: move setup of transaction start time to txn_prepare
2020-02-19 18:36 [Tarantool-patches] [PATCH 00/14] rework async and sync transactions Cyrill Gorcunov
` (6 preceding siblings ...)
2020-02-19 18:37 ` [Tarantool-patches] [PATCH 07/14] box/txn: rename txn_write to txn_commit_async Cyrill Gorcunov
@ 2020-02-19 18:37 ` Cyrill Gorcunov
2020-02-19 18:37 ` [Tarantool-patches] [PATCH 09/14] box/txn: make txn nop processing a separate routine Cyrill Gorcunov
` (5 subsequent siblings)
13 siblings, 0 replies; 26+ messages in thread
From: Cyrill Gorcunov @ 2020-02-19 18:37 UTC (permalink / raw)
To: tml
It is more logical to put this timestamp in prepare
state. Also this will allow us to reuse the txn_prepare
code.
Note the former code did the same -- this member get
set right after txn_prepare call so nothing changed.
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
src/box/txn.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/box/txn.c b/src/box/txn.c
index 0ad596c1e..3a784b18b 100644
--- a/src/box/txn.c
+++ b/src/box/txn.c
@@ -554,6 +554,8 @@ txn_prepare(struct txn *txn)
trigger_clear(&txn->fiber_on_stop);
if (!txn_has_flag(txn, TXN_CAN_YIELD))
trigger_clear(&txn->fiber_on_yield);
+
+ txn->start_tm = ev_monotonic_now(loop());
return 0;
}
@@ -569,7 +571,6 @@ txn_commit_async(struct txn *txn)
* After this point the transaction must not be used
* so reset the corresponding key in the fiber storage.
*/
- txn->start_tm = ev_monotonic_now(loop());
if (txn->n_new_rows + txn->n_applier_rows == 0) {
/* Nothing to do. */
txn->signature = 0;
--
2.20.1
^ permalink raw reply [flat|nested] 26+ messages in thread
* [Tarantool-patches] [PATCH 09/14] box/txn: make txn nop processing a separate routine
2020-02-19 18:36 [Tarantool-patches] [PATCH 00/14] rework async and sync transactions Cyrill Gorcunov
` (7 preceding siblings ...)
2020-02-19 18:37 ` [Tarantool-patches] [PATCH 08/14] box/txn: move setup of transaction start time to txn_prepare Cyrill Gorcunov
@ 2020-02-19 18:37 ` Cyrill Gorcunov
2020-02-19 18:37 ` [Tarantool-patches] [PATCH 10/14] box/txn: move journal entry allocation into " Cyrill Gorcunov
` (4 subsequent siblings)
13 siblings, 0 replies; 26+ messages in thread
From: Cyrill Gorcunov @ 2020-02-19 18:37 UTC (permalink / raw)
To: tml
To reuse in sync commits in next patches.
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
src/box/txn.c | 28 ++++++++++++++++++----------
1 file changed, 18 insertions(+), 10 deletions(-)
diff --git a/src/box/txn.c b/src/box/txn.c
index 3a784b18b..a6a0704c8 100644
--- a/src/box/txn.c
+++ b/src/box/txn.c
@@ -462,6 +462,22 @@ txn_complete(struct txn *txn)
}
}
+/**
+ * Try to complete transaction early if there is
+ * nothing to be sent to the journal engine.
+ */
+static bool
+txn_complete_nop(struct txn *txn)
+{
+ if (txn->n_new_rows + txn->n_applier_rows != 0)
+ return false;
+
+ txn->signature = 0;
+ txn_complete(txn);
+ fiber_set_txn(fiber(), NULL);
+ return 0;
+}
+
static void
txn_entry_complete_cb(struct journal_entry *entry, void *data)
{
@@ -567,17 +583,9 @@ txn_commit_async(struct txn *txn)
return -1;
}
- /*
- * After this point the transaction must not be used
- * so reset the corresponding key in the fiber storage.
- */
- if (txn->n_new_rows + txn->n_applier_rows == 0) {
- /* Nothing to do. */
- txn->signature = 0;
- txn_complete(txn);
- fiber_set_txn(fiber(), NULL);
+ if (txn_complete_nop(txn))
return 0;
- }
+
return txn_write_to_wal_async(txn);
}
--
2.20.1
^ permalink raw reply [flat|nested] 26+ messages in thread
* [Tarantool-patches] [PATCH 10/14] box/txn: move journal entry allocation into separate routine
2020-02-19 18:36 [Tarantool-patches] [PATCH 00/14] rework async and sync transactions Cyrill Gorcunov
` (8 preceding siblings ...)
2020-02-19 18:37 ` [Tarantool-patches] [PATCH 09/14] box/txn: make txn nop processing a separate routine Cyrill Gorcunov
@ 2020-02-19 18:37 ` Cyrill Gorcunov
2020-02-19 18:37 ` [Tarantool-patches] [PATCH 11/14] box/txn: merge txn_write_to_wal_async to txn_commit_async Cyrill Gorcunov
` (3 subsequent siblings)
13 siblings, 0 replies; 26+ messages in thread
From: Cyrill Gorcunov @ 2020-02-19 18:37 UTC (permalink / raw)
To: tml
For reuse in next patches.
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
src/box/txn.c | 44 ++++++++++++++++++++++++++++++--------------
1 file changed, 30 insertions(+), 14 deletions(-)
diff --git a/src/box/txn.c b/src/box/txn.c
index a6a0704c8..4dbaf5b91 100644
--- a/src/box/txn.c
+++ b/src/box/txn.c
@@ -494,41 +494,57 @@ txn_entry_complete_cb(struct journal_entry *entry, void *data)
fiber_set_txn(fiber(), NULL);
}
-static int64_t
-txn_write_to_wal_async(struct txn *txn)
+/**
+ * Allocate and prepare new journal entry.
+ */
+static struct journal_entry *
+txn_entry_new(struct txn *txn)
{
assert(txn->n_new_rows + txn->n_applier_rows > 0);
+ struct journal_entry *req;
+ struct txn_stmt *stmt;
- /* Prepare a journal entry. */
- struct journal_entry *req = journal_entry_new(txn->n_new_rows +
- txn->n_applier_rows,
- &txn->region,
- txn_entry_complete_cb,
- txn);
- if (req == NULL) {
- txn_rollback(txn);
- return -1;
- }
+ req = journal_entry_new(txn->n_new_rows + txn->n_applier_rows,
+ &txn->region, txn_entry_complete_cb, txn);
+ if (req == NULL)
+ return NULL;
- struct txn_stmt *stmt;
struct xrow_header **remote_row = req->rows;
struct xrow_header **local_row = req->rows + txn->n_applier_rows;
+
stailq_foreach_entry(stmt, &txn->stmts, next) {
if (stmt->has_triggers) {
txn_init_triggers(txn);
rlist_splice(&txn->on_commit, &stmt->on_commit);
}
+
+ /* A read (e.g. select) request */
if (stmt->row == NULL)
- continue; /* A read (e.g. select) request */
+ continue;
+
if (stmt->row->replica_id == 0)
*local_row++ = stmt->row;
else
*remote_row++ = stmt->row;
+
req->approx_len += xrow_approx_len(stmt->row);
}
+
assert(remote_row == req->rows + txn->n_applier_rows);
assert(local_row == remote_row + txn->n_new_rows);
+ return req;
+}
+
+static int64_t
+txn_write_to_wal_async(struct txn *txn)
+{
+ struct journal_entry *req = txn_entry_new(txn);
+ if (req == NULL) {
+ txn_rollback(txn);
+ return -1;
+ }
+
/* Send the entry to the journal. */
if (journal_write_async(req) < 0) {
diag_set(ClientError, ER_WAL_IO);
--
2.20.1
^ permalink raw reply [flat|nested] 26+ messages in thread
* [Tarantool-patches] [PATCH 11/14] box/txn: merge txn_write_to_wal_async to txn_commit_async
2020-02-19 18:36 [Tarantool-patches] [PATCH 00/14] rework async and sync transactions Cyrill Gorcunov
` (9 preceding siblings ...)
2020-02-19 18:37 ` [Tarantool-patches] [PATCH 10/14] box/txn: move journal entry allocation into " Cyrill Gorcunov
@ 2020-02-19 18:37 ` Cyrill Gorcunov
2020-02-19 18:37 ` [Tarantool-patches] [PATCH 12/14] box/txn: do not use journal_write_async under the hood Cyrill Gorcunov
` (2 subsequent siblings)
13 siblings, 0 replies; 26+ messages in thread
From: Cyrill Gorcunov @ 2020-02-19 18:37 UTC (permalink / raw)
To: tml
No need for separate routine, everything is ready
to be used inside txn_commit_async itself.
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
src/box/txn.c | 33 ++++++++++++++-------------------
1 file changed, 14 insertions(+), 19 deletions(-)
diff --git a/src/box/txn.c b/src/box/txn.c
index 4dbaf5b91..a95c28431 100644
--- a/src/box/txn.c
+++ b/src/box/txn.c
@@ -536,24 +536,6 @@ txn_entry_new(struct txn *txn)
return req;
}
-static int64_t
-txn_write_to_wal_async(struct txn *txn)
-{
- struct journal_entry *req = txn_entry_new(txn);
- if (req == NULL) {
- txn_rollback(txn);
- return -1;
- }
-
- /* Send the entry to the journal. */
- if (journal_write_async(req) < 0) {
- diag_set(ClientError, ER_WAL_IO);
- diag_log();
- return -1;
- }
- return 0;
-}
-
/*
* Prepare a transaction using engines.
*/
@@ -602,7 +584,20 @@ txn_commit_async(struct txn *txn)
if (txn_complete_nop(txn))
return 0;
- return txn_write_to_wal_async(txn);
+ struct journal_entry *req = txn_entry_new(txn);
+ if (req == NULL) {
+ txn_rollback(txn);
+ return -1;
+ }
+
+ /* Send the entry to the journal. */
+ if (journal_write_async(req) < 0) {
+ diag_set(ClientError, ER_WAL_IO);
+ diag_log();
+ return -1;
+ }
+
+ return 0;
}
int
--
2.20.1
^ permalink raw reply [flat|nested] 26+ messages in thread
* [Tarantool-patches] [PATCH 12/14] box/txn: do not use journal_write_async under the hood
2020-02-19 18:36 [Tarantool-patches] [PATCH 00/14] rework async and sync transactions Cyrill Gorcunov
` (10 preceding siblings ...)
2020-02-19 18:37 ` [Tarantool-patches] [PATCH 11/14] box/txn: merge txn_write_to_wal_async to txn_commit_async Cyrill Gorcunov
@ 2020-02-19 18:37 ` Cyrill Gorcunov
2020-02-19 18:37 ` [Tarantool-patches] [PATCH 13/14] box/journal: introduce journal_write Cyrill Gorcunov
2020-02-19 18:37 ` [Tarantool-patches] [PATCH 14/14] box/txn: use journal_write in txn_commit Cyrill Gorcunov
13 siblings, 0 replies; 26+ messages in thread
From: Cyrill Gorcunov @ 2020-02-19 18:37 UTC (permalink / raw)
To: tml
While there is some code duplication (maybe we should
make merge this into txn_prepare?) the idea is to
unweave journal_write_async from txn_commit
which is synchronous.
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
src/box/txn.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/src/box/txn.c b/src/box/txn.c
index a95c28431..38241c066 100644
--- a/src/box/txn.c
+++ b/src/box/txn.c
@@ -605,8 +605,27 @@ txn_commit(struct txn *txn)
{
txn->fiber = fiber();
- if (txn_commit_async(txn) != 0)
+ if (txn_prepare(txn) != 0) {
+ txn_rollback(txn);
+ return -1;
+ }
+
+ if (txn_complete_nop(txn))
+ return 0;
+
+ struct journal_entry *req = txn_entry_new(txn);
+ if (req == NULL) {
+ txn_rollback(txn);
+ return -1;
+ }
+
+ /* Send the entry to the journal. */
+ if (journal_write_async(req) < 0) {
+ diag_set(ClientError, ER_WAL_IO);
+ diag_log();
return -1;
+ }
+
/*
* In case of non-yielding journal the transaction could already
* be done and there is nothing to wait in such cases.
--
2.20.1
^ permalink raw reply [flat|nested] 26+ messages in thread
* [Tarantool-patches] [PATCH 13/14] box/journal: introduce journal_write
2020-02-19 18:36 [Tarantool-patches] [PATCH 00/14] rework async and sync transactions Cyrill Gorcunov
` (11 preceding siblings ...)
2020-02-19 18:37 ` [Tarantool-patches] [PATCH 12/14] box/txn: do not use journal_write_async under the hood Cyrill Gorcunov
@ 2020-02-19 18:37 ` Cyrill Gorcunov
2020-02-19 18:37 ` [Tarantool-patches] [PATCH 14/14] box/txn: use journal_write in txn_commit Cyrill Gorcunov
13 siblings, 0 replies; 26+ messages in thread
From: Cyrill Gorcunov @ 2020-02-19 18:37 UTC (permalink / raw)
To: tml
To write entries in synchronous way. It
uses journal_write_async under the hood
together with fiber_wait.
Note both journal_write and journal_write_async
require current fiber to come in with txn
bound to the storage for context unification
sake.
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
src/box/journal.c | 24 ++++++++++++++++++++++++
src/box/journal.h | 8 ++++++++
2 files changed, 32 insertions(+)
diff --git a/src/box/journal.c b/src/box/journal.c
index 238860165..d0860e883 100644
--- a/src/box/journal.c
+++ b/src/box/journal.c
@@ -98,3 +98,27 @@ journal_write_async(struct journal_entry *entry)
return ret;
}
+
+int
+journal_write(struct journal_entry *entry)
+{
+ struct txn *txn = in_txn();
+ assert(txn != NULL);
+
+ if (journal_write_async(entry))
+ return -1;
+
+ /*
+ * In case of non-yielding journal the transaction could
+ * already be done and there is nothing to wait in such
+ * case, otherwise wait for wakeup upon transaction
+ * is complete.
+ */
+ if (!txn_has_flag(txn, TXN_IS_DONE)) {
+ bool cancellable = fiber_set_cancellable(false);
+ fiber_yield();
+ fiber_set_cancellable(cancellable);
+ }
+
+ return txn->signature >= 0 ? 0 : -1;
+}
diff --git a/src/box/journal.h b/src/box/journal.h
index efb6ac8e1..e73d684e5 100644
--- a/src/box/journal.h
+++ b/src/box/journal.h
@@ -131,6 +131,14 @@ extern struct journal *current_journal;
extern int
journal_write_async(struct journal_entry *entry);
+/**
+ * Write a single entry to the journal.
+ *
+ * @return 0 if write was written or -1 in case of an error.
+ */
+extern int
+journal_write(struct journal_entry *entry);
+
/**
* Change the current implementation of the journaling API.
* Happens during life cycle of an instance:
--
2.20.1
^ permalink raw reply [flat|nested] 26+ messages in thread
* [Tarantool-patches] [PATCH 14/14] box/txn: use journal_write in txn_commit
2020-02-19 18:36 [Tarantool-patches] [PATCH 00/14] rework async and sync transactions Cyrill Gorcunov
` (12 preceding siblings ...)
2020-02-19 18:37 ` [Tarantool-patches] [PATCH 13/14] box/journal: introduce journal_write Cyrill Gorcunov
@ 2020-02-19 18:37 ` Cyrill Gorcunov
2020-02-19 19:09 ` Konstantin Osipov
13 siblings, 1 reply; 26+ messages in thread
From: Cyrill Gorcunov @ 2020-02-19 18:37 UTC (permalink / raw)
To: tml
This allows us to keep wait for transaction
complete in one place -- inside journal_write.
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
src/box/txn.c | 18 ++----------------
1 file changed, 2 insertions(+), 16 deletions(-)
diff --git a/src/box/txn.c b/src/box/txn.c
index 38241c066..8f9be3269 100644
--- a/src/box/txn.c
+++ b/src/box/txn.c
@@ -619,25 +619,11 @@ txn_commit(struct txn *txn)
return -1;
}
- /* Send the entry to the journal. */
- if (journal_write_async(req) < 0) {
+ int res = journal_write(req);
+ if (res != 0) {
diag_set(ClientError, ER_WAL_IO);
diag_log();
- return -1;
- }
-
- /*
- * In case of non-yielding journal the transaction could already
- * be done and there is nothing to wait in such cases.
- */
- if (!txn_has_flag(txn, TXN_IS_DONE)) {
- bool cancellable = fiber_set_cancellable(false);
- fiber_yield();
- fiber_set_cancellable(cancellable);
}
- int res = txn->signature >= 0 ? 0 : -1;
- if (res != 0)
- diag_set(ClientError, ER_WAL_IO);
/* Synchronous transactions are freed by the calling fiber. */
txn_free(txn);
--
2.20.1
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Tarantool-patches] [PATCH 14/14] box/txn: use journal_write in txn_commit
2020-02-19 18:37 ` [Tarantool-patches] [PATCH 14/14] box/txn: use journal_write in txn_commit Cyrill Gorcunov
@ 2020-02-19 19:09 ` Konstantin Osipov
2020-02-19 20:01 ` Cyrill Gorcunov
0 siblings, 1 reply; 26+ messages in thread
From: Konstantin Osipov @ 2020-02-19 19:09 UTC (permalink / raw)
To: Cyrill Gorcunov; +Cc: tml
* Cyrill Gorcunov <gorcunov@gmail.com> [20/02/19 21:44]:
> This allows us to keep wait for transaction
> complete in one place -- inside journal_write.
>
I thinks this patch set is on track. I would go further and would
add patches which push journal_write() implementation down to
wal.cc, since only wal.cc's journal_write is different from
journal_write_async().
--
Konstantin Osipov, Moscow, Russia
https://scylladb.com
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Tarantool-patches] [PATCH 14/14] box/txn: use journal_write in txn_commit
2020-02-19 19:09 ` Konstantin Osipov
@ 2020-02-19 20:01 ` Cyrill Gorcunov
0 siblings, 0 replies; 26+ messages in thread
From: Cyrill Gorcunov @ 2020-02-19 20:01 UTC (permalink / raw)
To: Konstantin Osipov; +Cc: tml
On Wed, Feb 19, 2020 at 10:09:35PM +0300, Konstantin Osipov wrote:
> * Cyrill Gorcunov <gorcunov@gmail.com> [20/02/19 21:44]:
> > This allows us to keep wait for transaction
> > complete in one place -- inside journal_write.
> >
>
> I thinks this patch set is on track. I would go further and would
> add patches which push journal_write() implementation down to
> wal.cc, since only wal.cc's journal_write is different from
> journal_write_async().
Thanks, will do.
^ permalink raw reply [flat|nested] 26+ messages in thread