[Tarantool-patches] [PATCH v3 01/10] fiber: use uint64_t for fiber IDs

Cyrill Gorcunov gorcunov at gmail.com
Tue May 4 18:58:10 MSK 2021


Historically we use uint32_t for fiber IDs. And these IDs were
wrapping in time, especially if instance is running for a long
period. Strictly speaking this is not very convenient because if
some external tool gonna track fibers by their IDs it might get
confused (or miss) by IDs wrapping.

Lets rather switch to wide integers and fixup outputs (such as
snprintf callers, Lua's fid reports and etc). This allows us to
simplify code a bit and forget about IDs wrapping at all.

Same time wrong output specificators resulted in weird informal
lines

 | main/-244760339/cartridge.failover.task I> Instance state changed

Thus changing IDs type forced us to review all printouts and fix
formatting to not confuse users.

Closes #5846

Signed-off-by: Cyrill Gorcunov <gorcunov at gmail.com>
---
 changelogs/unreleased/gh-5846-cformat.md  | 12 +++++++++
 changelogs/unreleased/gh-5846-fiber-id.md |  4 +++
 src/lib/core/fiber.c                      | 30 ++++++++++-----------
 src/lib/core/fiber.h                      |  8 +++---
 src/lib/core/say.c                        |  8 +++---
 src/lua/fiber.c                           | 33 ++++++++++++-----------
 6 files changed, 57 insertions(+), 38 deletions(-)
 create mode 100644 changelogs/unreleased/gh-5846-cformat.md
 create mode 100644 changelogs/unreleased/gh-5846-fiber-id.md

diff --git a/changelogs/unreleased/gh-5846-cformat.md b/changelogs/unreleased/gh-5846-cformat.md
new file mode 100644
index 000000000..027de35d7
--- /dev/null
+++ b/changelogs/unreleased/gh-5846-cformat.md
@@ -0,0 +1,12 @@
+## bugfix/core
+ * Fixed wrong type specificator when printing fiber state
+   change which lead to negative fiber's ID logging.
+
+   For exmaple
+   ```
+   main/-244760339/cartridge.failover.task I> Instance state changed
+   ```
+   instead of proper
+   ```
+   main/4050206957/cartridge.failover.task I> Instance state changed
+   ```
diff --git a/changelogs/unreleased/gh-5846-fiber-id.md b/changelogs/unreleased/gh-5846-fiber-id.md
new file mode 100644
index 000000000..b645da849
--- /dev/null
+++ b/changelogs/unreleased/gh-5846-fiber-id.md
@@ -0,0 +1,4 @@
+## feature/core
+ * Fiber IDs are switched to monotonically increasing unsigned 8 byte
+   integers so that there won't be IDs wrapping anymore. This allows
+   to detect fiber's precedence by their IDs if needed.
diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
index 196dffe26..216f8be5d 100644
--- a/src/lib/core/fiber.c
+++ b/src/lib/core/fiber.c
@@ -823,28 +823,28 @@ fiber_schedule_idle(ev_loop *loop, ev_idle *watcher,
 
 
 struct fiber *
-fiber_find(uint32_t fid)
+fiber_find(uint64_t fid)
 {
-	struct mh_i32ptr_t *fiber_registry = cord()->fiber_registry;
-	mh_int_t k = mh_i32ptr_find(fiber_registry, fid, NULL);
+	struct mh_i64ptr_t *fiber_registry = cord()->fiber_registry;
+	mh_int_t k = mh_i64ptr_find(fiber_registry, fid, NULL);
 
 	if (k == mh_end(fiber_registry))
 		return NULL;
-	return (struct fiber *) mh_i32ptr_node(fiber_registry, k)->val;
+	return mh_i64ptr_node(fiber_registry, k)->val;
 }
 
 static void
 register_fid(struct fiber *fiber)
 {
-	struct mh_i32ptr_node_t node = { fiber->fid, fiber };
-	mh_i32ptr_put(cord()->fiber_registry, &node, NULL, NULL);
+	struct mh_i64ptr_node_t node = { fiber->fid, fiber };
+	mh_i64ptr_put(cord()->fiber_registry, &node, NULL, NULL);
 }
 
 static void
 unregister_fid(struct fiber *fiber)
 {
-	struct mh_i32ptr_node_t node = { fiber->fid, NULL };
-	mh_i32ptr_remove(cord()->fiber_registry, &node, NULL);
+	struct mh_i64ptr_node_t node = { fiber->fid, NULL };
+	mh_i64ptr_remove(cord()->fiber_registry, &node, NULL);
 }
 
 struct fiber *
@@ -1257,13 +1257,13 @@ fiber_new_ex(const char *name, const struct fiber_attr *fiber_attr,
 	}
 
 	fiber->f = f;
-	/* Excluding reserved range */
-	if (++cord->max_fid < FIBER_ID_MAX_RESERVED)
-		cord->max_fid = FIBER_ID_MAX_RESERVED + 1;
-	fiber->fid = cord->max_fid;
+	fiber->fid = cord->next_fid;
 	fiber_set_name(fiber, name);
 	register_fid(fiber);
 
+	cord->next_fid++;
+	assert(cord->next_fid > FIBER_ID_MAX_RESERVED);
+
 	return fiber;
 
 }
@@ -1449,7 +1449,7 @@ cord_create(struct cord *cord, const char *name)
 	rlist_create(&cord->alive);
 	rlist_create(&cord->ready);
 	rlist_create(&cord->dead);
-	cord->fiber_registry = mh_i32ptr_new();
+	cord->fiber_registry = mh_i64ptr_new();
 
 	/* sched fiber is not present in alive/ready/dead list. */
 	cord->sched.fid = FIBER_ID_SCHED;
@@ -1461,7 +1461,7 @@ cord_create(struct cord *cord, const char *name)
 	cord->fiber = &cord->sched;
 	cord->sched.flags |= FIBER_IS_RUNNING;
 
-	cord->max_fid = FIBER_ID_MAX_RESERVED;
+	cord->next_fid = FIBER_ID_MAX_RESERVED + 1;
 	/*
 	 * No need to start this event since it's only used for
 	 * ev_feed_event(). Saves a few cycles on every
@@ -1502,7 +1502,7 @@ cord_destroy(struct cord *cord)
 	/* Only clean up if initialized. */
 	if (cord->fiber_registry) {
 		fiber_destroy_all(cord);
-		mh_i32ptr_delete(cord->fiber_registry);
+		mh_i64ptr_delete(cord->fiber_registry);
 	}
 	region_destroy(&cord->sched.gc);
 	diag_destroy(&cord->sched.diag);
diff --git a/src/lib/core/fiber.h b/src/lib/core/fiber.h
index 586bc9433..8f4e14796 100644
--- a/src/lib/core/fiber.h
+++ b/src/lib/core/fiber.h
@@ -550,7 +550,7 @@ struct fiber {
 	/** Number of context switches. */
 	int csw;
 	/** Fiber id. */
-	uint32_t fid;
+	uint64_t fid;
 	/** Fiber flags */
 	uint32_t flags;
 #if ENABLE_FIBER_TOP
@@ -661,7 +661,7 @@ struct cord {
 	 * Every new fiber gets a new monotonic id. Ids 0 - 100 are
 	 * reserved.
 	 */
-	uint32_t max_fid;
+	uint64_t next_fid;
 #if ENABLE_FIBER_TOP
 	struct clock_stat clock_stat;
 	struct cpu_stat cpu_stat;
@@ -669,7 +669,7 @@ struct cord {
 	pthread_t id;
 	const struct cord_on_exit *on_exit;
 	/** A helper hash to map id -> fiber. */
-	struct mh_i32ptr_t *fiber_registry;
+	struct mh_i64ptr_t *fiber_registry;
 	/** All fibers */
 	struct rlist alive;
 	/** Fibers, ready for execution */
@@ -828,7 +828,7 @@ void
 fiber_call(struct fiber *callee);
 
 struct fiber *
-fiber_find(uint32_t fid);
+fiber_find(uint64_t fid);
 
 void
 fiber_schedule_cb(ev_loop * /* loop */, ev_watcher *watcher, int revents);
diff --git a/src/lib/core/say.c b/src/lib/core/say.c
index cbd10e107..5307767b5 100644
--- a/src/lib/core/say.c
+++ b/src/lib/core/say.c
@@ -792,8 +792,9 @@ say_format_plain_tail(char *buf, int len, int level, const char *filename,
 	if (cord) {
 		SNPRINT(total, snprintf, buf, len, " %s", cord->name);
 		if (fiber() && fiber()->fid != FIBER_ID_SCHED) {
-			SNPRINT(total, snprintf, buf, len, "/%i/%s",
-				fiber()->fid, fiber_name(fiber()));
+			SNPRINT(total, snprintf, buf, len, "/%lld/%s",
+				(long long)fiber()->fid,
+				fiber_name(fiber()));
 		}
 	}
 
@@ -918,7 +919,8 @@ say_format_json(struct log *log, char *buf, int len, int level, const char *file
 		SNPRINT(total, snprintf, buf, len, "\"");
 		if (fiber() && fiber()->fid != FIBER_ID_SCHED) {
 			SNPRINT(total, snprintf, buf, len,
-				", \"fiber_id\": %i, ", fiber()->fid);
+				", \"fiber_id\": %llu, ",
+				(long long)fiber()->fid);
 			SNPRINT(total, snprintf, buf, len,
 				"\"fiber_name\": \"");
 			SNPRINT(total, json_escape, buf, len,
diff --git a/src/lua/fiber.c b/src/lua/fiber.c
index 02ec3d158..c792bf385 100644
--- a/src/lua/fiber.c
+++ b/src/lua/fiber.c
@@ -113,7 +113,7 @@ lbox_create_weak_table(struct lua_State *L, const char *name)
  * Push a userdata for the given fiber onto Lua stack.
  */
 static void
-lbox_pushfiber(struct lua_State *L, int fid)
+lbox_pushfiber(struct lua_State *L, uint64_t fid)
 {
 	/*
 	 * Use 'memoize'  pattern and keep a single userdata for
@@ -131,22 +131,22 @@ lbox_pushfiber(struct lua_State *L, int fid)
 		lbox_create_weak_table(L, "memoize");
 	}
 	/* Find out whether the fiber is  already in the memoize table. */
-	lua_pushinteger(L, fid);
+	luaL_pushuint64(L, fid);
 	lua_gettable(L, -2);
 	if (lua_isnil(L, -1)) {
 		/* no userdata for fiber created so far */
 		/* pop the nil */
 		lua_pop(L, 1);
 		/* push the key back */
-		lua_pushinteger(L, fid);
+		luaL_pushuint64(L, fid);
 		/* create a new userdata */
-		int *ptr = (int *) lua_newuserdata(L, sizeof(int));
+		uint64_t *ptr = lua_newuserdata(L, sizeof(*ptr));
 		*ptr = fid;
 		luaL_getmetatable(L, fiberlib_name);
 		lua_setmetatable(L, -2);
 		/* memoize it */
 		lua_settable(L, -3);
-		lua_pushinteger(L, fid);
+		luaL_pushuint64(L, fid);
 		/* get it back */
 		lua_gettable(L, -2);
 	}
@@ -155,11 +155,11 @@ lbox_pushfiber(struct lua_State *L, int fid)
 static struct fiber *
 lbox_checkfiber(struct lua_State *L, int index)
 {
-	uint32_t fid;
+	uint64_t fid;
 	if (lua_type(L, index) == LUA_TNUMBER) {
-		fid = lua_tonumber(L, index);
+		fid = luaL_touint64(L, index);
 	} else {
-		fid = *(uint32_t *) luaL_checkudata(L, index, fiberlib_name);
+		fid = *(uint64_t *) luaL_checkudata(L, index, fiberlib_name);
 	}
 	struct fiber *f = fiber_find(fid);
 	if (f == NULL)
@@ -170,12 +170,12 @@ lbox_checkfiber(struct lua_State *L, int index)
 static int
 lbox_fiber_id(struct lua_State *L)
 {
-	uint32_t fid;
+	uint64_t fid;
 	if (lua_gettop(L)  == 0)
 		fid = fiber()->fid;
 	else
-		fid = *(uint32_t *) luaL_checkudata(L, 1, fiberlib_name);
-	lua_pushinteger(L, fid);
+		fid = *(uint64_t *) luaL_checkudata(L, 1, fiberlib_name);
+	luaL_pushuint64(L, fid);
 	return 1;
 }
 
@@ -275,7 +275,7 @@ lbox_fiber_statof(struct fiber *f, void *cb_ctx, bool backtrace)
 	lua_settable(L, -3);
 
 	lua_pushstring(L, "fid");
-	lua_pushnumber(L, f->fid);
+	luaL_pushuint64(L, f->fid);
 	lua_settable(L, -3);
 
 	lua_pushstring(L, "csw");
@@ -539,7 +539,7 @@ lbox_fiber_status(struct lua_State *L)
 {
 	struct fiber *f;
 	if (lua_gettop(L)) {
-		uint32_t fid = *(uint32_t *)
+		uint64_t fid = *(uint64_t *)
 			luaL_checkudata(L, 1, fiberlib_name);
 		f = fiber_find(fid);
 	} else {
@@ -704,7 +704,7 @@ lbox_fiber_find(struct lua_State *L)
 {
 	if (lua_gettop(L) != 1)
 		luaL_error(L, "fiber.find(id): bad arguments");
-	int fid = lua_tonumber(L, -1);
+	uint64_t fid = luaL_touint64(L, -1);
 	struct fiber *f = fiber_find(fid);
 	if (f)
 		lbox_pushfiber(L, f->fid);
@@ -736,7 +736,7 @@ lbox_fiber_serialize(struct lua_State *L)
 {
 	struct fiber *f = lbox_checkfiber(L, 1);
 	lua_createtable(L, 0, 1);
-	lua_pushinteger(L, f->fid);
+	luaL_pushuint64(L, f->fid);
 	lua_setfield(L, -2, "id");
 	lua_pushstring(L, fiber_name(f));
 	lua_setfield(L, -2, "name");
@@ -750,7 +750,8 @@ lbox_fiber_tostring(struct lua_State *L)
 {
 	char buf[20];
 	struct fiber *f = lbox_checkfiber(L, 1);
-	snprintf(buf, sizeof(buf), "fiber: %d", f->fid);
+	snprintf(buf, sizeof(buf), "fiber: %llu",
+		 (long long)f->fid);
 	lua_pushstring(L, buf);
 	return 1;
 }
-- 
2.30.2



More information about the Tarantool-patches mailing list