From: Cyrill Gorcunov <gorcunov@gmail.com> To: Serge Petrenko <sergepetrenko@tarantool.org> Cc: tarantool-patches@dev.tarantool.org, v.shpilevoy@tarantool.org Subject: Re: [Tarantool-patches] [PATCH 2/2] replication: make relay send txs in batches Date: Tue, 19 May 2020 14:18:02 +0300 [thread overview] Message-ID: <20200519111802.GA193176@grain> (raw) In-Reply-To: <a321090e-3388-00e9-d849-2cbce6d05562@tarantool.org> On Tue, May 19, 2020 at 01:49:10PM +0300, Serge Petrenko wrote: ... > > > + if (packet->is_commit) { > > > +write: relay_send_tx(relay, &tx_rows); > > > + tx_rows = RLIST_HEAD_INITIALIZER(tx_rows); > > Why?! This is stack local variable, no? > Why? It's static. Yup, just found out ;) It is so hidden in the other vars declaration so was not obvious (a least for me :-). I'm appending the more clear approach. But Serge, this is my personal opinion, I'm fine with your current code as well, so Reviewed-by: Cyrill Gorcunov <gorcunov@gmail.com> it is up to you to pick or drop the diff below --- src/box/relay.cc | 37 ++++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/src/box/relay.cc b/src/box/relay.cc index 17b7bc667..27424e0ca 100644 --- a/src/box/relay.cc +++ b/src/box/relay.cc @@ -760,12 +760,12 @@ struct relay_tx_row { }; static void -relay_send_tx(struct relay *relay, struct rlist *tx_rows) +relay_send_tx(struct relay *relay, struct rlist *head) { ERROR_INJECT_YIELD(ERRINJ_RELAY_SEND_DELAY); struct relay_tx_row *tx_row; - rlist_foreach_entry(tx_row, tx_rows, link) { + rlist_foreach_entry(tx_row, head, link) { tx_row->row.sync = relay->sync; coio_write_xrow(&relay->io, &tx_row->row); } @@ -784,10 +784,13 @@ static void relay_collect_tx(struct xstream *stream, struct xrow_header *packet) { struct relay *relay = container_of(stream, struct relay, stream); - static RLIST_HEAD(tx_rows); - struct errinj *inj; struct relay_tx_row *tx_row; + struct errinj *inj; + + static RLIST_HEAD(tx_rows_list); + assert(iproto_type_is_dml(packet->type)); + if (packet->group_id == GROUP_LOCAL) { /* * We do not relay replica-local rows to other @@ -805,20 +808,22 @@ relay_collect_tx(struct xstream *stream, struct xrow_header *packet) * in case the transaction is not fully * local. */ - if (packet->is_commit && !rlist_empty(&tx_rows)) { - rlist_last_entry(&tx_rows, struct relay_tx_row, - link)->row.is_commit = true; - goto write; - } - return; + if (!packet->is_commit || rlist_empty(&tx_rows_list)) + return; + + rlist_last_entry(&tx_rows_list, struct relay_tx_row, + link)->row.is_commit = true; + goto write; } packet->type = IPROTO_NOP; packet->group_id = GROUP_DEFAULT; packet->bodycnt = 0; } + /* Check if the rows from the instance are filtered. */ - if ((1 << packet->replica_id & relay->id_filter) != 0) + if (((1u << packet->replica_id) & relay->id_filter) != 0) return; + /* * We're feeding a WAL, thus responding to FINAL JOIN or SUBSCRIBE * request. If this is FINAL JOIN (i.e. relay->replica is NULL), @@ -845,7 +850,7 @@ relay_collect_tx(struct xstream *stream, struct xrow_header *packet) } /* A short path for single-statement transactions. */ - if (packet->is_commit && rlist_empty(&tx_rows)) { + if (packet->is_commit && rlist_empty(&tx_rows_list)) { relay_send(relay, packet); return; } @@ -857,11 +862,13 @@ relay_collect_tx(struct xstream *stream, struct xrow_header *packet) "struct relay_tx_row"); } tx_row->row = *packet; - rlist_add_tail_entry(&tx_rows, tx_row, link); + rlist_add_tail_entry(&tx_rows_list, tx_row, link); if (packet->is_commit) { -write: relay_send_tx(relay, &tx_rows); - tx_rows = RLIST_HEAD_INITIALIZER(tx_rows); +write: + relay_send_tx(relay, &tx_rows_list); + tx_rows_list = RLIST_HEAD_INITIALIZER(tx_rows_list); + /* * Free all the relay_tx_rows allocated on the * fiber region. -- 2.26.2
next prev parent reply other threads:[~2020-05-19 11:18 UTC|newest] Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-05-18 12:24 [Tarantool-patches] [PATCH 0/2] fix replication tx boundaries after local space rework Serge Petrenko 2020-05-18 12:24 ` [Tarantool-patches] [PATCH 1/2] wal: fix tx boundaries Serge Petrenko 2020-05-19 9:08 ` Cyrill Gorcunov 2020-05-19 10:30 ` Serge Petrenko 2020-05-18 12:24 ` [Tarantool-patches] [PATCH 2/2] replication: make relay send txs in batches Serge Petrenko 2020-05-18 12:28 ` Serge Petrenko 2020-05-19 10:23 ` Cyrill Gorcunov 2020-05-19 10:49 ` Serge Petrenko 2020-05-19 11:18 ` Cyrill Gorcunov [this message] 2020-05-19 12:31 ` Serge Petrenko 2020-05-19 16:24 ` Serge Petrenko
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200519111802.GA193176@grain \ --to=gorcunov@gmail.com \ --cc=sergepetrenko@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --cc=v.shpilevoy@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH 2/2] replication: make relay send txs in batches' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox