Skip to content

asynchronous distributed commit - #1963

Open
aakash10292 wants to merge 18 commits into
bloomberg:mainfrom
aakash10292:aar_async_dist_commit
Open

aakash10292 wants to merge 18 commits into
bloomberg:mainfrom
aakash10292:aar_async_dist_commit

Conversation

@aakash10292

@aakash10292 aakash10292 commented Dec 4, 2019 •

Copy link
Copy Markdown
Contributor

This PR introduces a new feature that enables asynchronous wait for distributed commit.
Currently, Block processor threads on master wait sequentially for replicants to catch upto a particular LSN. Now, instead of the Block processor threads needlessly spending time on busy waits, the "waiting for replicants" work is offloaded to a separate thread that waits on distributed commit and signals clients with the respective results.

@riverszhang89

Copy link
Copy Markdown
Contributor

@roborivers /runtests please.

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

Code review suggestions:

@@ db/db_tunables.h @@
+ Please update the tunable documentation (docs/pages/config/config_files.md) accordingly.
@@ db/seqnum_wait.c @@
+ Please remove debugging statements, and replace fprintf/printf with logmsg.
@@ db/seqnum_wait.c @@
+ Please replace any pthread_xxx() function in your diff with corresponding Pthread_xxx() wrapper.

@dorinhogea

Copy link
Copy Markdown
Contributor

Please add a description to what this does

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

Coding style check: Error. Use clang-format or view diff ⚠.
Smoke testing: Success ✓.
Cbuild submission: Success ✓.
Regression testing: 3/325 tests failed ⚠.

The first 10 failing tests are:
tunables
selectv_rcode_serialretry_generated
selectv_rcode_serialize_reads_like_writes_generated

Detail | Log | Resubmit

@aakash10292

Copy link
Copy Markdown
Contributor Author

This PR introduces a new feature that enables asynchronous wait for distributed commit.
Currently, Block processor threads on master wait sequentially for replicants to catch upto a particular LSN. Now, instead of the Block processor threads needlessly spending time on busy waits, the "waiting for replicants" work is offloaded to a separate thread that waits on distributed commit and signals clients with the respective results.

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

Sysbench Transactions Per Second (commit: 0725cb39):

@@ sysbench/bulk_insert @@
-6.98%

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

Code review suggestions:

@@ db/db_tunables.h @@
+ Please update the tunable documentation (docs/pages/config/config_files.md) accordingly.
@@ db/seqnum_wait.c @@
+ Please remove debugging statements, and replace fprintf/printf with logmsg.
@@ db/seqnum_wait.c @@
+ Please replace any pthread_xxx() function in your diff with corresponding Pthread_xxx() wrapper.

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

Coding style check: Error. Use clang-format or view diff ⚠.
Smoke testing: Error ⚠ .
Cbuild submission: Success ✓.
Regression testing: 4/325 tests failed ⚠.

The first 10 failing tests are:
restart_socksql
replay_eventlog
tunables
incrementalcopy

Detail | Log | Resubmit

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

Sysbench Transactions Per Second (commit: 47196279):

@@ sysbench/select_random_ranges @@
+4.82%
@@ sysbench/oltp_read_only @@
-4.72%

Comment thread bbinc/gettimeofday_ms.h Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why do you split the static inline version into a .h and a .c?

Comment thread bdb/rep.c Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This function should still be static.

Comment thread db/seqnum_wait.c Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These should be moved to bdb_api.h.

@aakash10292
aakash10292 force-pushed the aar_async_dist_commit branch from 4719627 to 4c91833 Compare December 9, 2019 14:24

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

Code review suggestions:

@@ db/db_tunables.h @@
+ Please update the tunable documentation (docs/pages/config/config_files.md) accordingly.
@@ db/seqnum_wait.c @@
+ Please remove debugging statements, and replace fprintf/printf with logmsg.
@@ db/seqnum_wait.c @@
+ Please replace any pthread_xxx() function in your diff with corresponding Pthread_xxx() wrapper.

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

Coding style check: Error. Use clang-format or view diff ⚠.
Smoke testing: Success ✓.
Cbuild submission: Error ⚠ .
Regression testing: 0/0 tests failed ⚠.

Detail | Log | Resubmit

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

Code review suggestions:

@@ db/db_tunables.h @@
+ Please update the tunable documentation (docs/pages/config/config_files.md) accordingly.
@@ db/seqnum_wait.c @@
+ Please remove debugging statements, and replace fprintf/printf with logmsg.
@@ db/seqnum_wait.c @@
+ Please replace any pthread_xxx() function in your diff with corresponding Pthread_xxx() wrapper.

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

Coding style check: Error. Use clang-format or view diff ⚠.
Smoke testing: Success ✓.
Cbuild submission: Error ⚠ .
Regression testing: 0/0 tests failed ⚠.

Detail | Log | Resubmit

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

Sysbench Transactions Per Second (commit: 4c91833d):

@@ sysbench/oltp_read_only @@
-6.42%

@aakash10292
aakash10292 force-pushed the aar_async_dist_commit branch from 4c91833 to 5c90d40 Compare December 11, 2019 21:00
Comment thread db/glue.c Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Duplicate code?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, rectified it

@aakash10292
aakash10292 force-pushed the aar_async_dist_commit branch 2 times, most recently from 405beae to c709139 Compare December 19, 2019 15:59

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

Code review suggestions:

@@ db/db_tunables.h @@
+ Please update the tunable documentation (docs/pages/config/config_files.md) accordingly.
@@ db/seqnum_wait.c @@
+ Please remove debugging statements, and replace fprintf/printf with logmsg.
@@ db/seqnum_wait.c @@
+ Please replace any pthread_xxx() function in your diff with corresponding Pthread_xxx() wrapper.

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

