Skip to content

osql: don't send startgen with a zeroed rqid: fix for 'cldeadlock' test - #6175

Merged
markhannum merged 1 commit into
bloomberg:mainfrom
markhannum:fix-osql-rqid-zero
Aug 29, 2026
Merged

osql: don't send startgen with a zeroed rqid: fix for 'cldeadlock' test#6175
markhannum merged 1 commit into
bloomberg:mainfrom
markhannum:fix-osql-rqid-zero

Conversation

@markhannum

Copy link
Copy Markdown
Contributor

Two paths reach the master with rqid 0 after the session was unregistered:

  • osql_sock_start_int() called osql_end() on the bdb-lock-desired restart path even when keep_rqid was set. osql_end() unregisters from the checkboard, which zeroes clnt->osql.rqid — but keep_rqid means the caller is restarting this session and still needs it.
  • the master-swing retry resent the commit after osql_sock_restart() even when the restart had ended the session.

cldeadlock produces both conditions: truncate-under-cursor takes BDB_WRITELOCK so bdb_lock_desired is true, and the downgrade loop swings the master every 20s.

Keep the rqid when keep_rqid is set, and gate the resend on a live rqid.

Issue discovered bycldeadlock test.

@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:
ssl_san
sql_logfill_autodisable
consumer_non_atomic_default_consumer_generated **quarantined**
ssl_set_cmd
ssl_prefer
ssl_dbname
sc_downgrade [timeout] **quarantined**

osql_end() unregistered the session on the lock-desired restart path even
when keep_rqid was set, zeroing the rqid the caller still needed, and the
master-swing retry resent on an ended session. Keep the rqid when keep_rqid
is set, and gate the resend on a live rqid.

Signed-off-by: Mark Hannum <[email protected]>

@WalidNejmi WalidNejmi 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.

very interesting to look over this code and good catch!

@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:
reco-ddlk-sql **quarantined**
consumer_non_atomic_default_consumer_generated **quarantined**
sc_downgrade [timeout] **quarantined**

@markhannum
markhannum merged commit 8074db5 into bloomberg:main Aug 29, 2026
5 checks passed
@markhannum
markhannum deleted the fix-osql-rqid-zero branch August 29, 2026 15:28
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.

3 participants