Tarantool development patches archive
 help / color / mirror / Atom feed
From: Cyrill Gorcunov via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: tml <tarantool-patches@dev.tarantool.org>
Cc: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Subject: [Tarantool-patches] [PATCH v3 01/10] fiber: use uint64_t for fiber IDs
Date: Tue,  4 May 2021 18:58:10 +0300
Message-ID: <20210504155819.290874-2-gorcunov@gmail.com> (raw)
In-Reply-To: <20210504155819.290874-1-gorcunov@gmail.com>

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@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


  reply	other threads:[~2021-05-04 16:04 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-04 15:58 [Tarantool-patches] [PATCH v3 00/10] fix say_x format and rework fibers Cyrill Gorcunov via Tarantool-patches
2021-05-04 15:58 ` Cyrill Gorcunov via Tarantool-patches [this message]
2021-05-10 18:40   ` [Tarantool-patches] [PATCH v3 01/10] fiber: use uint64_t for fiber IDs Vladislav Shpilevoy via Tarantool-patches
2021-05-10 21:40     ` Cyrill Gorcunov via Tarantool-patches
2021-05-04 15:58 ` [Tarantool-patches] [PATCH v3 02/10] popen: fix say_x format arguments Cyrill Gorcunov via Tarantool-patches
2021-05-10 18:40   ` Vladislav Shpilevoy via Tarantool-patches
2021-05-10 21:41     ` Cyrill Gorcunov via Tarantool-patches
2021-05-04 15:58 ` [Tarantool-patches] [PATCH v3 03/10] raft: fix say_x arguments Cyrill Gorcunov via Tarantool-patches
2021-05-04 15:58 ` [Tarantool-patches] [PATCH v3 04/10] box/error: fix argument for CustomError Cyrill Gorcunov via Tarantool-patches
2021-05-04 15:58 ` [Tarantool-patches] [PATCH v3 05/10] xlog: fix say_x format Cyrill Gorcunov via Tarantool-patches
2021-05-04 15:58 ` [Tarantool-patches] [PATCH v3 06/10] box/vynil: " Cyrill Gorcunov via Tarantool-patches
2021-05-10 18:40   ` Vladislav Shpilevoy via Tarantool-patches
2021-05-10 21:51     ` Cyrill Gorcunov via Tarantool-patches
2021-05-04 15:58 ` [Tarantool-patches] [PATCH v3 07/10] txn: " Cyrill Gorcunov via Tarantool-patches
2021-05-04 15:58 ` [Tarantool-patches] [PATCH v3 08/10] limbo: " Cyrill Gorcunov via Tarantool-patches
2021-05-04 15:58 ` [Tarantool-patches] [PATCH v3 09/10] wal: " Cyrill Gorcunov via Tarantool-patches
2021-05-10 18:40   ` Vladislav Shpilevoy via Tarantool-patches
2021-05-10 21:55     ` Cyrill Gorcunov via Tarantool-patches
2021-05-04 15:58 ` [Tarantool-patches] [PATCH v3 10/10] say: fix CFORMAT specification Cyrill Gorcunov via Tarantool-patches
2021-05-10 18:40   ` Vladislav Shpilevoy via Tarantool-patches
2021-05-10 21:58     ` Cyrill Gorcunov via Tarantool-patches
2021-05-11 20:13 ` [Tarantool-patches] [PATCH v3 00/10] fix say_x format and rework fibers Vladislav Shpilevoy via Tarantool-patches
2021-05-11 21:15   ` Cyrill Gorcunov via Tarantool-patches
2021-05-11 21:24     ` Cyrill Gorcunov via Tarantool-patches
2021-05-12 18:41       ` Vladislav Shpilevoy via Tarantool-patches
2021-05-13  8:17         ` Cyrill Gorcunov via Tarantool-patches
2021-05-13 11:17           ` Vladislav Shpilevoy via Tarantool-patches
2021-05-13 11:44             ` Cyrill Gorcunov via Tarantool-patches
2021-05-13 12:09               ` Vladislav Shpilevoy via Tarantool-patches
2021-05-14  7:56 ` Kirill Yukhin via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210504155819.290874-2-gorcunov@gmail.com \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=gorcunov@gmail.com \
    --cc=v.shpilevoy@tarantool.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Tarantool development patches archive

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://lists.tarantool.org/tarantool-patches/0 tarantool-patches/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 tarantool-patches tarantool-patches/ https://lists.tarantool.org/tarantool-patches \
		tarantool-patches@dev.tarantool.org.
	public-inbox-index tarantool-patches

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git