[Tarantool-patches] [PATCH] Move rollback to savepoint from ffi to C-API
Leonid Vasiliev
lvasiliev at tarantool.org
Tue Jan 21 00:17:17 MSK 2020
We have two possible solution of the FFI sandwich problem with rollback
to savepoint (with some pros and cons).
After a conversation with Alexander the next proposal was elaborated:
"Arbitrator (Igor for example) will pick one of them."
First:
pros:
- make the fewest possible changes
- the lua savepoint table ({ csavepoint=csavepoint,
txn_id=builtin.box_txn_id() }) is used only in Lua (defenition and using
are )
cons:
- box.rollback_to_savepoint have some lua decorator
diff --git a/src/box/lua/init.c b/src/box/lua/init.c
index 7ffed409d..6d9f23681 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,44 @@ lbox_rollback(lua_State *L)
return 0;
}
+/**
+ * Extract a savepoint from the Lua stack.
+ */
+static struct txn_savepoint *
+luaT_check_txn_savepoint(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;
+}
+
+/**
+ * Rollback to a savepoint.
+ *
+ * This is the helper function: it is registered in box.internal
+ * and is not the same as box.rollback_to_savepoint().
+ *
+ * Push zero at success and -1 at an error.
+ */
+static int
+lbox_rollback_to_savepoint(struct lua_State *L)
+{
+ struct txn_savepoint *svp;
+ if (lua_gettop(L) != 1 ||
+ (svp = luaT_check_txn_savepoint(L, 1)) == NULL)
+ return luaL_error(L,
+ "Usage: box.rollback_to_savepoint(savepoint)");
+
+ int rc = box_txn_rollback_to_savepoint(svp);
+ lua_pushnumber(L, rc);
+ return 1;
+}
+
/**
* Get a next txn statement from the current transaction. This is
* a C closure and 2 upvalues should be available: first is a
@@ -288,11 +328,20 @@ static const struct luaL_Reg boxlib_backup[] = {
{NULL, NULL}
};
+static const struct luaL_Reg boxlib_internal[] = {
+ {"rollback_to_savepoint", lbox_rollback_to_savepoint},
+ {NULL, NULL}
+};
+
#include "say.h"
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);
@@ -300,6 +349,9 @@ box_lua_init(struct lua_State *L)
luaL_register(L, "box.backup", boxlib_backup);
lua_pop(L, 1);
+ luaL_register(L, "box.internal", boxlib_internal);
+ lua_pop(L, 1);
+
box_lua_error_init(L);
box_lua_tuple_init(L);
box_lua_call_init(L);
diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
index e898c3aa6..1c49531e7 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;
@@ -351,7 +348,7 @@ box.rollback_to_savepoint = function(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
+ if box.internal.rollback_to_savepoint(savepoint.csavepoint) == -1 then
box.error()
end
end
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. */
Second:
pros:
- the lua box.rollback_to_savepoint decorator is absent
cons:
- the lua savepoint table ({ csavepoint=csavepoint,
txn_id=builtin.box_txn_id() }) have defined in Lua and using in C part
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