fix(ha-commit-engine): raise commit-barrier default 50->500ms + add --sync-timeout-ms

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.
This commit is contained in:
Rongsong Shen 2026-06-14 20:19:05 +00:00
parent c4fd3e1c8f
commit ca1d9ae42d
5 changed files with 24 additions and 1 deletions

View file

@ -1139,6 +1139,10 @@ int cluster_init_phase2(cluster_t* c)
: "controller"; : "controller";
persist_config_t pcfg = persist_config_defaults(); 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); persist_set_workdir(&c->persist, workdir, name);
rc = persist_init(&c->persist, &pcfg, c); rc = persist_init(&c->persist, &pcfg, c);
if (rc != 0) { if (rc != 0) {

View file

@ -29,6 +29,7 @@ const char* argp_program_bug_address = BUG_EMAIL;
#define OPT_CONF_STALE_TIMEOUT 258 #define OPT_CONF_STALE_TIMEOUT 258
#define OPT_VCPUS 259 #define OPT_VCPUS 259
#define OPT_CERTDIR 260 #define OPT_CERTDIR 260
#define OPT_SYNC_TIMEOUT 261
struct arguments { struct arguments {
char* myname; char* myname;
@ -40,6 +41,7 @@ struct arguments {
long conf_stale_timeout; long conf_stale_timeout;
int vcpus; int vcpus;
char* certdir; char* certdir;
uint32_t sync_timeout_ms;
}; };
static struct argp_option options[] = { 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)" }, "Number of virtual CPUs for the task manager (must be >= 1)" },
{ "certdir", OPT_CERTDIR, "DIR", 0, { "certdir", OPT_CERTDIR, "DIR", 0,
"Directory containing xce.crt and xce.key (default: <workdir>/certs)" }, "Directory containing xce.crt and xce.key (default: <workdir>/certs)" },
{ "sync-timeout-ms", OPT_SYNC_TIMEOUT, "MS", 0,
"Synchronous majority-replication commit barrier timeout in ms (0 = default)" },
{ 0 } { 0 }
}; };
@ -100,6 +104,13 @@ static error_t parse_opt(int key, char* arg, struct argp_state* state)
case OPT_CERTDIR: case OPT_CERTDIR:
arguments->certdir = arg; arguments->certdir = arg;
break; 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: default:
return ARGP_ERR_UNKNOWN; return ARGP_ERR_UNKNOWN;
} }
@ -151,6 +162,7 @@ int main(int argc, char* argv[])
arguments.conf_keep_n = 2; arguments.conf_keep_n = 2;
arguments.conf_stale_timeout = 86400; arguments.conf_stale_timeout = 86400;
arguments.vcpus = 0; /* 0 = not set; use cluster config default */ 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); 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_keep_n = arguments.conf_keep_n;
myconf.conf_stale_timeout = (time_t)arguments.conf_stale_timeout; myconf.conf_stale_timeout = (time_t)arguments.conf_stale_timeout;
myconf.vcpus = arguments.vcpus; myconf.vcpus = arguments.vcpus;
myconf.sync_timeout_ms = arguments.sync_timeout_ms;
memset(&myconf.peers_cfg, 0, sizeof(myconf.peers_cfg)); memset(&myconf.peers_cfg, 0, sizeof(myconf.peers_cfg));
if (arguments.peers_str) { if (arguments.peers_str) {

View file

@ -60,6 +60,11 @@ static void test_persist_config_defaults_finished_retain_s(void)
static void test_persist_config_defaults_sync_timeout_ms(void) static void test_persist_config_defaults_sync_timeout_ms(void)
{ {
persist_config_t cfg = persist_config_defaults(); 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); TEST_ASSERT_EQUAL_UINT32(DEFAULT_SYNC_TIMEOUT_MS, cfg.sync_timeout_ms);
} }

View file

@ -90,7 +90,7 @@ typedef uint64_t jobid_t;
#define DEFAULT_TRANSIENT_JOB_TIMEOUT_S 300 #define DEFAULT_TRANSIENT_JOB_TIMEOUT_S 300
#define DEFAULT_FINISHED_RETAIN_S 3600 #define DEFAULT_FINISHED_RETAIN_S 3600
#define DEFAULT_WORKER_RETAIN_S 86400 #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_HB_INTERVAL_MS 3000
#define DEFAULT_MAX_WAL_RECORDS 10000 #define DEFAULT_MAX_WAL_RECORDS 10000

View file

@ -35,6 +35,7 @@ typedef struct {
time_t conf_stale_timeout; /* seconds before saved config flagged stale (default 86400) */ 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 conf_stale; /* 1 when saved config older than conf_stale_timeout */
int vcpus; /* Req 4.3: --vcpus CLI value; 0 = use config default */ 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; } myconf_t;
extern cluster_config_t* get_default_conf(void*); extern cluster_config_t* get_default_conf(void*);