Always enable check hb on blocked write for chunk transactions by chands10 · Pull Request #5864 · bloomberg/comdb2

chands10 · 2026-04-10T17:51:01Z

To help us review your pull request, please consider providing an overview of the following:

What is the type of the change (bug fix, feature, documentation and etc.) ?
What are the current behavior and expected behavior, if this is a bugfix ?
What are the steps required to reproduce the bug, if this is a bugfix ?
What is the current behavior and new behavior, if this is a feature change or enhancement ?
[Optional] Why is the new behavior better than the current behavior, if this is a feature change ?

akshatsikarwar · 2026-04-10T18:09:01Z

this timing guess is not robust. can we get cdb2buf_flush to indicate that failure was due to timeout?

when cdb2buf_flush returns -1 currently, it either means that sb is NULL, or that it was a timeout error. Can I just check that? Or can add new bool param to cdb2buf_flush *timeout_error

Added commit with new param to explicitly set it

roborivers

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

The first 10 failing tests are:
sc_resume_logicalsc_generated **quarantined**
consumer_non_atomic_default_consumer_generated **quarantined**
sc_truncate_lockorder_generated [timeout] **quarantined**
incoh_remsql [timeout] **quarantined**

roborivers

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

The first 10 failing tests are:
scindex_logicalsc_generated
sc_resume_logicalsc_generated **quarantined**
queuedb_rollover **quarantined**
cdb2api_chunk
consumer_non_atomic_default_consumer_generated **quarantined**
incoh_remsql [timeout] **quarantined**

roborivers

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

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

akshatsikarwar · 2026-04-13T19:58:05Z

-int CDB2BUF_FUNC(cdb2buf_flush)(COMDB2BUF *sb);
+/* flush output.  returns # of bytes written or <0 for error. If timeout_error is not NULL, it will be set to 1 if a
+ * timeout occurred */
+int CDB2BUF_FUNC(cdb2buf_flush)(COMDB2BUF *sb, int *timeout_error);


Introduce cdb2_flush_chk_timeout (or another suitable name), so all cdb2buf_flush calls don't have to be updated?

chands10 · 2026-04-14T14:39:36Z

+########################################
+## RUN WITHOUT FIX EXPLICITLY ENABLED ##
+########################################
 echo 'comdb2_config:check_hb_on_blocked_write=0' >> ${cfg}


Note: this means it's impossible to disable this for chunks

roborivers

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

The first 10 failing tests are:
sc_truncate [db unavailable at finish]
consumer_non_atomic_default_consumer_generated **quarantined**
api_tst
alias
cdb2api_chunk [timeout]
incoh_remsql [timeout] **quarantined**

chands10 · 2026-04-14T18:09:53Z

-        int64_t end_us = end.tv_sec * 1000000 + end.tv_usec;
-        int64_t start_us = start.tv_sec * 1000000 + start.tv_usec;
-        int64_t elapsed_us = end_us - start_us;
-        if (!is_begin && !hndl->is_read && hndl->in_trans && hndl->socket_timeout &&


Not sure how we haven't gotten seg faults here before because hndl could be NULL

chands10 · 2026-04-14T18:35:02Z

Maybe let's wait making more changes till after we get the first cdb2api merged rollout in

roborivers

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

The first 10 failing tests are:
truncatesc_offline_generated **quarantined**
sp_snapshot_generated
reco-ddlk-sql **quarantined**
consumer_non_atomic_default_consumer_generated **quarantined**
api_tst
dc
incoh_remsql [timeout] **quarantined**

roborivers

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

The first 10 failing tests are:
truncatesc_offline_generated **quarantined**
sc_resume_logicalsc_generated **quarantined**
sp_snapshot_generated
consumer_non_atomic_default_consumer_generated **quarantined**
api_tst
reco-ddlk-sql [timeout] **quarantined**

roborivers

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

roborivers

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

Signed-off-by: Salil Chandra <schandra107@bloomberg.net>

roborivers

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

The first 10 failing tests are:
sc_timepart [failed with core dumped] **quarantined**
sc_timepart_multiddl_generated
analyze **quarantined**
consumer_non_atomic_default_consumer_generated **quarantined**
tunables

chands10 force-pushed the chunk_hb branch from 2c79fa3 to eea4f67 Compare April 10, 2026 17:52

chands10 changed the title ~~Always enable check hn on blocked write for chunk transactions~~ Always enable check hb on blocked write for chunk transactions Apr 10, 2026

akshatsikarwar reviewed Apr 10, 2026

View reviewed changes

Comment thread cdb2api/cdb2api.c Outdated

akshatsikarwar reviewed Apr 10, 2026

View reviewed changes

chands10 force-pushed the chunk_hb branch 2 times, most recently from e2e3403 to 7e68096 Compare April 10, 2026 18:50

roborivers approved these changes Apr 10, 2026

View reviewed changes

roborivers approved these changes Apr 11, 2026

View reviewed changes

akshatsikarwar reviewed Apr 13, 2026

View reviewed changes

chands10 force-pushed the chunk_hb branch from 68bd7c1 to 8b25c13 Compare April 14, 2026 14:37

chands10 commented Apr 14, 2026

View reviewed changes

chands10 force-pushed the chunk_hb branch from 8b25c13 to 4265a69 Compare April 14, 2026 14:43

roborivers approved these changes Apr 14, 2026

View reviewed changes

chands10 force-pushed the chunk_hb branch from 4265a69 to 0ca39f6 Compare April 14, 2026 18:09

chands10 commented Apr 14, 2026

View reviewed changes

roborivers approved these changes Apr 14, 2026

View reviewed changes

chands10 marked this pull request as draft April 14, 2026 19:59

roborivers approved these changes Apr 14, 2026

View reviewed changes

roborivers approved these changes Apr 15, 2026

View reviewed changes

chands10 force-pushed the chunk_hb branch from 0ca39f6 to f7eb8a9 Compare April 21, 2026 17:03

roborivers suggested changes Apr 21, 2026

View reviewed changes

chands10 marked this pull request as ready for review April 22, 2026 14:37

Always enable check hb on blocked write for chunk transactions

fadd7ee

Signed-off-by: Salil Chandra <schandra107@bloomberg.net>

chands10 force-pushed the chunk_hb branch from f7eb8a9 to fadd7ee Compare April 22, 2026 20:46

roborivers approved these changes Apr 23, 2026

View reviewed changes

Conversation

chands10 commented Apr 10, 2026

Uh oh!

Uh oh!

akshatsikarwar Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

chands10 Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

chands10 Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

roborivers left a comment

Choose a reason for hiding this comment

Uh oh!

roborivers left a comment

Choose a reason for hiding this comment

Uh oh!

roborivers left a comment

Choose a reason for hiding this comment

Uh oh!

akshatsikarwar Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

chands10 Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

chands10 Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

roborivers left a comment

Choose a reason for hiding this comment

Uh oh!

chands10 Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

chands10 commented Apr 14, 2026

Uh oh!

roborivers left a comment

Choose a reason for hiding this comment

Uh oh!

roborivers left a comment

Choose a reason for hiding this comment

Uh oh!

roborivers left a comment

Choose a reason for hiding this comment

Uh oh!

roborivers left a comment

Choose a reason for hiding this comment

Uh oh!

roborivers left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants