[tarantool-patches] [PATCH 0/2] Box.session.sync

[tarantool-patches] [PATCH 0/2] Box.session.sync

[Vladislav Shpilevoy]

Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-3450-session-sync
Issue: https://github.com/tarantool/tarantool/issues/3450

Vladislav Shpilevoy (2):
  fiber: remove fiber local storage
  session: fix box.session.sync()

 src/box/iproto.cc             | 11 ++++--
 src/box/session.cc            |  4 +-
 src/box/session.h             | 31 ++++++---------
 src/box/txn.c                 |  2 +-
 src/box/txn.h                 |  2 +-
 src/fiber.c                   |  9 +----
 src/fiber.h                   | 89 ++++++++++++++++++-------------------------
 src/fiber_channel.c           | 31 +++++----------
 src/lua/fiber.c               | 19 ++++-----
 test/box-tap/session.test.lua | 29 +++++++++++++-
 10 files changed, 106 insertions(+), 121 deletions(-)

-- 
2.15.1 (Apple Git-101)

[tarantool-patches] [PATCH 1/2] fiber: remove fiber local storage

[Vladislav Shpilevoy]

Replace it with more specific structures and pointers in order to
prepare to add `net` storage.

This allows to make the code working with fiber storage simpler,
remove useless wrappers and casts, and in the next patch - remove
broken session.sync and add fiber sync.
---
 src/box/session.cc  |  4 +--
 src/box/session.h   | 14 ++++------
 src/box/txn.c       |  2 +-
 src/box/txn.h       |  2 +-
 src/fiber.c         |  9 ++----
 src/fiber.h         | 80 +++++++++++++++++++----------------------------------
 src/fiber_channel.c | 31 ++++++---------------
 src/lua/fiber.c     | 19 ++++++-------
 8 files changed, 56 insertions(+), 105 deletions(-)

diff --git a/src/box/session.cc b/src/box/session.cc
index 60adc5639..f868e9ecc 100644
--- a/src/box/session.cc
+++ b/src/box/session.cc
@@ -86,10 +86,8 @@ session_on_stop(struct trigger *trigger, void * /* event */)
 	 * after the trigger and its memory is long gone.
 	 */
 	trigger_clear(trigger);
-	struct session *session = (struct session *)
-		fiber_get_key(fiber(), FIBER_KEY_SESSION);
 	/* Destroy the session */
-	session_destroy(session);
+	session_destroy(fiber_get_session(fiber()));
 }
 
 struct session *
