From: Konstantin Osipov <kostja@tarantool.org> To: Vladimir Davydov <vdavydov.dev@gmail.com> Cc: tarantool-patches@freelists.org Subject: Re: [PATCH 4/6] txn: do not require space id for nop requests Date: Wed, 27 Jun 2018 20:45:09 +0300 [thread overview] Message-ID: <20180627174509.GH28358@chai> (raw) In-Reply-To: <4dfddcb3838385c88cbfdf51d52ef112f56cf130.1528906027.git.vdavydov.dev@gmail.com> * Vladimir Davydov <vdavydov.dev@gmail.com> [18/06/13 20:40]: > Currently, IPROTO_NOP can only be generated by a before_replace trigger, > when it returns the old tuple thus turning the original operation into a > NOP. In such a case we know the space id and we write it to the request > body. This allows us to dispatch NOP requests via DML route. > > As a part of replica local spaces feature, we will substitute requests > operating on local spaces with NOP in relay in order to promote vclock > on replicas without actual data modification. Since space_id is stored > in request body, sending it to replicas would mean decoding the request > body in relay, which is an overkill. To avoid that, let's separate NOP > and DML paths and remove space_id from NOP requests. LGTM, a few comments below. > @@ -277,6 +277,11 @@ apply_row(struct xstream *stream, struct xrow_header *row) > (void) stream; > struct request request; > xrow_decode_dml_xc(row, &request, dml_request_key_map(row->type)); > + if (request.type == IPROTO_NOP) { > + if (txn_commit_nop(&request) != 0) > + diag_raise(); > + return; > + } txn_commit_nop() -> box_process_nop() or process_nop(). > @@ -50,7 +50,6 @@ request_create_from_tuple(struct request *request, struct space *space, > struct tuple *old_tuple, struct tuple *new_tuple) > { > memset(request, 0, sizeof(*request)); > - request->space_id = space->def->id; > > if (old_tuple == new_tuple) { > /* > @@ -61,6 +60,7 @@ request_create_from_tuple(struct request *request, struct space *space, > return 0; > } > > + request->space_id = space->def->id; Please add a comment /* * Space pointer may be zero in case of NOP, in which case this * line is not reached. */ You changed a lot of code making it more NOP friendly, for example txn_begin_in_engine() now works even if there is already a transaction which has a NOP statement. This seems like a bug fix with no test. Could you please add a test? -- Konstantin Osipov, Moscow, Russia, +7 903 626 22 32 http://tarantool.io - www.twitter.com/kostja_osipov
next prev parent reply other threads:[~2018-06-27 17:45 UTC|newest] Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-06-13 16:10 [PATCH 0/6] Introduce replica local spaces Vladimir Davydov 2018-06-13 16:10 ` [PATCH 1/6] txn: remove unused C++ wrappers Vladimir Davydov 2018-06-27 17:08 ` Konstantin Osipov 2018-06-13 16:10 ` [PATCH 2/6] xrow: fix ret code on decode failure Vladimir Davydov 2018-06-27 17:29 ` Konstantin Osipov 2018-06-13 16:10 ` [PATCH 3/6] iproto: fix IPROTO_SERVER_IS_RO key code Vladimir Davydov 2018-06-27 17:48 ` Konstantin Osipov 2018-06-13 16:10 ` [PATCH 4/6] txn: do not require space id for nop requests Vladimir Davydov 2018-06-27 17:45 ` Konstantin Osipov [this message] 2018-06-28 9:13 ` Vladimir Davydov 2018-06-28 10:23 ` Konstantin Osipov 2018-06-28 10:35 ` Vladimir Davydov 2018-06-28 10:54 ` Konstantin Osipov 2018-06-28 11:10 ` Vladimir Davydov 2018-06-13 16:10 ` [PATCH 5/6] xrow: make NOP requests bodiless Vladimir Davydov 2018-06-27 17:49 ` Konstantin Osipov 2018-06-13 16:10 ` [PATCH 6/6] Introduce replica local spaces Vladimir Davydov 2018-06-13 21:26 ` [tarantool-patches] " Vladislav Shpilevoy 2018-06-27 18:24 ` Konstantin Osipov 2018-06-27 18:27 ` Konstantin Osipov
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=20180627174509.GH28358@chai \ --to=kostja@tarantool.org \ --cc=tarantool-patches@freelists.org \ --cc=vdavydov.dev@gmail.com \ --subject='Re: [PATCH 4/6] txn: do not require space id for nop requests' \ /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