From ca1d9ae42d4ddb4b407665ac619c5738db74e353 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Sun, 14 Jun 2026 20:19:05 +0000 Subject: [PATCH] fix(ha-commit-engine): raise commit-barrier default 50->500ms + add --sync-timeout-ms MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live symptom: jobsub intermittently failed with 'Connection timed out' even though the cluster had quorum. Root cause: the synchronous commit barrier waits only DEFAULT_SYNC_TIMEOUT_MS=50ms for durable majority replication, and that was not configurable. 50ms is too tight for durable sync majority replication — especially since standbys now fdatasync each replicated entry before ACK — so a multi-entry jobsub's later WAL entry intermittently misses the window: commit: WAIT deadline expired without commit (lsn=3 committed_lsn=2 timeout_ms=50) -> persist hook returns -ETIMEDOUT -> handle_jobsubmit rejects -> client ETIMEDOUT. Fix: - Raise DEFAULT_SYNC_TIMEOUT_MS 50 -> 500 (config.h) — realistic for durable sync majority replication, still bounded for failure detection. - Add a --sync-timeout-ms controller flag threaded arguments -> myconf -> persist pcfg (override applied before persist_init); 0 = use default, positive overrides — so operators can tune per environment. TDD: literal-pinned persist_config_defaults().sync_timeout_ms == 500. All timeout-sensitive tests already pass an explicit timeout_ms (none rely on the default), so none are slowed; full controller suite green under ASAN. No commit-engine/barrier/replication/wire change beyond the default value + plumbing. bug-561. --- src/controller/cluster.c | 4 ++++ src/controller/main.c | 13 +++++++++++++ src/controller/testsuite/test_cluster_persist.c | 5 +++++ src/include/config.h | 2 +- src/include/myconf.h | 1 + 5 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/controller/cluster.c b/src/controller/cluster.c index d151243..076cab2 100644 --- a/src/controller/cluster.c +++ b/src/controller/cluster.c @@ -1139,6 +1139,10 @@ int cluster_init_phase2(cluster_t* c) : "controller"; persist_config_t pcfg = persist_config_defaults(); + /* --sync-timeout-ms override (0 = keep the persist default). */ + if (c->myconf->sync_timeout_ms > 0) + pcfg.sync_timeout_ms = c->myconf->sync_timeout_ms; + persist_set_workdir(&c->persist, workdir, name); rc = persist_init(&c->persist, &pcfg, c); if (rc != 0) { diff --git a/src/controller/main.c b/src/controller/main.c index 12e35c5..ab346fb 100644 --- a/src/controller/main.c +++ b/src/controller/main.c @@ -29,6 +29,7 @@ const char* argp_program_bug_address = BUG_EMAIL; #define OPT_CONF_STALE_TIMEOUT 258 #define OPT_VCPUS 259 #define OPT_CERTDIR 260 +#define OPT_SYNC_TIMEOUT 261 struct arguments { char* myname; @@ -40,6 +41,7 @@ struct arguments { long conf_stale_timeout; int vcpus; char* certdir; + uint32_t sync_timeout_ms; }; static struct argp_option options[] = { @@ -58,6 +60,8 @@ static struct argp_option options[] = { "Number of virtual CPUs for the task manager (must be >= 1)" }, { "certdir", OPT_CERTDIR, "DIR", 0, "Directory containing xce.crt and xce.key (default: /certs)" }, + { "sync-timeout-ms", OPT_SYNC_TIMEOUT, "MS", 0, + "Synchronous majority-replication commit barrier timeout in ms (0 = default)" }, { 0 } }; @@ -100,6 +104,13 @@ static error_t parse_opt(int key, char* arg, struct argp_state* state) case OPT_CERTDIR: arguments->certdir = arg; break; + case OPT_SYNC_TIMEOUT: { + int v = atoi(arg); + if (v < 0) + argp_error(state, "--sync-timeout-ms must be >= 0 (got: %s)", arg); + arguments->sync_timeout_ms = (uint32_t)v; + break; + } default: return ARGP_ERR_UNKNOWN; } @@ -151,6 +162,7 @@ int main(int argc, char* argv[]) arguments.conf_keep_n = 2; arguments.conf_stale_timeout = 86400; arguments.vcpus = 0; /* 0 = not set; use cluster config default */ + arguments.sync_timeout_ms = 0; /* 0 = not set; use persist config default */ argp_parse(&argp, argc, argv, 0, 0, &arguments); @@ -199,6 +211,7 @@ int main(int argc, char* argv[]) myconf.conf_keep_n = arguments.conf_keep_n; myconf.conf_stale_timeout = (time_t)arguments.conf_stale_timeout; myconf.vcpus = arguments.vcpus; + myconf.sync_timeout_ms = arguments.sync_timeout_ms; memset(&myconf.peers_cfg, 0, sizeof(myconf.peers_cfg)); if (arguments.peers_str) { diff --git a/src/controller/testsuite/test_cluster_persist.c b/src/controller/testsuite/test_cluster_persist.c index 801611c..678045a 100644 --- a/src/controller/testsuite/test_cluster_persist.c +++ b/src/controller/testsuite/test_cluster_persist.c @@ -60,6 +60,11 @@ static void test_persist_config_defaults_finished_retain_s(void) static void test_persist_config_defaults_sync_timeout_ms(void) { persist_config_t cfg = persist_config_defaults(); + /* Default raised to 500 ms: realistic for durable sync majority + * replication (each standby fdatasyncs before ACK), still bounded for + * failure detection. Literal-pinned so a regression to the old 50 ms + * tight window fails here, not just on a live cluster. */ + TEST_ASSERT_EQUAL_UINT32(500, cfg.sync_timeout_ms); TEST_ASSERT_EQUAL_UINT32(DEFAULT_SYNC_TIMEOUT_MS, cfg.sync_timeout_ms); } diff --git a/src/include/config.h b/src/include/config.h index 11eeb1a..babe0b9 100644 --- a/src/include/config.h +++ b/src/include/config.h @@ -90,7 +90,7 @@ typedef uint64_t jobid_t; #define DEFAULT_TRANSIENT_JOB_TIMEOUT_S 300 #define DEFAULT_FINISHED_RETAIN_S 3600 #define DEFAULT_WORKER_RETAIN_S 86400 -#define DEFAULT_SYNC_TIMEOUT_MS 50 +#define DEFAULT_SYNC_TIMEOUT_MS 500 #define DEFAULT_HB_INTERVAL_MS 3000 #define DEFAULT_MAX_WAL_RECORDS 10000 diff --git a/src/include/myconf.h b/src/include/myconf.h index 934d287..3ca4604 100644 --- a/src/include/myconf.h +++ b/src/include/myconf.h @@ -35,6 +35,7 @@ typedef struct { time_t conf_stale_timeout; /* seconds before saved config flagged stale (default 86400) */ int conf_stale; /* 1 when saved config older than conf_stale_timeout */ int vcpus; /* Req 4.3: --vcpus CLI value; 0 = use config default */ + uint32_t sync_timeout_ms; /* --sync-timeout-ms; 0 = use persist default */ } myconf_t; extern cluster_config_t* get_default_conf(void*);