diff --git a/src/box/session.h b/src/box/session.h
index f4e9b4109..17190ab47 100644
--- a/src/box/session.h
+++ b/src/box/session.h
@@ -154,7 +154,7 @@ extern struct rlist session_on_auth;
 static inline struct session *
 fiber_get_session(struct fiber *fiber)
 {
-	return (struct session *) fiber_get_key(fiber, FIBER_KEY_SESSION);
+	return fiber->storage.session;
 }
 
 /**
@@ -165,13 +165,13 @@ fiber_get_session(struct fiber *fiber)
 static inline void
 fiber_set_user(struct fiber *fiber, struct credentials *cr)
 {
-	fiber_set_key(fiber, FIBER_KEY_USER, cr);
+	fiber->storage.credentials = cr;
 }
 
 static inline void
 fiber_set_session(struct fiber *fiber, struct session *session)
 {
-	fiber_set_key(fiber, FIBER_KEY_SESSION, session);
+	fiber->storage.session = session;
 }
 
 static inline void
@@ -227,13 +227,11 @@ current_session()
 static inline struct credentials *
 effective_user()
 {
-	struct credentials *u =
-		(struct credentials *) fiber_get_key(fiber(),
-						     FIBER_KEY_USER);
+	struct fiber *f = fiber();
+	struct credentials *u = f->storage.credentials;
 	if (u == NULL) {
 		session_create_on_demand();
-		u = (struct credentials *) fiber_get_key(fiber(),
-							 FIBER_KEY_USER);
+		u = f->storage.credentials;
 	}
 	return u;
 }
diff --git a/src/box/txn.c b/src/box/txn.c
index e25c0e0e0..5714d485d 100644
--- a/src/box/txn.c
+++ b/src/box/txn.c
@@ -40,7 +40,7 @@ double too_long_threshold;
 static inline void
 fiber_set_txn(struct fiber *fiber, struct txn *txn)
 {
-	fiber_set_key(fiber, FIBER_KEY_TXN, (void *) txn);
+	fiber->storage.txn = txn;
 }
 
 static int
diff --git a/src/box/txn.h b/src/box/txn.h
index 4db74dfca..2bec73ec1 100644
--- a/src/box/txn.h
+++ b/src/box/txn.h
@@ -142,7 +142,7 @@ struct txn {
 static inline struct txn *
 in_txn()
 {
-	return (struct txn *) fiber_get_key(fiber(), FIBER_KEY_TXN);
+	return fiber()->storage.txn;
 }
 
 /**
diff --git a/src/fiber.c b/src/fiber.c
index 9eb7aed50..75a4846d4 100644
--- a/src/fiber.c
+++ b/src/fiber.c
@@ -612,7 +612,8 @@ fiber_recycle(struct fiber *fiber)
 	fiber_reset(fiber);
 	fiber->name[0] = '\0';
 	fiber->f = NULL;
-	memset(fiber->fls, 0, sizeof(fiber->fls));
+	fiber->wait_pad = NULL;
+	memset(&fiber->storage, 0, sizeof(fiber->storage));
 	unregister_fid(fiber);
 	fiber->fid = 0;
 	region_free(&fiber->gc);
@@ -682,12 +683,6 @@ fiber_set_name(struct fiber *fiber, const char *name)
 	snprintf(fiber->name, sizeof(fiber->name), "%s", name);
 }
 
-extern inline void
-fiber_set_key(struct fiber *fiber, enum fiber_key key, void *value);
-
-extern inline void *
-fiber_get_key(struct fiber *fiber, enum fiber_key key);
-
 static inline void *
 page_align_down(void *ptr)
 {
diff --git a/src/fiber.h b/src/fiber.h
index 8231bba24..b7003aeb6 100644
--- a/src/fiber.h
+++ b/src/fiber.h
@@ -92,24 +92,6 @@ enum {
 	FIBER_DEFAULT_FLAGS = FIBER_IS_CANCELLABLE
 };
 
-/**
- * \brief Pre-defined key for fiber local storage
- */
-enum fiber_key {
-	/** box.session */
-	FIBER_KEY_SESSION = 0,
-	/** Lua fiber.storage */
-	FIBER_KEY_LUA_STORAGE = 1,
-	/** transaction */
-	FIBER_KEY_TXN = 2,
-	/** User global privilege and authentication token */
-	FIBER_KEY_USER = 3,
-	FIBER_KEY_MSG = 4,
-	/** Storage for lua stack */
-	FIBER_KEY_LUA_STACK = 5,
-	FIBER_KEY_MAX = 6
-};
-
 /** \cond public */
 
 /**
@@ -349,6 +331,12 @@ struct fiber_attr {
 void
 fiber_attr_create(struct fiber_attr *fiber_attr);
 
+struct session;
+struct txn;
+struct credentials;
+struct lua_State;
+struct ipc_wait_pad;
+
 struct fiber {
 	coro_context ctx;
 	/** Coro stack slab. */