Coding style check: Error. Use clang-format or view diff ⚠.
Smoke testing: Success ✓.
Cbuild submission: Success ✓.
Regression testing: 4/325 tests failed ⚠.

The first 10 failing tests are:
dohsql_leaks
selectv_rcode_force_commit_generated
tunables
incrementalcopy

Detail | Log | Resubmit

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

Code review suggestions:

@@ db/db_tunables.h @@
+ Please update the tunable documentation (docs/pages/config/config_files.md) accordingly.
@@ db/seqnum_wait.c @@
+ Please remove debugging statements, and replace fprintf/printf with logmsg.
@@ db/seqnum_wait.c @@
+ Please replace any pthread_xxx() function in your diff with corresponding Pthread_xxx() wrapper.

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

Coding style check: Error. Use clang-format or view diff ⚠.
Smoke testing: Success ✓.
Cbuild submission: Success ✓.
Regression testing: 3/325 tests failed ⚠.

The first 10 failing tests are:
selectv_rcode
tunables
incrementalcopy

Detail | Log | Resubmit

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

Sysbench Transactions Per Second (commit: 815b6f73):

@@ sysbench/select_random_ranges @@
+11.52%
@@ sysbench/oltp_read_only @@
+9.35%

@aakash10292
aakash10292 force-pushed the aar_async_dist_commit branch from 815b6f7 to 3ecc8ce Compare December 23, 2019 15:48
@aakash10292
aakash10292 force-pushed the aar_async_dist_commit branch from 3ecc8ce to 20daf11 Compare January 6, 2020 20:18
@aakash10292
aakash10292 force-pushed the aar_async_dist_commit branch 3 times, most recently from 57479a5 to 0b7492c Compare March 11, 2020 15:36
@aakash10292
aakash10292 force-pushed the aar_async_dist_commit branch 2 times, most recently from 36c68f9 to eea2abd Compare March 30, 2020 15:18
@aakash10292
aakash10292 force-pushed the aar_async_dist_commit branch 2 times, most recently from d1c238a to 15d00e3 Compare April 5, 2020 18:39
…#1952)"

This reverts commit e41b402.

Signed-off-by: aakash10292 <aakash10292@gmail.com>
Signed-off-by: aakash10292 <aakash10292@gmail.com>
…#1952)"

This reverts commit e41b402.

Signed-off-by: aakash10292 <aakash10292@gmail.com>
Signed-off-by: aakash10292 <aakash10292@gmail.com>
Signed-off-by: aakash10292 <aakash10292@gmail.com>
Signed-off-by: aakash10292 <aakash10292@gmail.com>
This reverts commit 0750ba34f35eacca52f6e596ffd70a30d71f4a4d.

Signed-off-by: aakash10292 <aakash10292@gmail.com>
Signed-off-by: aakash10292 <aakash10292@gmail.com>
Signed-off-by: aakash10292 <aakash10292@gmail.com>
Signed-off-by: aakash10292 <aakash10292@gmail.com>
Signed-off-by: aakash10292 <aakash10292@gmail.com>
… INTERNAL_FORWARD

Signed-off-by: aakash10292 <aakash10292@gmail.com>
Signed-off-by: aakash10292 <aakash10292@gmail.com>
This reverts commit ebf113b10476c8001bbaea885aaefe5ca5f18490.

Signed-off-by: aakash10292 <aakash10292@gmail.com>
Signed-off-by: aakash10292 <aakash10292@gmail.com>
Signed-off-by: aakash10292 <aakash10292@gmail.com>
Signed-off-by: aakash10292 <aakash10292@gmail.com>
Signed-off-by: aakash10292 <aakash10292@gmail.com>
@aakash10292
aakash10292 force-pushed the aar_async_dist_commit branch from 15d00e3 to 737df5b Compare April 8, 2020 13:57
emelialei88 added a commit to emelialei88/comdb2 that referenced this pull request Aug 17, 2026
Port of bloomberg#1963 onto current main. Block processors commit locally and then
block in trans_wait_for_seqnum_int until every replicant acks, so with the
default 8 writer threads only 8 commits can be in flight regardless of
client concurrency. This hands the wait to a dedicated seqnum-wait thread
and returns the block processor to the queue immediately; the ack is still
performed, so replicants stay bounded.

Off by default (async_dist_commit). The thread always starts but stays idle
until work is queued, so the tunable can be flipped at runtime.

Rather than carry bloomberg#1963's copy of bdb_wait_for_seqnum_from_all_int, which
predated durable-majority accounting, gbl_2pc, non_durable_retry and
set_durable_lsn, the shared logic is factored out of main into
bdb_wait_for_seqnum_finish and bdb_wait_for_seqnum_mark_incoherent.

Signed-off-by: Emelia Lei <wlei29@bloomberg.net>
emelialei88 added a commit to emelialei88/comdb2 that referenced this pull request Sep 17, 2026
Port of bloomberg#1963 onto current main. Block processors commit locally and then
block in trans_wait_for_seqnum_int until every replicant acks, so with the
default 8 writer threads only 8 commits can be in flight regardless of
client concurrency. This hands the wait to a dedicated seqnum-wait thread
and returns the block processor to the queue immediately; the ack is still
performed, so replicants stay bounded.

Off by default (async_dist_commit). The thread always starts but stays idle
until work is queued, so the tunable can be flipped at runtime.

Rather than carry bloomberg#1963's copy of bdb_wait_for_seqnum_from_all_int, which
predated durable-majority accounting, gbl_2pc, non_durable_retry and
set_durable_lsn, the shared logic is factored out of main into
bdb_wait_for_seqnum_finish and bdb_wait_for_seqnum_mark_incoherent.

Signed-off-by: Emelia Lei <wlei29@bloomberg.net>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants