[Tarantool-patches] [PATCH v2 3/3] box: remove dead code from box_process_call/eval()
Georgy Kirichenko
georgy at tarantool.org
Fri Jan 17 20:41:48 MSK 2020
On Friday, 17 January 2020 00:54:23 MSK Vladislav Shpilevoy wrote:
> box_process_call/eval() in the end check if there is an
> active transaction. If there is, it is rolled back, and
> an error is set.
>
> But rollback is not needed anymore, because anyway in
> the end of the request the fiber is stopped, and its
> not finished transaction is rolled back. Just setting
> of the error is enough.
Hi!
Thanks for the patch, but I do not think that to remove an explicit rollback
is a a good idea because of broken encapsulation - call and eval handlers
should not rely on its execution context - a simple fiber, iproto fiber pool
member or whatever else. Also I would like to mention that box_call and
box_eval are members of the public api and it is not necessary that user will
stop a fiber.
>
> Follow-up #4662
> ---
> src/box/call.c | 28 ++++------------------------
> src/box/txn.c | 1 +
> 2 files changed, 5 insertions(+), 24 deletions(-)
>
> diff --git a/src/box/call.c b/src/box/call.c
> index bcaa453ea..a46a61c3c 100644
> --- a/src/box/call.c
> +++ b/src/box/call.c
> @@ -122,23 +122,13 @@ box_process_call(struct call_request *request, struct
> port *port) SC_FUNCTION, tt_cstr(name, name_len))) == 0) {
> rc = box_lua_call(name, name_len, &args, port);
> }
> -
> - struct txn *txn = in_txn();
> - if (rc != 0) {
> - if (txn != NULL)
> - txn_rollback(txn);
> - fiber_gc();
> + if (rc != 0)
> return -1;
> - }
> -
> - if (txn != NULL) {
> + if (in_txn() != NULL) {
> diag_set(ClientError, ER_FUNCTION_TX_ACTIVE);
> port_destroy(port);
> - txn_rollback(txn);
> - fiber_gc();
> return -1;
> }
> -
> return 0;
> }
>
> @@ -154,21 +144,11 @@ box_process_eval(struct call_request *request, struct
> port *port) request->args_end - request->args);
> const char *expr = request->expr;
> uint32_t expr_len = mp_decode_strl(&expr);
> - struct txn *txn;
> - if (box_lua_eval(expr, expr_len, &args, port) != 0) {
> - txn = in_txn();
> - if (txn != NULL)
> - txn_rollback(txn);
> - fiber_gc();
> + if (box_lua_eval(expr, expr_len, &args, port) != 0)
> return -1;
> - }
> -
> - txn = in_txn();
> - if (txn != NULL) {
> + if (in_txn() != 0) {
> diag_set(ClientError, ER_FUNCTION_TX_ACTIVE);
> port_destroy(port);
> - txn_rollback(txn);
> - fiber_gc();
> return -1;
> }
> return 0;
> diff --git a/src/box/txn.c b/src/box/txn.c
> index 963ec8eeb..0854bbacc 100644
> --- a/src/box/txn.c
> +++ b/src/box/txn.c
> @@ -845,6 +845,7 @@ txn_on_stop(struct trigger *trigger, void *event)
> (void) trigger;
> (void) event;
> txn_rollback(in_txn()); /* doesn't yield or fail */
> + fiber_gc();
> return 0;
> }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20200117/d7d14ec9/attachment.sig>
More information about the Tarantool-patches
mailing list