From: Petr Špaček <petr.spacek@nic.cz>
Date: Fri, 23 Oct 2020 15:48:41 +0000 (+0200)
Subject: worker: migrate worker.id to string
X-Git-Tag: v5.2.0~6^2~4
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=822e4342d518bdffbc16acac5c8579d1b58596ee;p=thirdparty%2Fknot-resolver.git

worker: migrate worker.id to string

It now contains either content of SYSTEMD_INSTANCE environment variable
or stringified version of getpid() value.

Main motivation is that the old worker.id was broken on systemd,
i.e. the default installation.

Related: #631
---

diff --git a/daemon/bindings/worker.rst b/daemon/bindings/worker.rst
index e530c845c..9dfcbe83a 100644
--- a/daemon/bindings/worker.rst
+++ b/daemon/bindings/worker.rst
@@ -7,11 +7,15 @@ Worker is a service over event loop that tracks and schedules outstanding querie
 you can see the statistics or schedule new queries. It also contains information about
 specified worker count and process rank.
 
+.. envvar:: worker.id
+
+   Value from environment variable ``SYSTEMD_INSTANCE``,
+   or if it is not set, :envvar:`PID <worker.pid>` (string).
+
 .. envvar:: worker.pid
 
    Current worker process PID (number).
 
-
 .. function:: worker.stats()
 
    Return table of statistics.  See member descriptions in :c:type:`worker_stats`.
diff --git a/daemon/main.c b/daemon/main.c
index cd83a976d..28c998a75 100644
--- a/daemon/main.c
+++ b/daemon/main.c
@@ -496,7 +496,7 @@ int main(int argc, char **argv)
 		return EXIT_FAILURE;
 	}
 	/* Initialize the worker */
-	ret = worker_init(&engine, fork_id, the_args->forks);
+	ret = worker_init(&engine, the_args->forks);
 	if (ret != 0) {
 		kr_log_error("[system] failed to initialize worker: %s\n", kr_strerror(ret));
 		return EXIT_FAILURE;
diff --git a/daemon/worker.c b/daemon/worker.c
index e4e1cdc38..c420350dd 100644
--- a/daemon/worker.c
+++ b/daemon/worker.c
@@ -10,6 +10,7 @@
 #include <lauxlib.h>
 #include <libknot/packet/pkt.h>
 #include <libknot/descriptor.h>
+#include <contrib/cleanup.h>
 #include <contrib/ucw/lib.h>
 #include <contrib/ucw/mempool.h>
 #include <contrib/wire.h>
@@ -2046,7 +2047,7 @@ void worker_deinit(void)
 	the_worker = NULL;
 }
 
-int worker_init(struct engine *engine, int worker_id, int worker_count)
+int worker_init(struct engine *engine, int worker_count)
 {
 	assert(engine && engine->L);
 	assert(the_worker == NULL);
@@ -2060,7 +2061,6 @@ int worker_init(struct engine *engine, int worker_id, int worker_count)
 	uv_loop_t *loop = uv_default_loop();
 	worker->loop = loop;
 
-	worker->id = worker_id;
 	worker->count = worker_count;
 
 	/* Register table for worker per-request variables */
@@ -2080,9 +2080,20 @@ int worker_init(struct engine *engine, int worker_id, int worker_count)
 
 	/* Set some worker.* fields in Lua */
 	lua_getglobal(engine->L, "worker");
-	lua_pushnumber(engine->L, worker_id);
+	pid_t pid = getpid();
+
+	auto_free char *pid_str = NULL;
+	const char *inst_name = getenv("SYSTEMD_INSTANCE");
+	if (inst_name) {
+		lua_pushstring(engine->L, inst_name);
+	} else {
+		ret = asprintf(&pid_str, "%ld", (long)pid);
+		assert(ret > 0);
+		lua_pushstring(engine->L, pid_str);
+	}
 	lua_setfield(engine->L, -2, "id");
-	lua_pushnumber(engine->L, getpid());
+
+	lua_pushnumber(engine->L, pid);
 	lua_setfield(engine->L, -2, "pid");
 	lua_pushnumber(engine->L, worker_count);
 	lua_setfield(engine->L, -2, "count");
diff --git a/daemon/worker.h b/daemon/worker.h
index 0d7f3f82e..2e93617df 100644
--- a/daemon/worker.h
+++ b/daemon/worker.h
@@ -23,7 +23,7 @@ KR_EXPORT extern struct worker_ctx *the_worker;
 
 /** Create and initialize the worker.
  * \return error code (ENOMEM) */
-int worker_init(struct engine *engine, int worker_id, int worker_count);
+int worker_init(struct engine *engine, int worker_count);
 
 /** Destroy the worker (free memory). */
 void worker_deinit(void);
@@ -152,8 +152,7 @@ typedef array_t(struct qr_task *) qr_tasklist_t;
 struct worker_ctx {
 	struct engine *engine;
 	uv_loop_t *loop;
-	int id;
-	int count;
+	int count;  /** unreliable, does not count systemd instance, do not use */
 	int vars_table_ref;
 	unsigned tcp_pipeline_max;
 
diff --git a/doc/upgrading.rst b/doc/upgrading.rst
index 2c6d14e14..4f11fc1bc 100644
--- a/doc/upgrading.rst
+++ b/doc/upgrading.rst
@@ -36,12 +36,19 @@ newer versions when they are released.
 ==========
 
 Users
-~~~~~
+-----
 
 * Users of :ref:`control-sockets` API need to terminate each command sent to resolver with newline
   character (ASCII ``\n``). Correct usage: ``cache.stats()\n``.
   Newline terminated commands are accepted by all resolver versions >= 1.0.0.
 
+Configuration file
+------------------
+
+* Lua variable :envvar:`worker.id` is now a string with either Systemd instance name or PID
+  (instead of number). If your custom configuration uses :envvar:`worker.id` value please
+  check your scripts.
+
 Module changes
 --------------
 * Reply packet :c:type:`kr_request.answer`