@@ -395,8 +383,28 @@ struct fiber {
 	fiber_func f;
 	va_list f_data;
 	int f_ret;
-	/** Fiber local storage */
-	void *fls[FIBER_KEY_MAX];
+	/** Fiber local storage. */
+	struct {
+		/**
+		 * Current transaction, session and the active
+		 * user credentials are shared among multiple
+		 * requests and valid even out of a former.
+		 */
+		struct session *session;
+		struct credentials *credentials;
+		struct txn *txn;
+		/**
+		 * Fields used by a fiber created in Lua: Lua
+		 * stack and the optional fiber.storage Lua
+		 * reference.
+		 */
+		struct {
+			struct lua_State *stack;
+			int ref;
+		} lua;
+	} storage;
+	/** An object to wait for incoming message or a reader. */
+	struct ipc_wait_pad *wait_pad;
 	/** Exception which caused this fiber's death. */
 	struct diag diag;
 	char name[FIBER_NAME_MAX];
@@ -577,44 +585,12 @@ fiber_find(uint32_t fid);
 void
 fiber_schedule_cb(ev_loop * /* loop */, ev_watcher *watcher, int revents);
 
-/**
- * \brief Associate \a value with \a key in fiber local storage
- * \param fiber fiber
- * \param key pre-defined key
- * \param value value to set
- */
-inline void
-fiber_set_key(struct fiber *fiber, enum fiber_key key, void *value)
-{
-	assert(key < FIBER_KEY_MAX);
-	fiber->fls[key] = value;
-}
-
-
 static inline bool
 fiber_is_dead(struct fiber *f)
 {
 	return f->flags & FIBER_IS_DEAD;
 }
 
-/**
- * \brief Retrieve value by \a key from fiber local storage
- * \param fiber fiber
- * \param key pre-defined key
- * \return value from from fiber local storage
- */
-inline void *
-fiber_get_key(struct fiber *fiber, enum fiber_key key)
-{
-	assert(key < FIBER_KEY_MAX);
-	return fiber->fls[key];
-}
-
-/**
- * Finalizer callback
- * \sa fiber_key_on_gc()
- */
-typedef void (*fiber_key_gc_cb)(enum fiber_key key, void *arg);
 typedef int (*fiber_stat_cb)(struct fiber *f, void *ctx);
 
 int
diff --git a/src/fiber_channel.c b/src/fiber_channel.c
index 99e51a47e..4bbff45c5 100644
--- a/src/fiber_channel.c
+++ b/src/fiber_channel.c
@@ -85,9 +85,7 @@ fiber_channel_has_waiter(struct fiber_channel *ch,
 	if (rlist_empty(&ch->waiters))
 		return false;
 	struct fiber *f = rlist_first_entry(&ch->waiters, struct fiber, state);
-	struct ipc_wait_pad *pad = (struct ipc_wait_pad *)
-		fiber_get_key(f, FIBER_KEY_MSG);
-	return pad->status == status;
+	return f->wait_pad->status == status;
 }
 
 bool
@@ -134,14 +132,12 @@ static inline void
 fiber_channel_waiter_wakeup(struct fiber *f,
 			    enum fiber_channel_wait_status status)
 {
-	struct ipc_wait_pad *pad = (struct ipc_wait_pad *)
-		fiber_get_key(f, FIBER_KEY_MSG);
 	/*
 	 * Safe to overwrite the status without looking at it:
 	 * whoever is touching the status, removes the fiber
 	 * from the wait list.
 	 */
-	pad->status = status;
+	f->wait_pad->status = status;
 	/*
 	 * fiber_channel allows an asynchronous cancel. If a fiber
 	 * is cancelled while waiting on a timeout, it is done via
@@ -317,10 +313,7 @@ fiber_channel_put_msg_timeout(struct fiber_channel *ch,
 							    struct fiber,
 							    state);
 			/* Place the message on the pad. */
-			struct ipc_wait_pad *pad = (struct ipc_wait_pad *)
-				fiber_get_key(f, FIBER_KEY_MSG);
-
-			pad->msg = msg;
+			f->wait_pad->msg = msg;
 
 			fiber_channel_waiter_wakeup(f, FIBER_CHANNEL_WAIT_DONE);
 			return 0;
@@ -355,7 +348,7 @@ fiber_channel_put_msg_timeout(struct fiber_channel *ch,
 		struct ipc_wait_pad pad;
 		pad.status = FIBER_CHANNEL_WAIT_WRITER;
 		pad.msg = msg;
-		fiber_set_key(f, FIBER_KEY_MSG, &pad);
+		f->wait_pad = &pad;
 
 		if (first_try) {
 			rlist_add_tail_entry(&ch->waiters, f, state);
@@ -370,7 +363,7 @@ fiber_channel_put_msg_timeout(struct fiber_channel *ch,
 		 * rlist_del_entry() is a no-op if already done.
 		 */
 		rlist_del_entry(f, state);
-		fiber_set_key(f, FIBER_KEY_MSG, NULL);
+		f->wait_pad = NULL;
 
 		if (pad.status == FIBER_CHANNEL_WAIT_CLOSED) {
 			/*
@@ -423,10 +416,7 @@ fiber_channel_get_msg_timeout(struct fiber_channel *ch,
 				f = rlist_first_entry(&ch->waiters,
 						      struct fiber,
 						      state);
-				struct ipc_wait_pad *pad =
-					(struct ipc_wait_pad *)
-					fiber_get_key(f, FIBER_KEY_MSG);
-				fiber_channel_buffer_push(ch, pad->msg);
+				fiber_channel_buffer_push(ch, f->wait_pad->msg);
 				fiber_channel_waiter_wakeup(f,
 					FIBER_CHANNEL_WAIT_DONE);
 			}
@@ -445,10 +435,7 @@ fiber_channel_get_msg_timeout(struct fiber_channel *ch,
 			f = rlist_first_entry(&ch->waiters,
 					      struct fiber,
 					      state);
-			struct ipc_wait_pad *pad =
-				(struct ipc_wait_pad *)
-				fiber_get_key(f, FIBER_KEY_MSG);
-			*msg = pad->msg;
+			*msg = f->wait_pad->msg;
 			fiber_channel_waiter_wakeup(f, FIBER_CHANNEL_WAIT_DONE);
 			return 0;
 		}
@@ -461,7 +448,7 @@ fiber_channel_get_msg_timeout(struct fiber_channel *ch,
 		 */
 		struct ipc_wait_pad pad;
 		pad.status = FIBER_CHANNEL_WAIT_READER;
-		fiber_set_key(f, FIBER_KEY_MSG, &pad);
+		f->wait_pad = &pad;
 		if (first_try) {
 			rlist_add_tail_entry(&ch->waiters, f, state);
 			first_try = false;
@@ -475,7 +462,7 @@ fiber_channel_get_msg_timeout(struct fiber_channel *ch,
 		 * rlist_del_entry() is a no-op if already done.
 		 */
 		rlist_del_entry(f, state);
-		fiber_set_key(f, FIBER_KEY_MSG, NULL);
+		f->wait_pad = NULL;
 		if (pad.status == FIBER_CHANNEL_WAIT_CLOSED) {
 			diag_set(ChannelIsClosed);
 			return -1;
diff --git a/src/lua/fiber.c b/src/lua/fiber.c
index 312edd3d5..147add89b 100644
--- a/src/lua/fiber.c
+++ b/src/lua/fiber.c
@@ -294,15 +294,14 @@ static int
 lua_fiber_run_f(MAYBE_UNUSED va_list ap)
 {
 	int result;
-	struct lua_State *L = (struct lua_State *)
-		fiber_get_key(fiber(), FIBER_KEY_LUA_STACK);
+	struct fiber *f = fiber();
+	struct lua_State *L = f->storage.lua.stack;
 	int coro_ref = lua_tointeger(L, -1);
 	lua_pop(L, 1);
 	result = luaT_call(L, lua_gettop(L) - 1, LUA_MULTRET);
 
 	/* Destroy local storage */
-	int storage_ref = (int)(intptr_t)
-		fiber_get_key(fiber(), FIBER_KEY_LUA_STORAGE);
+	int storage_ref = f->storage.lua.ref;
 	if (storage_ref > 0)
 		luaL_unref(L, LUA_REGISTRYINDEX, storage_ref);
 	/*
@@ -310,7 +309,7 @@ lua_fiber_run_f(MAYBE_UNUSED va_list ap)
 	 * We can unref child stack here,
 	 * otherwise we have to unref child stack in join
 	 */
-	if (fiber()->flags & FIBER_IS_JOINABLE)
+	if (f->flags & FIBER_IS_JOINABLE)
 		lua_pushinteger(L, coro_ref);
 	else
 		luaL_unref(L, LUA_REGISTRYINDEX, coro_ref);
@@ -343,7 +342,7 @@ fiber_create(struct lua_State *L)
 	 * At that time we can pop coro_ref from stack
 	 */
 	lua_pushinteger(child_L, coro_ref);
-	fiber_set_key(f, FIBER_KEY_LUA_STACK, child_L);
+	f->storage.lua.stack = child_L;
 	return f;
 }
 
@@ -467,13 +466,11 @@ static int
 lbox_fiber_storage(struct lua_State *L)
 {
 	struct fiber *f = lbox_checkfiber(L, 1);
-	int storage_ref = (int)(intptr_t)
-		fiber_get_key(f, FIBER_KEY_LUA_STORAGE);
+	int storage_ref = f->storage.lua.ref;
 	if (storage_ref <= 0) {
 		lua_newtable(L); /* create local storage on demand */
 		storage_ref = luaL_ref(L, LUA_REGISTRYINDEX);
-		fiber_set_key(f, FIBER_KEY_LUA_STORAGE,
-			      (void *)(intptr_t) storage_ref);
+		f->storage.lua.ref = storage_ref;
 	}
 	lua_rawgeti(L, LUA_REGISTRYINDEX, storage_ref);
 	return 1;
@@ -613,7 +610,7 @@ static int
 lbox_fiber_join(struct lua_State *L)
 {
 	struct fiber *fiber = lbox_checkfiber(L, 1);
-	struct lua_State *child_L = fiber_get_key(fiber, FIBER_KEY_LUA_STACK);
+	struct lua_State *child_L = fiber->storage.lua.stack;
 	fiber_join(fiber);
 	struct error *e = NULL;
 	int num_ret = 0;
-- 
2.15.1 (Apple Git-101)

[tarantool-patches] [PATCH 2/2] session: fix box.session.sync()

[Vladislav Shpilevoy]

Before the patch box.session.sync() is global for the session and
is updated on each new iproto request. When the connection is
multiplexed, box.session.sync() can be changed with no finishing
a current request, if a new one arrives.

The patch makes box.session.push() local for the request,
protecting it from multiplexing mess. Box.session.sync() after
the patch can be safely used inside a request.

Closes #3450

@TarantoolBot document
Title: box.session.sync() became request local
Box.session.sync() was global for a session, so it was unusable
when the connection behind the session is multiplexed. Now
box.session.sync() is request local and can be safely used inside
the request processor.
---
 src/box/iproto.cc             | 11 +++++++----
 src/box/session.h             | 17 ++++++-----------
 src/fiber.h                   | 27 ++++++++++++++++++---------
 test/box-tap/session.test.lua | 29 ++++++++++++++++++++++++++++-
 4 files changed, 59 insertions(+), 25 deletions(-)

diff --git a/src/box/iproto.cc b/src/box/iproto.cc
index 0102aec4b..759945278 100644
--- a/src/box/iproto.cc
+++ b/src/box/iproto.cc
@@ -1171,7 +1171,8 @@ error:
 static void
 tx_fiber_init(struct session *session, uint64_t sync)
 {
-	session->meta.sync = sync;
+	struct fiber *f = fiber();
+	f->storage.net.sync = sync;
 	/*
 	 * We do not cleanup fiber keys at the end of each request.
 	 * This does not lead to privilege escalation as long as
@@ -1182,8 +1183,8 @@ tx_fiber_init(struct session *session, uint64_t sync)
 	 * background tasks clean up their session in on_stop
 	 * trigger as well.
 	 */
-	fiber_set_session(fiber(), session);
-	fiber_set_user(fiber(), &session->credentials);
+	fiber_set_session(f, session);
+	fiber_set_user(f, &session->credentials);
 }
 
 /**
@@ -1818,7 +1819,9 @@ iproto_session_fd(struct session *session)
 int64_t
 iproto_session_sync(struct session *session)
 {
-	return session->meta.sync;
+	(void) session;
+	assert(session == fiber()->storage.session);
+	return fiber()->storage.net.sync;
 }
 
 /** {{{ IPROTO_PUSH implementation. */
diff --git a/src/box/session.h b/src/box/session.h
index 17190ab47..6ed97dc46 100644
--- a/src/box/session.h
+++ b/src/box/session.h
@@ -66,16 +66,11 @@ extern const char *session_type_strs[];
  * types, and allows to do not store attributes in struct session,
  * that are used only by a session of particular type.
  */
-struct session_meta {
-	union {
-		/** IProto connection meta. */
-		struct {
-			uint64_t sync;
-			void *connection;
-		};
-		/** Only by console is used. */
-		int fd;
-	};
+union session_meta {
+	/** IProto connection. */
+	void *connection;
+	/** Console file/socket descriptor. */
+	int fd;
 };
 
 /**
@@ -92,7 +87,7 @@ struct session {
 	uint64_t id;
 	enum session_type type;
 	/** Session metadata. */
-	struct session_meta meta;
+	union session_meta meta;
 	/** Session user id and global grants */
 	struct credentials credentials;
 	/** Trigger for fiber on_stop to cleanup created on-demand session */
diff --git a/src/fiber.h b/src/fiber.h
index b7003aeb6..021929822 100644
--- a/src/fiber.h
+++ b/src/fiber.h
@@ -393,15 +393,24 @@ struct fiber {
 		struct session *session;
 		struct credentials *credentials;
 		struct txn *txn;
-		/**
-		 * Fields used by a fiber created in Lua: Lua
-		 * stack and the optional fiber.storage Lua
-		 * reference.
-		 */
-		struct {
-			struct lua_State *stack;
-			int ref;
-		} lua;
+		union {
+			/**
+			 * Fields used by a fiber created in Lua:
+			 * Lua stack and the optional
+			 * fiber.storage Lua reference.
+			 */
+			struct {
+				struct lua_State *stack;
+				int ref;
+			} lua;
+			/**
+			 * Fields used by a fiber created to
+			 * process an iproto request.
+			 */
+			struct {
+				uint64_t sync;
+			} net;
+		};
 	} storage;
 	/** An object to wait for incoming message or a reader. */
 	struct ipc_wait_pad *wait_pad;
diff --git a/test/box-tap/session.test.lua b/test/box-tap/session.test.lua
index 6fddced3c..c3c07a67c 100755
--- a/test/box-tap/session.test.lua
+++ b/test/box-tap/session.test.lua
@@ -15,7 +15,7 @@ session = box.session
 space = box.schema.space.create('tweedledum')
 index = space:create_index('primary', { type = 'hash' })
 
-test:plan(53)
+test:plan(55)
 
 ---
 --- Check that Tarantool creates ADMIN session for #! script
@@ -183,6 +183,33 @@ local sp = conn:eval("return box.space._space.index.name:get{\"sp1\"}[2]")
 test:is(sp, 1, "effective ddl owner")
 conn:close()
 
+--
+-- gh-3450: box.session.sync() becomes request local.
+--
+cond = fiber.cond()
+local sync1, sync2
+local started = 0
+function f1()
+	started = started + 1
+	cond:wait()
+	sync1 = box.session.sync()
+end
+function f2()
+	started = started + 1
+	sync2 = box.session.sync()
+	cond:signal()
+end
+box.schema.user.grant('guest', 'read,write,execute', 'universe')
+conn = net.box.connect(box.cfg.listen)
+test:ok(conn:ping(), 'connect to self')
+_ = fiber.create(function() conn:call('f1') end)
+while started ~= 1 do fiber.sleep(0.01) end
+_ = fiber.create(function() conn:call('f2') end)
+while started ~= 2 do fiber.sleep(0.01) end
+test:isnt(sync1, sync2, 'session.sync() is request local')
+conn:close()
+box.schema.user.revoke('guest', 'read,write,execute', 'universe')
+
 inspector:cmd('stop server session with cleanup=1')
 session = nil
 os.exit(test:check() == true and 0 or -1)
-- 
2.15.1 (Apple Git-101)

[tarantool-patches] Re: [PATCH 0/2] Box.session.sync

[Vladislav Shpilevoy]

Sorry. Forgot the description.

The patchset fixes box.session.sync(). Before the patchset
box.session.sync() is session global, and is changed after
a new request arrives. So box.session.sync() can be safely
used only when a connection behind the session is not
multiplexed.

But box.session.push() can be used in a multiplexed
connection and uses box.session.sync() as a default
sync. So it needs box.session.sync() be request local.

On 09/06/2018 20:49, Vladislav Shpilevoy wrote:
> Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-3450-session-sync
> Issue: https://github.com/tarantool/tarantool/issues/3450
> 
> Vladislav Shpilevoy (2):
>    fiber: remove fiber local storage
>    session: fix box.session.sync()
> 
>   src/box/iproto.cc             | 11 ++++--
>   src/box/session.cc            |  4 +-
>   src/box/session.h             | 31 ++++++---------
>   src/box/txn.c                 |  2 +-
>   src/box/txn.h                 |  2 +-
>   src/fiber.c                   |  9 +----
>   src/fiber.h                   | 89 ++++++++++++++++++-------------------------
>   src/fiber_channel.c           | 31 +++++----------
>   src/lua/fiber.c               | 19 ++++-----
>   test/box-tap/session.test.lua | 29 +++++++++++++-
>   10 files changed, 106 insertions(+), 121 deletions(-)
> 

[tarantool-patches] Re: [PATCH 1/2] fiber: remove fiber local storage

[Konstantin Osipov]

* Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [18/06/09 22:33]:
> Replace it with more specific structures and pointers in order to
> prepare to add `net` storage.
> 
> This allows to make the code working with fiber storage simpler,
> remove useless wrappers and casts, and in the next patch - remove
> broken session.sync and add fiber sync.

Please write a comment stating that under no circumstances fiber.h
is allowed to include application-specific headers, one only is allowed to add opaque
pointers to the storage struct. I actually thought that you're
going to make it more general, adopting a concept similar to Lua
stack. Anyway, the simpler the better, we can make it more general
when we need it, it's OK to push.
> ---
>  src/box/session.cc  |  4 +--
>  src/box/session.h   | 14 ++++------
>  src/box/txn.c       |  2 +-
>  src/box/txn.h       |  2 +-
>  src/fiber.c         |  9 ++----
>  src/fiber.h         | 80 +++++++++++++++++++----------------------------------
>  src/fiber_channel.c | 31 ++++++---------------
>  src/lua/fiber.c     | 19 ++++++-------

-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov

[tarantool-patches] Re: [PATCH 2/2] session: fix box.session.sync()

[Konstantin Osipov]

* Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [18/06/09 22:33]:
> Before the patch box.session.sync() is global for the session and
> is updated on each new iproto request. When the connection is
> multiplexed, box.session.sync() can be changed with no finishing
> a current request, if a new one arrives.
> 
> The patch makes box.session.push() local for the request,
> protecting it from multiplexing mess. Box.session.sync() after
> the patch can be safely used inside a request.
> 
> Closes #3450

OK to push.


-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov

[tarantool-patches] Re: [PATCH 1/2] fiber: remove fiber local storage

[Vladislav Shpilevoy]

Hello. Thanks for the review!

On 14/06/2018 00:11, Konstantin Osipov wrote:
> * Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [18/06/09 22:33]:
>> Replace it with more specific structures and pointers in order to
>> prepare to add `net` storage.
>>
>> This allows to make the code working with fiber storage simpler,
>> remove useless wrappers and casts, and in the next patch - remove
>> broken session.sync and add fiber sync.
> 
> Please write a comment stating that under no circumstances fiber.h
> is allowed to include application-specific headers, one only is allowed to add opaque
> pointers to the storage struct. I actually thought that you're
> going to make it more general, adopting a concept similar to Lua
> stack. Anyway, the simpler the better, we can make it more general
> when we need it, it's OK to push.

I have added the comment to the code:

@@ -331,6 +331,11 @@ struct fiber_attr {
  void
  fiber_attr_create(struct fiber_attr *fiber_attr);
  
+/**
+ * Under no circumstances this header file is allowed to include
+ * application-specific headers like session.h or txn.h. One only
+ * is allowed to announce a struct and add opaque pointer to it.
+ */
  struct session;
  struct txn;
  struct credentials;

And to the commit message.

The patchset is pushed.


>> ---
>>   src/box/session.cc  |  4 +--
>>   src/box/session.h   | 14 ++++------
>>   src/box/txn.c       |  2 +-
>>   src/box/txn.h       |  2 +-
>>   src/fiber.c         |  9 ++----
>>   src/fiber.h         | 80 +++++++++++++++++++----------------------------------
>>   src/fiber_channel.c | 31 ++++++---------------
>>   src/lua/fiber.c     | 19 ++++++-------
>