From: Konstantin Osipov <kostja@tarantool.org> To: tarantool-patches@freelists.org Cc: Georgy Kirichenko <georgy@tarantool.org> Subject: [tarantool-patches] Re: [PATCH 2/2] Get rid of aurocommit from a txn structure Date: Thu, 11 Apr 2019 09:43:51 +0300 [thread overview] Message-ID: <20190411064351.GQ8268@chai> (raw) In-Reply-To: <3b4ffe843914867d84b8b136ffb892bc580d8581.1554880565.git.georgy@tarantool.org> * Georgy Kirichenko <georgy@tarantool.org> [19/04/10 10:26]: > Move transaction auto start and auto commit behavior to box level. > Now transaction won't start and commit automatically without > txn_begin/txn_commit invocations. This is a part of a bigger transaction > refactoring in order to implement detachable transactions and a parallel > applier. This change is looking good, the only nit is about so much labor one has to do to run an autocommit transaction. If it was C++, I would suggest adding a lambda which would auto-commit on destruction, But given it's plain C, how about simply making the code a bit more straightforward in its flow ? > + if (txn_commit_stmt(txn, request) != 0 || > + (autocommit && txn_commit(txn) != 0)) > + goto fail; > + if (autocommit) > + fiber_gc(); > + This piece in particular, I would rewrite: if (txn_commit_stmt()) goto fail() if (autocommit) { if (txn_commit() goto fail; fiber_gc(); } > + rc = 0; > +done: > + if (tuple != NULL) > + tuple_unref(tuple); > return rc; > + > +fail: > + if (autocommit) > + txn_rollback(); > + goto done; I think using double labels like here is on the verge of goto abuse. > - if (box_process_rw(&request, space, NULL) != 0) { > + struct txn *txn = txn_begin(); > + if (txn == NULL || box_process_rw(&request, space, NULL) != 0 || > + txn_commit(txn) != 0) { > say_error("error applying row: %s", request_str(&request)); > + if (txn != NULL) > + txn_rollback(); > diag_raise(); Here is another example. -- Konstantin Osipov, Moscow, Russia, +7 903 626 22 32 http://tarantool.io - www.twitter.com/kostja_osipov
prev parent reply other threads:[~2019-04-11 6:43 UTC|newest] Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-04-10 7:22 [tarantool-patches] [PATCH 0/2] Transaction refactoring Georgy Kirichenko 2019-04-10 7:22 ` [tarantool-patches] [PATCH 1/2] Introduce a txn memory region Georgy Kirichenko 2019-04-10 10:11 ` [tarantool-patches] " Konstantin Osipov 2019-04-10 7:22 ` [tarantool-patches] [PATCH 2/2] Get rid of aurocommit from a txn structure Georgy Kirichenko 2019-04-11 6:43 ` Konstantin Osipov [this message]
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=20190411064351.GQ8268@chai \ --to=kostja@tarantool.org \ --cc=georgy@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='[tarantool-patches] Re: [PATCH 2/2] Get rid of aurocommit from a txn structure' \ /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