Skip to content

enforce max call time #5587

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Dec 2, 2025
Merged

enforce max call time #5587

merged 1 commit into from
Dec 2, 2025

Conversation

@mohitkhullar
Copy link
Contributor

@mohitkhullar mohitkhullar commented Nov 20, 2025

from bb

chands10
chands10 requested changes Nov 20, 2025
View reviewed changes
@chands10
Copy link
Contributor

chands10 commented Nov 20, 2025

Can you also try rebasing with latest main and rerunning tests? That will run tests with CDB2API_TEST flag enabled

roborivers
roborivers suggested changes Nov 20, 2025
View reviewed changes
Copy link

@roborivers roborivers left a comment

Choose a reason for hiding this comment

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

Coding style check: Error. ⚠.
Smoke testing: Success ✓.
Cbuild submission: Error ⚠.
Regression testing: 0/0 tests failed ⚠.

roborivers
roborivers suggested changes Nov 21, 2025
View reviewed changes
Copy link

@roborivers roborivers left a comment

Choose a reason for hiding this comment

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

Coding style check: Error. ⚠.
Smoke testing: Success ✓.
Cbuild submission: Success ✓.
Regression testing: 0/0 tests failed ⚠.

@mohitkhullar mohitkhullar requested a review from chands10 December 1, 2025 15:32
@chands10
Copy link
Contributor

chands10 commented Dec 1, 2025

Can you fix the build?

/home/runner/work/comdb2/comdb2/cdb2api/cdb2api.c:3734:43: error: format specifies type 'int' but the argument has type 'long long' [-Werror,-Wformat]
 3734 |         printf("RETRY with timeout %d\n", get_call_timeout(hndl, connect_timeout));
      |                                    ~~     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                    %lld
/home/runner/work/comdb2/comdb2/cdb2api/cdb2api.c:3734:43: error: format specifies type 'int' but the argument has type 'long long' [-Werror,-Wformat]
 3734 |         printf("RETRY with timeout %d\n", get_call_timeout(hndl, connect_timeout));
      |                                    ~~     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                    %lld
/home/runner/work/comdb2/comdb2/cdb2api/cdb2api.c:3773:43: error: format specifies type 'int' but the argument has type 'long long' [-Werror,-Wformat]
 3773 |         printf("RETRY with timeout %d\n", get_call_timeout(hndl, connect_timeout));
      |                                    ~~     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                    %lld
/home/runner/work/comdb2/comdb2/cdb2api/cdb2api.c:3773:43: error: format specifies type 'int' but the argument has type 'long long' [-Werror,-Wformat]
 3773 |         printf("RETRY with timeout %d\n", get_call_timeout(hndl, connect_timeout));
      |                                    ~~     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                    %lld
[  2%] Linking C static library libmem.a

@chands10
Copy link
Contributor

chands10 commented Dec 1, 2025

And format

@mohitkhullar mohitkhullar force-pushed the enforce_max_call_time branch 2 times, most recently from c1b42df to 8d0c270 Compare December 1, 2025 16:43
@mohitkhullar
enforce max call time
f175820 
Signed-off-by: mohitkhullar <[email protected]>
@mohitkhullar mohitkhullar force-pushed the enforce_max_call_time branch from 8d0c270 to f175820 Compare December 1, 2025 16:43
roborivers
roborivers approved these changes Dec 1, 2025
View reviewed changes
Copy link

@roborivers roborivers left a comment

Choose a reason for hiding this comment

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

Coding style check: Success ✓.
Smoke testing: Success ✓.
Cbuild submission: Success ✓.
Regression testing: Success ✓.

The first 10 failing tests are:
analyze_partial_index_off_generated
consumer_non_atomic_default_consumer_generated
insert_lots_ssl_generated
insert_lots
reco-ddlk-sql
incoh_remsql_rte_connect_generated
incoh_remsql

@chands10
Copy link
Contributor

chands10 commented Dec 1, 2025

Some robomark tests might be failing? Unsure if related to your changes

Candidate enforce_max_call_time.R20251201.1 test  comdbg               linux from sundev1 host -> failed.
Candidate enforce_max_call_time.R20251201.1 test  comdbg               sundev1 from sundev1 host -> failed.
Candidate enforce_max_call_time.R20251201.1 test  comdbgtables         linux from sundev1 host -> failed.
Candidate enforce_max_call_time.R20251201.1 test  comdbgtables         sundev1 from sundev1 host -> failed.

@chands10
Copy link
Contributor

chands10 commented Dec 2, 2025

Doesn't look related

@mohitkhullar
Copy link
Contributor Author

mohitkhullar commented Dec 2, 2025 •
edited
Loading

Thanks
these tests are failing in 8.2.50 too

@mohitkhullar mohitkhullar merged commit 0fec7bb into bloomberg:main Dec 2, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chands10 chands10 chands10 approved these changes

+1 more reviewer

@roborivers roborivers roborivers approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mohitkhullar @chands10 @roborivers