Skip to content

Commit 435b354

Browse files
committed
clusterer_controller: a member list says who belongs, not who is alive
cl_ctr_handle_member_list() refreshed last_seen for every IP in the list, via cl_ctr_upsert_peer_locked(). A MEMBER_LIST is a membership announcement: the master lists a node until it prunes it, whether that node is reachable or not. Treating it as evidence of liveness makes every receiver believe it has just heard from peers it has never heard from. That is what made a dead member immortal. cl_ctr_alive_bitmap() is built from last_seen, so a backup holding refreshed timestamps for a node that is down asserts to the whole cluster that it is up the moment that backup becomes master - and from then on nobody can age it out. It survives its own funeral. Seen in production: 10.22.20.241 stayed in cluster 243 for about a day, across a master change, after being rolled back to a build with no controller at all. Nothing was listening on its BIN port, and the two surviving nodes spent ~3,500 failed connects a day each trying to reach it - roughly 10,668 of one node's 10,801 daily ERROR lines, which buried every other error on the box. The member-list path now inserts peers it did not know about and leaves last_seen alone for peers it already tracks. Liveness continues to come only from direct evidence: a packet sent BY the peer, or the master's alive bitmap, which the master derives from packets it received itself. A newly learned peer is still seeded with last_seen = now, so it gets one purge window to prove itself instead of being dropped on the next tick. This terminates rather than oscillating: once the master prunes a dead node it stops listing it, and every receiver ages it out one window later.
1 parent 840be47 commit 435b354

1 file changed

Lines changed: 37 additions & 1 deletion

File tree

‎modules/clusterer_controller/clusterer_controller.c‎

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1939,6 +1939,41 @@ static cl_ctr_peer_t *cl_ctr_peer_by_ip_locked(cl_ctr_cluster_t *cl, const char
19391939
return NULL;
19401940
}
19411941

1942+
static void cl_ctr_upsert_peer_locked(const char *src_ip, cl_ctr_cluster_t *cl);
1943+
1944+
/**
1945+
* cl_ctr_learn_peer_locked() - membership WITHOUT a liveness claim.
1946+
*
1947+
* A MEMBER_LIST says who BELONGS to the cluster, not who is reachable now: the
1948+
* master lists a node until it prunes it, whether that node is up or not.
1949+
* Refreshing last_seen from such a list - which is what upsert does - makes
1950+
* every receiver believe it has just heard from peers it has never heard from.
1951+
*
1952+
* That is not cosmetic. cl_ctr_alive_bitmap() is built from last_seen, so a
1953+
* backup holding refreshed timestamps for a dead node asserts that node is
1954+
* alive to the entire cluster the moment it becomes master - and then nobody
1955+
* can age it out. A node survived in a live cluster for about a day that way,
1956+
* across a master change, while every TCP connect to it was refused.
1957+
*
1958+
* So: insert what we did not already know about, and leave last_seen of
1959+
* anything we do track to direct evidence only - cl_ctr_upsert_peer_locked()
1960+
* from a packet actually sent BY that peer, or the master's alive bitmap,
1961+
* which the master derives from its own direct evidence.
1962+
*
1963+
* A newly learned peer is still seeded with last_seen = now, deliberately: it
1964+
* gets one purge window to prove itself rather than being pruned on the next
1965+
* tick. This terminates - once the master prunes a dead node it stops listing
1966+
* it, and every receiver ages it out one window later.
1967+
*
1968+
* Must be called with cl->peers->lock held for write.
1969+
*/
1970+
static void cl_ctr_learn_peer_locked(const char *src_ip, cl_ctr_cluster_t *cl)
1971+
{
1972+
if (cl_ctr_peer_by_ip_locked(cl, src_ip))
1973+
return; /* already known - membership adds nothing */
1974+
cl_ctr_upsert_peer_locked(src_ip, cl);
1975+
}
1976+
19421977
/**
19431978
* cl_ctr_upsert_peer_locked() - insert or refresh a peer entry.
19441979
* Does NOT call cl_ctr_elect_master(cl); callers do that explicitly.
@@ -4251,7 +4286,8 @@ static void cl_ctr_handle_member_list(const char *payload, int payload_len,
42514286
ip_buf[CL_CTR_MAX_IP_LEN] = '\0';
42524287
if (ip_buf[0] == '\0')
42534288
continue;
4254-
cl_ctr_upsert_peer_locked(ip_buf, cl);
4289+
/* membership, not liveness - see cl_ctr_learn_peer_locked() */
4290+
cl_ctr_learn_peer_locked(ip_buf, cl);
42554291
for (_j = 0; _j < cl->peers->count; _j++) {
42564292
if (strcmp(cl->peers->entries[_j].ip, ip_buf) == 0) {
42574293
cl->peers->entries[_j].last_seq = 0;

0 commit comments

Comments
 (0)