Skip to content

Logfill bounded tranlog stream - #6171

Open
markhannum wants to merge 2 commits into
bloomberg:mainfrom
markhannum:logfill-bounded-tranlog-stream
Open

Logfill bounded tranlog stream#6171
markhannum wants to merge 2 commits into
bloomberg:mainfrom
markhannum:logfill-bounded-tranlog-stream

Conversation

@markhannum

Copy link
Copy Markdown
Contributor

Problem

A replicant running sql_logfill destroys one connection to the master per gap-fill, and each of those is a sockpool miss. Observed in production at roughly 890 log-requests/sec across seven replicants, sustained for hours.

Cause

request_logs_from_master() asks for an unbounded stream (NULL stop LSN, TRANLOG_FLAGS_BLOCK), so it never ends on its own. logfill consumes only as far as the gap and abandons the rest — in production the master averaged 636 rows per query against a gap of a couple of records. newsql_disconnect() will not donate a handle whose last response was not LAST_ROW, and cdb2_discard_unread_socket_data defaults to off, so the socket is closed rather than pooled and the next request misses.

This happens on both sides of 0ed0d3f3b: with it, logfill hangs up and reconnects explicitly; without it, consume_previous_query() drains at most 10 rows and then disconnects from inside cdb2_run_statement.

Fix

Pass gap_lsn as the stop LSN. tranlogEof() already terminates on it, so the result set ends with LAST_ROW, the existing consumed path skips the disconnect, and the connection persists across gap-fills.

A second hunk stops tranlogNext() from entering the blocking poll once it has returned maxLsn — that loop checks only blockLsn and timeout, so otherwise the bound would trade connection churn for a stall.

Test

tests/sql_logfill_socket_reuse.test drops every Nth rep message on one replicant to force out-of-order arrivals, then counts log-requests against master disconnects. A gap is an out-of-order arrival rather than simply being behind — gap_lsn is repdb's waiting_lsn — so dropping messages is what reproduces the production condition. Unbounded, disconnects track requests one-for-one; the test allows a tenth. Verified 8 consecutive passes with the fix, and confirmed failing without it.

@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: Error ⚠.
Regression testing: Success ✓.

The first 10 failing tests are:
logfill [db unavailable at finish] **quarantined**
sc_resume_logicalsc_generated **quarantined**
sp_snapshot_generated
comdb2sys_queueodh_generated
consumer_non_atomic_default_consumer_generated **quarantined**
sc_downgrade [timeout] **quarantined**

An unbounded stream never ends, so logfill abandoned it mid-result-set and
cdb2api dropped the socket rather than donating it back to sockpool.
Also stop tranlogNext from blocking once it has returned maxLsn.

Signed-off-by: Mark Hannum <[email protected]>
Drops every Nth rep message on one replicant to force gaps, then counts
log-requests against master disconnects: an abandoned result set forces a
disconnect, so the two track one-for-one when the stream is unbounded.

Signed-off-by: Mark Hannum <[email protected]>
@markhannum
markhannum force-pushed the logfill-bounded-tranlog-stream branch from cf8657c to 2873bd0 Compare August 28, 2026 14:51

@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: Error ⚠.
Regression testing: Success ✓.

The first 10 failing tests are:
logfill [db unavailable at finish] **quarantined**
ssl_san
consumer_non_atomic_default_consumer_generated **quarantined**
ssl_set_cmd
ssl_prefer
ssl_dbname
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