wdgzyjx/myxce
1
0
Fork 0
forked from lei.chen/myxce
Code Pull requests Activity

fix(lib): normalize delaymgr to one CLOCK_MONOTONIC ms timescale, fixing seconds-task starvation

Browse source 
Root-caused via raft-ha-core-safety 5.10a investigation: schedule_checkpoint()
(persist.c) fires correctly once wal_record_count crosses its threshold, but
its delay_task()-scheduled checkpoint_task_fn never executed in a live
multi-node deployment -- confirmed live via temporary diagnostic logging
(wal_record_count=100 max=100 logged, but the task itself never ran).

Root cause: delay_task_t carried TWO incomparable time representations --
`nextrun` (wall-clock time_t seconds, used by delay_task()) and `nextrun_ts`
(CLOCK_MONOTONIC timespec, used by delay_task_ms()). task_cmp()'s mixed-type
fallback compared them directly: an ms-task's nextrun is always 0, a
seconds-task's nextrun is a real epoch value (e.g. 1785600123) -- so 0
always sorted "less than" any seconds-task, regardless of actual due time.
ordq (src/lib/ordq.c) keeps its queue as an ascending sorted linked list by
this comparator, and delay_worker() only ever inspects the list HEAD
(ordq_peek with no matchfn returns q->head immediately, no scanning). Since
cluster_t.delaymgr is shared with ha.c's ~10 recurring delay_task_ms() call
sites (heartbeats, election timers, RDHUP polling), the head was
structurally almost-always occupied by an ms-task -- any delay_task()
(seconds) caller sharing that delaymgr, including schedule_checkpoint's
checkpoint_task_fn, could be starved indefinitely.

Fix: eliminate the dual representation entirely. Every task -- whether
scheduled via delay_task() (seconds) or delay_task_ms() (milliseconds) --
now stores one flat uint64_t CLOCK_MONOTONIC millisecond deadline
(nextrun_ms). delay_task() becomes a thin seconds->ms wrapper around
delay_task_ms(). task_cmp() and task_due() collapse to a single integer
comparison; delay_worker()'s reschedule is a flat integer add (no more
tv_sec/tv_nsec carry arithmetic, no more is_ms_task branching). Net -42
lines. Live-verified: after this fix, a forced checkpoint round produces a
real installed snapshot file (previously: schedule_checkpoint fired but the
task silently never ran).

controller_unit_test: full 58-group sweep, 0 Failures, 0 ASAN reports
(before and after this change) -- no regression, including every
delaymgr-touching group (test_demotion_convergence, test_ha_start_election_async,
test_ha_core_safety, test_ha_startup, etc).
This commit is contained in:
Rongsong Shen 2026-07-02 03:40:08 +00:00
parent 4696f43a74
commit b9ad01878c
2 changed files with 55 additions and 97 deletions
Download patch file Download diff file Expand all files Collapse all files

12
src/include/delay.h
View file

@ -12,10 +12,16 @@
typedef int (*delay_task_entry_fn_t)(void*);
typedef int (*task_run_fn_t)(delay_task_entry_fn_t, void*);
/* Single normalized time domain: every task (whether scheduled via
* delay_task() seconds or delay_task_ms() milliseconds) is stored as one
* flat CLOCK_MONOTONIC millisecond deadline. There is no separate
* wall-clock/ms task "flavor" to distinguish -- delay_task() is a thin
* seconds->ms wrapper around delay_task_ms() (see delay.c), so every entry
* in the queue is always comparable on the same scale with a single
* integer comparison (no tv_sec/tv_nsec pair, no carry arithmetic). */
typedef struct {
time_t nextrun;
struct timespec nextrun_ts; /* non-zero tv_nsec flags an ms-task */
int run_period;
uint64_t nextrun_ms; /* CLOCK_MONOTONIC deadline, in ms */
int run_period_ms;
int runcount;
delay_task_entry_fn_t entry;
void* args;