fix(persist): interruptible fsync_worker sleep so persist_stop joins promptly
fsync_worker slept the full wal_sync_ms in one uninterruptible nanosleep()
before re-checking fsync_stop. persist_stop sets the flag and immediately
pthread_join()s, so the join blocked until the in-flight sleep returned —
up to a full wal_sync_ms per teardown. Tests set wal_sync_ms=5000 ("slow
fsync — no races"), so each persist_stop cost ~5s; across the ~7 suites
that use it the cumulative teardown time blew past the 300s controller-test
meson cap, killing the suite with SIGTERM.
Replace the nanosleep with pthread_cond_timedwait on a new fsync_mutex/
fsync_cond (added to persist_t, init in persist_init, destroyed in
persist_fini). persist_stop now sets fsync_stop and signals fsync_cond
under fsync_mutex before joining, so the worker wakes immediately. fdatasync
still runs under p->lock, released before re-acquiring fsync_mutex — no lock
nesting. The "commit: WAIT deadline expired" log lines were unrelated
intentional negative tests (50-300ms), not the cause.
Full controller suite now completes in ~10s (was >300s TIMEOUT); 0 failures.
Logged as bug-604.
This commit is contained in:
parent
5503c7a176
commit
8c11655e53
4 changed files with 552 additions and 414 deletions
File diff suppressed because it is too large
Load diff
|
|
@ -1775,3 +1775,26 @@ Implemented sub-spec 3/4 of ha-consistency-invariants autonomously (per-task imp
|
||||||
| 00:55 | Edited .kiro/specs/ha-prevote/tasks.md | inline fix | ~60 |
|
| 00:55 | Edited .kiro/specs/ha-prevote/tasks.md | inline fix | ~60 |
|
||||||
| 00:55 | Edited .kiro/specs/ha-prevote/tasks.md | inline fix | ~68 |
|
| 00:55 | Edited .kiro/specs/ha-prevote/tasks.md | inline fix | ~68 |
|
||||||
| 03:12 | ha-prevote cleanup: issue-1 log-reason fixed (47f7338); issue-2 leak deferred | .kiro/specs/ha-elect-ctx-lifecycle/, tasks.md, cerebrum.md | follow-up spec scaffolded + tracked | ~3k |
|
| 03:12 | ha-prevote cleanup: issue-1 log-reason fixed (47f7338); issue-2 leak deferred | .kiro/specs/ha-elect-ctx-lifecycle/, tasks.md, cerebrum.md | follow-up spec scaffolded + tracked | ~3k |
|
||||||
|
| 00:56 | Edited CLAUDE.md | removed 40 lines | ~23 |
|
||||||
|
| 00:56 | Session end: 21 writes across 12 files (tasks.md, test_ha_prevote.c, test_raft.c, research.md, ha.c) | 20 reads | ~57330 tok |
|
||||||
|
| 00:56 | Edited .claude/settings.json | removed 13 lines | ~11 |
|
||||||
|
| 00:57 | Edited .claude/settings.json | removed 13 lines | ~10 |
|
||||||
|
|
||||||
|
## Session: 2026-06-16 00:57
|
||||||
|
|
||||||
|
| Time | Action | File(s) | Outcome | ~Tokens |
|
||||||
|
|------|--------|---------|---------|--------|
|
||||||
|
| 00:59 | Created .mcp.json | — | ~7 |
|
||||||
|
| 01:07 | Edited .claude/settings.json | removed 13 lines | ~11 |
|
||||||
|
| 01:07 | Edited .claude/settings.json | removed 13 lines | ~10 |
|
||||||
|
| 01:09 | Created .mcp.json | — | ~7 |
|
||||||
|
| 01:09 | Created .claude/settings.json | — | ~497 |
|
||||||
|
| 01:09 | Created uninstall-code-review-graph.sh | — | ~627 |
|
||||||
|
| 01:10 | Session end: 6 writes across 3 files (.mcp.json, settings.json, uninstall-code-review-graph.sh) | 8 reads | ~1708 tok |
|
||||||
|
| 01:18 | Edited src/include/persist.h | 3→5 lines | ~69 |
|
||||||
|
| 01:18 | Edited src/controller/persist.c | 2→4 lines | ~52 |
|
||||||
|
| 01:18 | Edited src/controller/persist.c | added 2 condition(s) | ~383 |
|
||||||
|
| 01:18 | Edited src/controller/persist.c | 5→10 lines | ~155 |
|
||||||
|
| 01:19 | Edited src/controller/persist.c | 4→6 lines | ~59 |
|
||||||
|
| 15:55 | Fixed controller test timeout: interruptible fsync_worker sleep (condvar) so persist_stop joins promptly | persist.c, persist.h | suite 300s+ TIMEOUT → 10s, 0 failures | ~25k |
|
||||||
|
| 01:23 | Session end: 11 writes across 5 files (.mcp.json, settings.json, uninstall-code-review-graph.sh, persist.h, persist.c) | 9 reads | ~2478 tok |
|
||||||
|
|
|
||||||
|
|
@ -174,19 +174,37 @@ static void* fsync_worker(void* arg)
|
||||||
{
|
{
|
||||||
persist_t* p = arg;
|
persist_t* p = arg;
|
||||||
|
|
||||||
|
/* Sleep the configured interval via a timed condvar wait rather than an
|
||||||
|
* uninterruptible nanosleep, so persist_stop wakes us immediately instead
|
||||||
|
* of blocking its pthread_join for up to a full wal_sync_ms (5s in tests
|
||||||
|
* that set a slow interval — was the controller-suite timeout cause). */
|
||||||
|
pthread_mutex_lock(&p->fsync_mutex);
|
||||||
while (!atomic_load_explicit(&p->fsync_stop, memory_order_acquire)) {
|
while (!atomic_load_explicit(&p->fsync_stop, memory_order_acquire)) {
|
||||||
long ms = (long)p->config.wal_sync_ms;
|
long ms = (long)p->config.wal_sync_ms;
|
||||||
struct timespec ts = {
|
struct timespec dl;
|
||||||
.tv_sec = ms / 1000,
|
|
||||||
.tv_nsec = (ms % 1000) * 1000000L,
|
|
||||||
};
|
|
||||||
nanosleep(&ts, NULL);
|
|
||||||
|
|
||||||
|
clock_gettime(CLOCK_REALTIME, &dl);
|
||||||
|
dl.tv_sec += ms / 1000;
|
||||||
|
dl.tv_nsec += (ms % 1000) * 1000000L;
|
||||||
|
if (dl.tv_nsec >= 1000000000L) {
|
||||||
|
dl.tv_sec += 1;
|
||||||
|
dl.tv_nsec -= 1000000000L;
|
||||||
|
}
|
||||||
|
pthread_cond_timedwait(&p->fsync_cond, &p->fsync_mutex, &dl);
|
||||||
|
|
||||||
|
/* Stop requested during the wait: exit before the next fdatasync. */
|
||||||
|
if (atomic_load_explicit(&p->fsync_stop, memory_order_acquire))
|
||||||
|
break;
|
||||||
|
|
||||||
|
/* fdatasync under p->lock — never held together with fsync_mutex. */
|
||||||
|
pthread_mutex_unlock(&p->fsync_mutex);
|
||||||
pthread_mutex_lock(&p->lock);
|
pthread_mutex_lock(&p->lock);
|
||||||
if (p->wal_fd >= 0)
|
if (p->wal_fd >= 0)
|
||||||
fdatasync(p->wal_fd);
|
fdatasync(p->wal_fd);
|
||||||
pthread_mutex_unlock(&p->lock);
|
pthread_mutex_unlock(&p->lock);
|
||||||
|
pthread_mutex_lock(&p->fsync_mutex);
|
||||||
}
|
}
|
||||||
|
pthread_mutex_unlock(&p->fsync_mutex);
|
||||||
return NULL;
|
return NULL;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -253,6 +271,8 @@ int persist_init(persist_t* p, persist_config_t* cfg, cluster_t* c)
|
||||||
pthread_mutex_init(&p->lock, NULL);
|
pthread_mutex_init(&p->lock, NULL);
|
||||||
pthread_mutex_init(&p->checkpoint_mutex, NULL);
|
pthread_mutex_init(&p->checkpoint_mutex, NULL);
|
||||||
pthread_cond_init(&p->checkpoint_done, NULL);
|
pthread_cond_init(&p->checkpoint_done, NULL);
|
||||||
|
pthread_mutex_init(&p->fsync_mutex, NULL);
|
||||||
|
pthread_cond_init(&p->fsync_cond, NULL);
|
||||||
repl_init(&p->repl); /* P2-4: fan-out replicator lifecycle */
|
repl_init(&p->repl); /* P2-4: fan-out replicator lifecycle */
|
||||||
|
|
||||||
WARN("WAL/snapshot stored unencrypted in %s — "
|
WARN("WAL/snapshot stored unencrypted in %s — "
|
||||||
|
|
@ -285,7 +305,12 @@ int persist_start(persist_t* p)
|
||||||
void persist_stop(persist_t* p)
|
void persist_stop(persist_t* p)
|
||||||
{
|
{
|
||||||
if (atomic_load_explicit(&p->fsync_thread_running, memory_order_acquire)) {
|
if (atomic_load_explicit(&p->fsync_thread_running, memory_order_acquire)) {
|
||||||
|
/* Set the stop flag and wake the timer wait under fsync_mutex so the
|
||||||
|
* worker cannot miss the signal between its flag check and the wait. */
|
||||||
|
pthread_mutex_lock(&p->fsync_mutex);
|
||||||
atomic_store_explicit(&p->fsync_stop, 1, memory_order_release);
|
atomic_store_explicit(&p->fsync_stop, 1, memory_order_release);
|
||||||
|
pthread_cond_signal(&p->fsync_cond);
|
||||||
|
pthread_mutex_unlock(&p->fsync_mutex);
|
||||||
pthread_join(p->fsync_thread, NULL);
|
pthread_join(p->fsync_thread, NULL);
|
||||||
atomic_store_explicit(&p->fsync_thread_running, 0, memory_order_release);
|
atomic_store_explicit(&p->fsync_thread_running, 0, memory_order_release);
|
||||||
}
|
}
|
||||||
|
|
@ -305,6 +330,8 @@ void persist_fini(persist_t* p)
|
||||||
pthread_mutex_destroy(&p->lock);
|
pthread_mutex_destroy(&p->lock);
|
||||||
pthread_mutex_destroy(&p->checkpoint_mutex);
|
pthread_mutex_destroy(&p->checkpoint_mutex);
|
||||||
pthread_cond_destroy(&p->checkpoint_done);
|
pthread_cond_destroy(&p->checkpoint_done);
|
||||||
|
pthread_mutex_destroy(&p->fsync_mutex);
|
||||||
|
pthread_cond_destroy(&p->fsync_cond);
|
||||||
}
|
}
|
||||||
|
|
||||||
/* Forward declaration — defined after checkpoint internals */
|
/* Forward declaration — defined after checkpoint internals */
|
||||||
|
|
|
||||||
|
|
@ -86,6 +86,8 @@ struct persist_s {
|
||||||
atomic_int fsync_stop;
|
atomic_int fsync_stop;
|
||||||
atomic_int fsync_thread_running;
|
atomic_int fsync_thread_running;
|
||||||
pthread_t fsync_thread;
|
pthread_t fsync_thread;
|
||||||
|
pthread_mutex_t fsync_mutex; /* guards the fsync timer wait */
|
||||||
|
pthread_cond_t fsync_cond; /* signaled by persist_stop for prompt join */
|
||||||
|
|
||||||
/* Checkpoint (stubs for now — wired in Task 10) */
|
/* Checkpoint (stubs for now — wired in Task 10) */
|
||||||
atomic_int checkpoint_requested;
|
atomic_int checkpoint_requested;
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue