[tarantool-patches] Re: [PATCH v7 1/2] sql: return all generated ids via IPROTO
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Mon Oct 29 13:17:00 MSK 2018
Hi!
On 27/10/2018 15:43, imeevma at tarantool.org wrote:
> According to documentation some JDBC functions have an ability to
> return all ids that were generated in executed INSERT statement.
> This patch gives a way to implement such functionality.
>
> Closes #2618
> ---
Do not omit branch and issue name.
Why did not you send the second commit?
Now I see, that it would be better to split this commit
into 2 parts: factoring fiber_gc() out of txn_commit and
auto-generated ids. I did it already and force pushed on the
branch.
But it appeared that tests fail with this reproduce
file:
========================= rep.yaml ===========================
---
- [sql/select-null.test.lua, memtx]
- [sql/errinj.test.lua, memtx]
- [sql/persistency.test.lua, memtx]
- [sql/errinj.test.lua, vinyl]
- [sql/gh-2929-primary-key.test.lua, vinyl]
- [sql/message-func-indexes.test.lua, vinyl]
- [sql/drop-table.test.lua, memtx]
- [sql/gh2808-inline-unique-persistency-check.test.lua, memtx]
- [sql/transitive-transactions.test.lua, memtx]
- [sql/max-on-index.test.lua, vinyl]
- [sql/iproto.test.lua, vinyl]
- [sql/checks.test.lua, vinyl]
- [sql/triggers.test.lua, memtx]
- [sql/gh-3199-no-mem-leaks.test.lua, memtx]
- [sql/savepoints.test.lua, vinyl]
- [sql/gh2808-inline-unique-persistency-check.test.lua, vinyl]
================================================================
python test-run.py --reproduce rep.yaml
[001] Test failed! Result content mismatch:
[001] --- sql/gh-3199-no-mem-leaks.result Thu Oct 25 22:58:31 2018
[001] +++ sql/gh-3199-no-mem-leaks.reject Mon Oct 29 13:12:35 2018
[001] @@ -27,7 +27,7 @@
[001] ...
[001] fiber.info()[fiber.self().id()].memory.used
[001] ---
[001] -- 0
[001] +- 4036
[001] ...
It repeats both on your branch and on my force
pushed version. Please, fix.
> src/box/execute.c | 28 ++++++++++++++-
> src/box/execute.h | 1 +
> src/box/lua/net_box.c | 29 ++++++++++++---
> src/box/sequence.c | 7 ++++
> src/box/sequence.h | 13 +++++++
> src/box/sql/vdbe.c | 27 ++++++++++++--
> src/box/sql/vdbe.h | 12 ++++++-
> src/box/sql/vdbeInt.h | 5 +++
> src/box/sql/vdbeaux.c | 21 ++++++-----
> src/box/txn.c | 15 +++++---
> src/box/txn.h | 27 ++++++++++++++
> src/box/vy_scheduler.c | 1 +
> test/sql/iproto.result | 92 ++++++++++++++++++++++++++++++++++++++++++++++++
> test/sql/iproto.test.lua | 24 +++++++++++++
> 14 files changed, 279 insertions(+), 23 deletions(-)
>
Just for record, my new commit:
=====================================================================
commit bcd32c9ee07f365efe142414f5456e34b7ae7bbb
Author: Vladislav Shpilevoy <v.shpilevoy at tarantool.org>
Date: Mon Oct 29 12:52:56 2018 +0300
box: factor fiber_gc out of txn_commit
Now txn_commit is judge, jury and executioner. It both
commits or rollbacks data, and collects it calling fiber_gc,
which destroys the region.
But SQL wants to use some transactional data after commit. It is
autogenerated identifiers - a list of sequence values generated
for autoincrement columns and explicit sequence:next() calls.
It is possible to store the list on malloced mem inside Vdbe, but
it complicates deallocation. Much more convenient to store all
transactional data on the transaction memory region, so it would
be freed together with fiber_gc.
After this patch applied, Vdbe takes care of txn memory
deallocation in a finalizer routine. Between commit and
finalization transactional data can be serialized wherever.
Needed for #2618
diff --git a/src/box/txn.c b/src/box/txn.c
index 617ceb8a2..9bcc6727e 100644
--- a/src/box/txn.c
+++ b/src/box/txn.c
@@ -250,8 +250,11 @@ txn_commit_stmt(struct txn *txn, struct request *request)
goto fail;
}
--txn->in_sub_stmt;
- if (txn->is_autocommit && txn->in_sub_stmt == 0)
- return txn_commit(txn);
+ if (txn->is_autocommit && txn->in_sub_stmt == 0) {
+ int rc = txn_commit(txn);
+ fiber_gc();
+ return rc;
+ }
return 0;
fail:
txn_rollback_stmt();
@@ -354,8 +357,6 @@ txn_commit(struct txn *txn)
txn_stmt_unref_tuples(stmt);
TRASH(txn);
- /** Free volatile txn memory. */
- fiber_gc();
fiber_set_txn(fiber(), NULL);
return 0;
fail:
@@ -463,7 +464,9 @@ box_txn_commit()
diag_set(ClientError, ER_COMMIT_IN_SUB_STMT);
return -1;
}
- return txn_commit(txn);
+ int rc = txn_commit(txn);
+ fiber_gc();
+ return rc;
}
int
diff --git a/src/box/vy_scheduler.c b/src/box/vy_scheduler.c
index eab3f6c54..4a46243ed 100644
--- a/src/box/vy_scheduler.c
+++ b/src/box/vy_scheduler.c
@@ -886,7 +886,7 @@ vy_deferred_delete_batch_process_f(struct cmsg *cmsg)
if (txn_commit(txn) != 0)
goto fail;
-
+ fiber_gc();
return;
fail:
batch->is_failed = true;
More information about the Tarantool-patches
mailing list