Skip to content

Hand the replication-ack wait to a background thread (port of #1963, with measurements) - #6144

Draft
emelialei88 wants to merge 1 commit into
bloomberg:mainfrom
emelialei88:port/async-dist-commit
Draft

Hand the replication-ack wait to a background thread (port of #1963, with measurements)#6144
emelialei88 wants to merge 1 commit into
bloomberg:mainfrom
emelialei88:port/async-dist-commit

Conversation

@emelialei88

Copy link
Copy Markdown
Contributor

Port of #1963 (3126 commits behind main, hand-ported rather than rebased) onto current main, plus an A/B measurement of what it actually buys.

Why. Block processors commit locally and then block in trans_wait_for_seqnum_int until every replicant acks. With the default 8 writer threads, that caps in-flight commits at 8 no matter how many clients are connected.

What. A dedicated seqnum-wait thread takes the wait; the block processor returns to the queue immediately. The ack still happens, so replicants stay bounded — unlike rep-sync none, which drops it and drove two nodes permanently incoherent inside one 5-minute run at 64 clients. Off by default (async_dist_commit), togglable at runtime.

Rather than carry #1963's copy of bdb_wait_for_seqnum_from_all_int — which predated durable-majority accounting, gbl_2pc, non_durable_retry and set_durable_lsn — the shared logic is factored out of main into bdb_wait_for_seqnum_finish and bdb_wait_for_seqnum_mark_incoherent.

Measured. 8-node btda cluster, 300s runs, single-row autocommit INSERTs, one table per client, inserts/sec:

clients control waiter on rep-sync none ctrl spread waiter gain
4 7,375 7,553 10,309 0.23% +2.4%
8 11,656 12,077 14,111 1.02% +3.6%
16 12,146 12,455 13,722 0.81% +2.5%
32 12,040 12,453 13,653 1.31% +3.4%

Scope confirmed reachable: 726,674 commits enqueued vs 726,658 rows, zero inline fallbacks.

Risk / why this is a draft. The gain is +2.4–3.6% and flat across client counts, so writer-thread starvation was not the binding constraint — the working hypothesis going in. Raising async_dist_commit_max_outstanding_trans 8→32→128 does not help either. rep-sync none shows 12–35% of real headroom, but this is not the lever that reaches it. Opening for the port and the numbers; I would not merge it on a 3% return for ~500 lines of concurrent code that has to be kept in step with bdb_wait_for_seqnum_from_all_int forever.

Three latent bugs found and fixed during the port, which would have hit the original too: thrman_register(THRTYPE_GENERIC) made begin_clean_exit() wait forever on a thread that never exits; the waiter took BDB_READLOCK with no bdb lock slot and killed the master with [FATAL] bdb lock not inited in this thread; and gbl_set_coherent_state_trace defaults to 1, whose per-node-per-poll trace is catastrophic for a polling waiter.

Port of bloomberg#1963 onto current main. Block processors commit locally and then
block in trans_wait_for_seqnum_int until every replicant acks, so with the
default 8 writer threads only 8 commits can be in flight regardless of
client concurrency. This hands the wait to a dedicated seqnum-wait thread
and returns the block processor to the queue immediately; the ack is still
performed, so replicants stay bounded.

Off by default (async_dist_commit). The thread always starts but stays idle
until work is queued, so the tunable can be flipped at runtime.

Rather than carry bloomberg#1963's copy of bdb_wait_for_seqnum_from_all_int, which
predated durable-majority accounting, gbl_2pc, non_durable_retry and
set_durable_lsn, the shared logic is factored out of main into
bdb_wait_for_seqnum_finish and bdb_wait_for_seqnum_mark_incoherent.

Signed-off-by: Emelia Lei <[email protected]>

@roborivers roborivers left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cbuild submission: Success ✓.
Regression testing: Success ✓.

The first 10 failing tests are:
reco-ddlk-sql **quarantined**
consumer_non_atomic_default_consumer_generated **quarantined**
tunables
sc_downgrade [timeout] **quarantined**

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants