Skip to content

Commit 1798c83

Browse files
committed
tm: render the anycast Via cid from the live node id, not a frozen one
tm_init_cluster() read this node cluster id once, at mod_init, and baked it into the ";cid=<id>" Via parameter (and into a cached tm_node_id used to decide whether an anycast reply is ours). That assumes the id is known and stable by mod_init, which holds for a statically configured clusterer but not for a controller-managed one: there the id is assigned at runtime, after the node joins, and can change on re-election. So mod_init froze the still-unassigned id (-1, printed as its unsigned form 18446744073709551615) into every outgoing Via, and every node compared incoming cids against -1 - t_anycast_replicate() could never route a reply to the owning node. Read the id live instead: pre-build only the fixed ";<param>=" prefix at init, and have tm_via_cid() append the current get_my_id() per request (returning no parameter while the node still has no id); compare incoming cids against the live get_my_id() too. cl_get_my_id() already returns the runtime id, so this needs nothing from the caller and self-corrects across re-elections.
1 parent be7dbf2 commit 1798c83

2 files changed

Lines changed: 39 additions & 13 deletions

File tree

‎modules/tm/cluster.c‎

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,9 @@ str tm_cid;
3232
int tm_repl_cluster = 0;
3333
int tm_repl_auto_cancel = 1;
3434
str tm_cluster_param = str_init(TM_CLUSTER_DEFAULT_PARAM);
35-
static int tm_node_id = 0;
35+
/* length of the fixed ";<param>=" prefix of tm_cid; the node id is appended
36+
* live per-request by tm_via_cid() */
37+
static int tm_cid_prefix_len;
3638
static str tm_repl_cap = str_init("tm-repl");
3739

3840
struct clusterer_binds cluster_api;
@@ -256,8 +258,6 @@ static void receive_tm_repl(bin_packet_t *packet)
256258

257259
int tm_init_cluster(void)
258260
{
259-
str cid;
260-
261261
if (tm_repl_cluster == 0) {
262262
LM_DBG("tm_replication_cluster not set - not engaging!\n");
263263
return 0;
@@ -282,11 +282,13 @@ int tm_init_cluster(void)
282282
/* overwrite structure to disable clusterer */
283283
goto cluster_error;
284284
}
285-
tm_node_id = cluster_api.get_my_id();
286-
287-
/* build the via param */
288-
cid.s = int2str(tm_node_id, &cid.len);
289-
tm_cid.s = pkg_malloc(1/*;*/ + tm_cluster_param.len + 1/*=*/ + cid.len);
285+
/* Pre-build only the fixed ";<param>=" prefix. The node id is NOT read
286+
* here: under a controller-managed cluster it is assigned at runtime
287+
* (after this mod_init) and may change on re-election, so freezing it
288+
* now would stamp a stale (typically -1) id into every Via. tm_via_cid()
289+
* appends the current id per-request instead. Room is left for the
290+
* largest id an int2str() can render. */
291+
tm_cid.s = pkg_malloc(1/*;*/ + tm_cluster_param.len + 1/*=*/ + INT2STR_MAX_LEN);
290292
if (!tm_cid.s) {
291293
LM_ERR("out of pkg memory!\n");
292294
goto cluster_error;
@@ -296,8 +298,7 @@ int tm_init_cluster(void)
296298
memcpy(tm_cid.s + tm_cid.len, tm_cluster_param.s, tm_cluster_param.len);
297299
tm_cid.len += tm_cluster_param.len;
298300
tm_cid.s[tm_cid.len++] = '=';
299-
memcpy(tm_cid.s + tm_cid.len, cid.s, cid.len);
300-
tm_cid.len += cid.len;
301+
tm_cid_prefix_len = tm_cid.len;
301302

302303
return 0;
303304

@@ -306,6 +307,28 @@ int tm_init_cluster(void)
306307
return -1;
307308
}
308309

310+
str *tm_via_cid(void)
311+
{
312+
char *id_s;
313+
int id, id_len;
314+
315+
if (!tm_cluster_enabled())
316+
return NULL;
317+
318+
/* read this node's id live - it is only known once the node has been
319+
* assigned one in the cluster (immediately for a static clusterer,
320+
* after the runtime join for a controller-managed one) and can change
321+
* on re-election; advertise no cid until we actually have one */
322+
id = cluster_api.get_my_id();
323+
if (id < 0)
324+
return NULL;
325+
326+
id_s = int2str((unsigned long)id, &id_len);
327+
memcpy(tm_cid.s + tm_cid_prefix_len, id_s, id_len);
328+
tm_cid.len = tm_cid_prefix_len + id_len;
329+
return &tm_cid;
330+
}
331+
309332
#define TM_BIN_PUSH(_t, _f, _d) \
310333
do { \
311334
if (bin_push_##_t(&packet, _f) < 0) { \
@@ -522,7 +545,7 @@ int tm_reply_replicate(struct sip_msg *msg)
522545
/* if there was no parameter, or it was, but it was ours, handle it */
523546
if (cid < 0)
524547
return 0;
525-
if (cid == tm_node_id) {
548+
if (cid == cluster_api.get_my_id()) {
526549
LM_DBG("reply should be processed by us (%d)\n", cid);
527550
return 0;
528551
}

‎modules/tm/cluster.h‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,10 @@ int tm_anycast_cancel(struct sip_msg *msg);
5151
/* returns true if clusterer is enabled */
5252
#define tm_cluster_enabled() (cluster_api.register_capability != 0)
5353

54-
/* returns the via parameter for the cluster */
55-
#define tm_via_cid() (tm_cluster_enabled()?&tm_cid:0)
54+
/* Returns the ";<param>=<node_id>" Via parameter for the cluster, rendered
55+
* with this node's CURRENT clusterer id (read live, not frozen at init - the
56+
* id is assigned at runtime under a controller-managed cluster and may change
57+
* on re-election). NULL when clustering is off or no id is assigned yet. */
58+
str *tm_via_cid(void);
5659

5760
#endif /* _TM_CLUSTER_H_ */

0 commit comments

Comments
 (0)