Skip to content

Make blocking-physrep default #5590

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 2 commits into from
Dec 4, 2025
Merged

Make blocking-physrep default #5590

merged 2 commits into from
Dec 4, 2025
+63 −19

Conversation

@markhannum
Copy link
Contributor

@markhannum markhannum commented Nov 21, 2025

Blocking physrep shows significant performance boost. This PR makes blocking physrep the default.

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: Error ⚠.
Regression testing: 0/0 tests failed ⚠.

@markhannum markhannum force-pushed the default_blocking_physrep branch 2 times, most recently from 4e2590a to 2959059 Compare November 21, 2025 21:40
roborivers
roborivers suggested changes Nov 22, 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: Success ✓.

The first 10 failing tests are:
truncatesc_offline_generated
truncatesc
consumer_non_atomic_default_consumer_generated
insert_lots
reco-ddlk-sql

@markhannum markhannum force-pushed the default_blocking_physrep branch from 2959059 to e34b3df Compare November 22, 2025 15:53
roborivers
roborivers suggested changes Nov 25, 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: Success ✓.

The first 10 failing tests are:
truncatesc
consumer_non_atomic_default_consumer_generated
insert_lots_ssl_generated
insert_lots
reco-ddlk-sql

@markhannum markhannum force-pushed the default_blocking_physrep branch from e34b3df to 33e9ab3 Compare November 25, 2025 19:09
roborivers
roborivers suggested changes Nov 26, 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: Success ✓.

The first 10 failing tests are:
nogensc [setup failure]
analyze_partial_index_off_generated
consumer_non_atomic_default_consumer_generated
truncatesc_offline_generated
insert_lots_ssl_generated
insert_lots
reco-ddlk-sql
lock_views

roborivers
roborivers suggested changes Nov 26, 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: Success ✓.

The first 10 failing tests are:
truncatesc
yast_stat4scan_generated
ixmap
analyze_recover_deadlock
autoanalyze
analyze_partial_index_off_generated
analyze
temp_table_truncate_leak
consumer_non_atomic_default_consumer_generated
auth_twofiles_generated

roborivers
roborivers suggested 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: Error. ⚠.
Smoke testing: Success ✓.
Cbuild submission: Success ✓.
Regression testing: Success ✓.

The first 10 failing tests are:
truncatesc
reco-ddlk-sql
analyze_partial_index_off_generated
consumer_non_atomic_default_consumer_generated
sc_transactional_rowlocks_generated
sc_downgrade
insert_lots_ssl_generated
insert_lots

@markhannum markhannum force-pushed the default_blocking_physrep branch from 33e9ab3 to 31c4391 Compare December 2, 2025 20:51
roborivers
roborivers suggested changes Dec 3, 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: Success ✓.

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

SChakravorti21
SChakravorti21 reviewed Dec 3, 2025
View reviewed changes
tests/tunables.test/t00_all_tunables.expected Outdated
(name='blobstripe', description='', type='BOOLEAN', value='ON', read_only='Y')
(name='blocking_latches', description='Block on latch rather than deadlock', type='BOOLEAN', value='OFF', read_only='N')
(name='blocking_physrep', description='Physical replicant blocks on select. (Default: off)', type='BOOLEAN', value='OFF', read_only='N')
(name='blocking_physrep', description='Physical replicant blocks on select. (Default: off)', type='BOOLEAN', value='ON', read_only='N')
Copy link
Contributor

@SChakravorti21 SChakravorti21 Dec 3, 2025

Choose a reason for hiding this comment

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

Default value in description should be updated here and below, otherwise LGTM!

@markhannum markhannum force-pushed the default_blocking_physrep branch from 31c4391 to 6fab327 Compare December 4, 2025 12:33
@markhannum markhannum force-pushed the default_blocking_physrep branch from 6fab327 to 54c5951 Compare December 4, 2025 12:34
@markhannum markhannum merged commit f82a2c1 into bloomberg:main Dec 4, 2025
3 of 4 checks passed
@markhannum markhannum deleted the default_blocking_physrep branch December 4, 2025 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SChakravorti21 SChakravorti21 SChakravorti21 approved these changes

+1 more reviewer

@roborivers roborivers roborivers requested 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

@markhannum @SChakravorti21 @roborivers