[Tarantool-patches] [PATCH] Move rollback to savepoint from ffi to C-API
Leonid Vasiliev
lvasiliev at tarantool.org
Mon Jan 27 10:24:18 MSK 2020
The result patch:
https://github.com/tarantool/tarantool/issues/4427
https://github.com/tarantool/tarantool/tree/lvasiliev/gh-4427-move-rollback-from-ffi-to-c-api-v2
box: rewrite rollback to savepoint to Lua/C
The fix is prevent an emerge of "FFI sandwich".
See
https://github.com/tarantool/tarantool/issues/4427#issuecomment-524798109
for more information.
One of the aims of this change (in addition to the fix) is to
increase locality of highly
coupled code for the rollback to savepoint.
Noted that <struct txn>.id starts from 1, because I lean on this
fact to
use luaL_toint64(), which does not distinguish an unexpected Lua type
case and cdata<int64_t> with zero value. It seems that this assumption
already exists: the code that prepare arguments for 'on_commit'
triggers
uses luaL_toint64() too (see lbox_txn_pairs()).
Fixes #4427
Co-authored-by: Alexander Turenko<alexander.turenko at tarantool.org>
Co-authored-by: Leonid Vasiliev<lvasiliev at tarantool.org>
---
diff --git a/src/box/lua/init.c b/src/box/lua/init.c
index 7ffed409d..620a10600 100644
--- a/src/box/lua/init.c
+++ b/src/box/lua/init.c
@@ -63,6 +63,8 @@
#include "box/lua/key_def.h"
#include "box/lua/merger.h"
+static uint32_t CTID_STRUCT_TXN_SAVEPOINT_PTR = 0;
+
extern char session_lua[],
tuple_lua[],
key_def_lua[],
@@ -107,6 +109,101 @@ lbox_rollback(lua_State *L)
return 0;
}
+/**
+ * Extract <struct txn_savepoint *> from a cdata value on the Lua
+ * stack.
+ *
+ * The function is a helper for extracting 'csavepoint' field from
+ * a Lua table created using box.savepoint().
+ */
+static struct txn_savepoint *
+luaT_check_txn_savepoint_cdata(struct lua_State *L, int idx)
+{
+ if (lua_type(L, idx) != LUA_TCDATA)
+ return NULL;
+
+ uint32_t cdata_type;
+ struct txn_savepoint **svp_ptr = luaL_checkcdata(L, idx, &cdata_type);
+ if (svp_ptr == NULL || cdata_type != CTID_STRUCT_TXN_SAVEPOINT_PTR)
+ return NULL;
+ return *svp_ptr;
+}
+
+/**
+ * Extract a savepoint from the Lua stack.
+ *
+ * Expected a value that is created using box.savepoint():
+ *
+ * {
+ * csavepoint = <cdata<struct txn_savepoint *>>,
+ * txn_id = <cdata<int64_t>>,
+ * }
+ */
+static struct txn_savepoint *
+luaT_check_txn_savepoint(struct lua_State *L, int idx, int64_t
*svp_txn_id_ptr)
+{
+ /* Verify passed value type. */
+ if (lua_type(L, idx) != LUA_TTABLE)
+ return NULL;
+
+ /* Extract and verify csavepoint. */
+ lua_getfield(L, idx, "csavepoint");
+ struct txn_savepoint *svp = luaT_check_txn_savepoint_cdata(L, -1);
+ lua_pop(L, 1);
+ if (svp == NULL)
+ return NULL;
+
+ /* Extract and verify transaction id from savepoint. */
+ lua_getfield(L, idx, "txn_id");
+ int64_t svp_txn_id = luaL_toint64(L, -1);
+ lua_pop(L, 1);
+ if (svp_txn_id == 0)
+ return NULL;
+ *svp_txn_id_ptr = svp_txn_id;
+
+ return svp;
+}
+
+/**
+ * Rollback to a savepoint.
+ *
+ * At success push nothing to the Lua stack.
+ *
+ * At any error raise a Lua error.
+ */
+static int
+lbox_rollback_to_savepoint(struct lua_State *L)
+{
+ int64_t svp_txn_id;
+ struct txn_savepoint *svp;
+
+ if (lua_gettop(L) != 1 ||
+ (svp = luaT_check_txn_savepoint(L, 1, &svp_txn_id)) == NULL)
+ return luaL_error(L,
+ "Usage: box.rollback_to_savepoint(savepoint)");
+
+ /*
+ * Verify that we're in a transaction and that it is the
+ * same transaction as one where the savepoint was
+ * created.
+ */
+ struct txn *txn = in_txn();
+ if (txn == NULL || svp_txn_id != txn->id) {
+ diag_set(ClientError, ER_NO_SUCH_SAVEPOINT);
+ return luaT_error(L);
+ }
+
+ /*
+ * All checks have been passed: try to rollback to the
+ * savepoint.
+ */
+ int rc = box_txn_rollback_to_savepoint(svp);
+ if (rc != 0)
+ return luaT_error(L);
+
+ return 0;
+}
+
/**
* Get a next txn statement from the current transaction. This is
* a C closure and 2 upvalues should be available: first is a
@@ -279,6 +376,7 @@ static const struct luaL_Reg boxlib[] = {
{"on_commit", lbox_on_commit},
{"on_rollback", lbox_on_rollback},
{"snapshot", lbox_snapshot},
+ {"rollback_to_savepoint", lbox_rollback_to_savepoint},
{NULL, NULL}
};
@@ -293,6 +391,10 @@ static const struct luaL_Reg boxlib_backup[] = {
void
box_lua_init(struct lua_State *L)
{
+ luaL_cdef(L, "struct txn_savepoint;");
+ CTID_STRUCT_TXN_SAVEPOINT_PTR = luaL_ctypeid(L,
+ "struct txn_savepoint*");
+
/* Use luaL_register() to set _G.box */
luaL_register(L, "box", boxlib);
lua_pop(L, 1);
diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
index e898c3aa6..50c96a335 100644
--- a/src/box/lua/schema.lua
+++ b/src/box/lua/schema.lua
@@ -77,9 +77,6 @@ ffi.cdef[[
box_txn_savepoint_t *
box_txn_savepoint();
- int
- box_txn_rollback_to_savepoint(box_txn_savepoint_t *savepoint);
-
struct port_tuple_entry {
struct port_tuple_entry *next;
struct tuple *tuple;
@@ -334,28 +331,6 @@ box.savepoint = function()
return { csavepoint=csavepoint, txn_id=builtin.box_txn_id() }
end
-local savepoint_type = ffi.typeof('box_txn_savepoint_t')
-
-local function check_savepoint(savepoint)
- if savepoint == nil or savepoint.txn_id == nil or
- savepoint.csavepoint == nil or
- type(tonumber(savepoint.txn_id)) ~= 'number' or
- type(savepoint.csavepoint) ~= 'cdata' or
- not ffi.istype(savepoint_type, savepoint.csavepoint) then
- error("Usage: box.rollback_to_savepoint(savepoint)")
- end
-end
-
-box.rollback_to_savepoint = function(savepoint)
- check_savepoint(savepoint)
- if savepoint.txn_id ~= builtin.box_txn_id() then
- box.error(box.error.NO_SUCH_SAVEPOINT)
- end
- if builtin.box_txn_rollback_to_savepoint(savepoint.csavepoint) ==
-1 then
- box.error()
- end
-end
-
local function atomic_tail(status, ...)
if not status then
box.rollback()
@@ -371,7 +346,7 @@ box.atomic = function(fun, ...)
end
-- box.commit yields, so it's defined as Lua/C binding
--- box.rollback yields as well
+-- box.rollback and box.rollback_to_savepoint yields as well
function update_format(format)
local result = {}
diff --git a/src/box/txn.h b/src/box/txn.h
index da12feebf..97d7b5de5 100644
--- a/src/box/txn.h
+++ b/src/box/txn.h
@@ -166,6 +166,8 @@ struct txn {
* A sequentially growing transaction id, assigned when
* a transaction is initiated. Used to identify
* a transaction after it has possibly been destroyed.
+ *
+ * Valid IDs start from 1.
*/
int64_t id;
/** List of statements in a transaction. */
diff --git a/test/engine/savepoint.result b/test/engine/savepoint.result
index 86a2d0be2..c23645ce6 100644
--- a/test/engine/savepoint.result
+++ b/test/engine/savepoint.result
@@ -18,7 +18,7 @@ s1 = box.savepoint()
...
box.rollback_to_savepoint(s1)
---
-- error: 'builtin/box/schema.lua: Usage:
box.rollback_to_savepoint(savepoint)'
+- error: 'Usage: box.rollback_to_savepoint(savepoint)'
...
box.begin() s1 = box.savepoint()
---
@@ -327,27 +327,27 @@ test_run:cmd("setopt delimiter ''");
ok1, errmsg1
---
- false
-- 'builtin/box/schema.lua: Usage: box.rollback_to_savepoint(savepoint)'
+- 'Usage: box.rollback_to_savepoint(savepoint)'
...
ok2, errmsg2
---
- false
-- 'builtin/box/schema.lua: Usage: box.rollback_to_savepoint(savepoint)'
+- 'Usage: box.rollback_to_savepoint(savepoint)'
...
ok3, errmsg3
---
- false
-- 'builtin/box/schema.lua: Usage: box.rollback_to_savepoint(savepoint)'
+- 'Usage: box.rollback_to_savepoint(savepoint)'
...
ok4, errmsg4
---
- false
-- 'builtin/box/schema.lua: Usage: box.rollback_to_savepoint(savepoint)'
+- 'Usage: box.rollback_to_savepoint(savepoint)'
...
ok5, errmsg5
---
- false
-- 'builtin/box/schema.lua: Usage: box.rollback_to_savepoint(savepoint)'
+- 'Usage: box.rollback_to_savepoint(savepoint)'
...
s:select{}
---
More information about the Tarantool-patches
mailing list