[kudu-CR] KUDU-798 (part 2) Remove automatic safe time adjustment from mvcc
David Ribeiro Alves has submitted this change and it was merged. Change subject: KUDU-798 (part 2) Remove automatic safe time adjustment from mvcc .. KUDU-798 (part 2) Remove automatic safe time adjustment from mvcc This patch completely removes the APIs that allow automatic timestamp assignment and automatic safe time adjustment from Mvcc. The previous patch took care of making sure these APIs were unused in regular TabletServer operations. This patch completes that by removing the APIs from Mvcc and changing tests appropriately. Because, for non-tserver based tests, timestamp assignment and transaction start are now done under a lock this does introduce a slowdown in tests like mt-tablet-test. I measured about 10% slowdown on an mvcc-stressing config of mt-tablet-test (run config and perf stats in the end of this commit message). However, this patch series does not add any overhead on the actual tablet server write path. In fact, this even simplifies it in some circumstances: for instance, we were advancing safe time twice for leader side txns: one when it was consensus committed and one when the apply was complete. Due to this, there was even a moderate speedup in this more important case, measured with full_stack-insert-scan-test, (run config and perf stats below) of about 9%. === full_stack-insert-scan-test run config: bin/full_stack-insert-scan-test \ --gtest_filter=*MRSOnlyStressTest* \ --inserts_per_client=20 \ --concurrent_inserts=10 \ --rows_per_batch=1 \ --skip_scans Results on master before this patch series: 344086.766163 task-clock#5.659 CPUs utilized 14,435,389 context-switches #0.042 M/sec 91,225 cpu-migrations#0.265 K/sec 112,036 page-faults #0.326 K/sec 956,637,266,040 cycles#2.780 GHz stalled-cycles-frontend stalled-cycles-backend 577,637,169,921 instructions #0.60 insns per cycle 109,720,205,884 branches # 318.874 M/sec 730,807,372 branch-misses #0.67% of all branches 60.807013290 seconds time elapsed Results on master after this patch series: 328574.272742 task-clock#5.932 CPUs utilized 13,820,330 context-switches #0.042 M/sec 170,370 cpu-migrations#0.519 K/sec 111,997 page-faults #0.341 K/sec 911,264,247,114 cycles#2.773 GHz stalled-cycles-frontend stalled-cycles-backend 572,898,176,369 instructions #0.63 insns per cycle 108,872,113,035 branches # 331.347 M/sec 728,934,228 branch-misses #0.67% of all branches 55.387672052 seconds time elapsed === mt-tablet-test run config: bin/mt-tablet-test \ --gtest_filter=*0*DoTestAllAtOnce* \ --num_counter_threads=0 \ --num_slowreader_threads=0 \ --flusher_backoff=1.0 \ --flusher_initial_frequency_ms=1 \ --inserts_per_thread=100 \ --num_summer_threads=0 \ --gtest_repeat=3 \ --tablet_test_flush_threshold_mb=2000 \ --minloglevel=10 Results on master before this patch series: 618894.806683 task-clock#7.716 CPUs utilized 7,243,713 context-switches #0.012 M/sec 782 cpu-migrations#0.001 K/sec 1,457,677 page-faults #0.002 M/sec 1,794,712,092,284 cycles#2.900 GHz stalled-cycles-frontend stalled-cycles-backend 648,757,335,519 instructions #0.36 insns per cycle 128,766,989,780 branches # 208.060 M/sec 790,345,583 branch-misses #0.61% of all branches 80.204388663 seconds time elapsed Result on master after this patch series: 620594.911121 task-clock#7.012 CPUs utilized 17,645,922 context-switches #0.028 M/sec 1,163 cpu-migrations#0.002 K/sec 1,252,812 page-faults #0.002 M/sec 1,775,937,664,119 cycles#2.862 GHz stalled-cycles-frontend stalled-cycles-backend 757,882,564,990 instructions #0.43 insns per cycle 155,500,757,477 branches # 250.567 M/sec 982,110,478 branch-misses #0.63% of all branches 88.500973260 seconds time elapsed Change-Id: I6cd0b8aa52e4b0f5c0f4872d806e76387f6b9538 Reviewed-on: http://gerrit.cloudera.org:8080/5057 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon --- M src/kudu/tablet/compaction-test.cc M src/kudu/tablet/deltamemstore-test.cc M src/kudu/tablet/diskrowset-test-base.h M src/kudu/table
[kudu-CR] KUDU-798 (part 2) Remove automatic safe time adjustment from mvcc
Todd Lipcon has posted comments on this change. Change subject: KUDU-798 (part 2) Remove automatic safe time adjustment from mvcc .. Patch Set 13: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/5057 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6cd0b8aa52e4b0f5c0f4872d806e76387f6b9538 Gerrit-PatchSet: 13 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-798 (part 2) Remove automatic safe time adjustment from mvcc
Hello Mike Percy, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5057 to look at the new patch set (#13). Change subject: KUDU-798 (part 2) Remove automatic safe time adjustment from mvcc .. KUDU-798 (part 2) Remove automatic safe time adjustment from mvcc This patch completely removes the APIs that allow automatic timestamp assignment and automatic safe time adjustment from Mvcc. The previous patch took care of making sure these APIs were unused in regular TabletServer operations. This patch completes that by removing the APIs from Mvcc and changing tests appropriately. Because, for non-tserver based tests, timestamp assignment and transaction start are now done under a lock this does introduce a slowdown in tests like mt-tablet-test. I measured about 10% slowdown on an mvcc-stressing config of mt-tablet-test (run config and perf stats in the end of this commit message). However, this patch series does not add any overhead on the actual tablet server write path. In fact, this even simplifies it in some circumstances: for instance, we were advancing safe time twice for leader side txns: one when it was consensus committed and one when the apply was complete. Due to this, there was even a moderate speedup in this more important case, measured with full_stack-insert-scan-test, (run config and perf stats below) of about 9%. === full_stack-insert-scan-test run config: bin/full_stack-insert-scan-test \ --gtest_filter=*MRSOnlyStressTest* \ --inserts_per_client=20 \ --concurrent_inserts=10 \ --rows_per_batch=1 \ --skip_scans Results on master before this patch series: 344086.766163 task-clock#5.659 CPUs utilized 14,435,389 context-switches #0.042 M/sec 91,225 cpu-migrations#0.265 K/sec 112,036 page-faults #0.326 K/sec 956,637,266,040 cycles#2.780 GHz stalled-cycles-frontend stalled-cycles-backend 577,637,169,921 instructions #0.60 insns per cycle 109,720,205,884 branches # 318.874 M/sec 730,807,372 branch-misses #0.67% of all branches 60.807013290 seconds time elapsed Results on master after this patch series: 328574.272742 task-clock#5.932 CPUs utilized 13,820,330 context-switches #0.042 M/sec 170,370 cpu-migrations#0.519 K/sec 111,997 page-faults #0.341 K/sec 911,264,247,114 cycles#2.773 GHz stalled-cycles-frontend stalled-cycles-backend 572,898,176,369 instructions #0.63 insns per cycle 108,872,113,035 branches # 331.347 M/sec 728,934,228 branch-misses #0.67% of all branches 55.387672052 seconds time elapsed === mt-tablet-test run config: bin/mt-tablet-test \ --gtest_filter=*0*DoTestAllAtOnce* \ --num_counter_threads=0 \ --num_slowreader_threads=0 \ --flusher_backoff=1.0 \ --flusher_initial_frequency_ms=1 \ --inserts_per_thread=100 \ --num_summer_threads=0 \ --gtest_repeat=3 \ --tablet_test_flush_threshold_mb=2000 \ --minloglevel=10 Results on master before this patch series: 618894.806683 task-clock#7.716 CPUs utilized 7,243,713 context-switches #0.012 M/sec 782 cpu-migrations#0.001 K/sec 1,457,677 page-faults #0.002 M/sec 1,794,712,092,284 cycles#2.900 GHz stalled-cycles-frontend stalled-cycles-backend 648,757,335,519 instructions #0.36 insns per cycle 128,766,989,780 branches # 208.060 M/sec 790,345,583 branch-misses #0.61% of all branches 80.204388663 seconds time elapsed Result on master after this patch series: 620594.911121 task-clock#7.012 CPUs utilized 17,645,922 context-switches #0.028 M/sec 1,163 cpu-migrations#0.002 K/sec 1,252,812 page-faults #0.002 M/sec 1,775,937,664,119 cycles#2.862 GHz stalled-cycles-frontend stalled-cycles-backend 757,882,564,990 instructions #0.43 insns per cycle 155,500,757,477 branches # 250.567 M/sec 982,110,478 branch-misses #0.63% of all branches 88.500973260 seconds time elapsed Change-Id: I6cd0b8aa52e4b0f5c0f4872d806e76387f6b9538 --- M src/kudu/tablet/compaction-test.cc M src/kudu/tablet/deltamemstore-test.cc M src/kudu/tablet/diskrowset-test-base.h M src/kudu/tablet/di
[kudu-CR] KUDU-798 (part 2) Remove automatic safe time adjustment from mvcc
Hello Mike Percy, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5057 to look at the new patch set (#12). Change subject: KUDU-798 (part 2) Remove automatic safe time adjustment from mvcc .. KUDU-798 (part 2) Remove automatic safe time adjustment from mvcc This patch completely removes the APIs that allow automatic timestamp assignment and automatic safe time adjustment from Mvcc. The previous patch took care of making sure these APIs were unused in regular TabletServer operations. This patch completes that by removing the APIs from Mvcc and changing tests appropriately. Because, for non-tserver based tests, timestamp assignment and transaction start are now done under a lock this does introduce a slowdown in tests like mt-tablet-test. I measured about 10% slowdown on an mvcc-stressing config of mt-tablet-test (run config and perf stats in the end of this commit message). However, this patch series does not add any overhead on the actual tablet server write path. In fact, this even simplifies it in some circumstances: for instance, we were advancing safe time twice for leader side txns: one when it was consensus committed and one when the apply was complete. Due to this, there was even a moderate speedup in this more important case, measured with full_stack-insert-scan-test, (run config and perf stats below) of about 9%. === full_stack-insert-scan-test run config: bin/full_stack-insert-scan-test \ --gtest_filter=*MRSOnlyStressTest* \ --inserts_per_client=20 \ --concurrent_inserts=10 \ --rows_per_batch=1 \ --skip_scans Results on master before this patch series: 344086.766163 task-clock#5.659 CPUs utilized 14,435,389 context-switches #0.042 M/sec 91,225 cpu-migrations#0.265 K/sec 112,036 page-faults #0.326 K/sec 956,637,266,040 cycles#2.780 GHz stalled-cycles-frontend stalled-cycles-backend 577,637,169,921 instructions #0.60 insns per cycle 109,720,205,884 branches # 318.874 M/sec 730,807,372 branch-misses #0.67% of all branches 60.807013290 seconds time elapsed Results on master after this patch series: 328574.272742 task-clock#5.932 CPUs utilized 13,820,330 context-switches #0.042 M/sec 170,370 cpu-migrations#0.519 K/sec 111,997 page-faults #0.341 K/sec 911,264,247,114 cycles#2.773 GHz stalled-cycles-frontend stalled-cycles-backend 572,898,176,369 instructions #0.63 insns per cycle 108,872,113,035 branches # 331.347 M/sec 728,934,228 branch-misses #0.67% of all branches 55.387672052 seconds time elapsed === mt-tablet-test run config: bin/mt-tablet-test \ --gtest_filter=*0*DoTestAllAtOnce* \ --num_counter_threads=0 \ --num_slowreader_threads=0 \ --flusher_backoff=1.0 \ --flusher_initial_frequency_ms=1 \ --inserts_per_thread=100 \ --num_summer_threads=0 \ --gtest_repeat=3 \ --tablet_test_flush_threshold_mb=2000 \ --minloglevel=10 Results on master before this patch series: 618894.806683 task-clock#7.716 CPUs utilized 7,243,713 context-switches #0.012 M/sec 782 cpu-migrations#0.001 K/sec 1,457,677 page-faults #0.002 M/sec 1,794,712,092,284 cycles#2.900 GHz stalled-cycles-frontend stalled-cycles-backend 648,757,335,519 instructions #0.36 insns per cycle 128,766,989,780 branches # 208.060 M/sec 790,345,583 branch-misses #0.61% of all branches 80.204388663 seconds time elapsed Result on master after this patch series: 620594.911121 task-clock#7.012 CPUs utilized 17,645,922 context-switches #0.028 M/sec 1,163 cpu-migrations#0.002 K/sec 1,252,812 page-faults #0.002 M/sec 1,775,937,664,119 cycles#2.862 GHz stalled-cycles-frontend stalled-cycles-backend 757,882,564,990 instructions #0.43 insns per cycle 155,500,757,477 branches # 250.567 M/sec 982,110,478 branch-misses #0.63% of all branches 88.500973260 seconds time elapsed Change-Id: I6cd0b8aa52e4b0f5c0f4872d806e76387f6b9538 --- M src/kudu/tablet/compaction-test.cc M src/kudu/tablet/deltamemstore-test.cc M src/kudu/tablet/diskrowset-test-base.h M src/kudu/tablet/di
[kudu-CR] KUDU-798 (part 2) Remove automatic safe time adjustment from mvcc
David Ribeiro Alves has posted comments on this change.
Change subject: KUDU-798 (part 2) Remove automatic safe time adjustment from
mvcc
..
Patch Set 10:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/5057/10/src/kudu/tablet/mvcc-test.cc
File src/kudu/tablet/mvcc-test.cc:
PS10, Line 228: / and committing this transaction this should not advance the
MvccManager
: // 'all_committed_before_' watermark.
> not quite a sentence
Done
http://gerrit.cloudera.org:8080/#/c/5057/10/src/kudu/tablet/mvcc.cc
File src/kudu/tablet/mvcc.cc:
PS10, Line 48: // This shouldn't happen in general, so in debug more just
crash.
> hrm, maybe this shoudl just be a CHECK then? if this happens can't we get a
when this returns IllegalState it causes a crash on ScopedTransaction, but I
think you're right that is makes more sense to crash here. Done
Line 58: strings::Substitute("There is already a transaction with
timestamp: $0 in flight or "
> same, do we ever expect this to happen?
same
http://gerrit.cloudera.org:8080/#/c/5057/10/src/kudu/tablet/mvcc.h
File src/kudu/tablet/mvcc.h:
PS10, Line 186: 'all_committed_before_
> should this be referring to safe_time_ now? seems this all_committed_before
removed this since this method no longer returns a status.
PS10, Line 220: with timestamp
> 'with timestamps equal to'
Done
PS10, Line 220: anymore
> nit: 'any more'
Done
PS10, Line 360: ID
> timestamp
Done
--
To view, visit http://gerrit.cloudera.org:8080/5057
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6cd0b8aa52e4b0f5c0f4872d806e76387f6b9538
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: David Ribeiro Alves
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] KUDU-798 (part 2) Remove automatic safe time adjustment from mvcc
Hello Mike Percy, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5057 to look at the new patch set (#11). Change subject: KUDU-798 (part 2) Remove automatic safe time adjustment from mvcc .. KUDU-798 (part 2) Remove automatic safe time adjustment from mvcc This patch completely removes the APIs that allow automatic timestamp assignment and automatic safe time adjustment from Mvcc. The previous patch took care of making sure these APIs were unused in regular TabletServer operations. This patch completes that by removing the APIs from Mvcc and changing tests appropriately. Because, for non-tserver based tests, timestamp assignment and transaction start are now done under a lock this does introduce a slowdown in tests like mt-tablet-test. I measured about 10% slowdown on an mvcc-stressing config of mt-tablet-test (run config and perf stats in the end of this commit message). However, this patch series does not add any overhead on the actual tablet server write path. In fact, this even simplifies it in some circumstances: for instance, we were advancing safe time twice for leader side txns: one when it was consensus committed and one when the apply was complete. Due to this, there was even a moderate speedup in this more important case, measured with full_stack-insert-scan-test, (run config and perf stats below) of about 9%. === full_stack-insert-scan-test run config: bin/full_stack-insert-scan-test \ --gtest_filter=*MRSOnlyStressTest* \ --inserts_per_client=20 \ --concurrent_inserts=10 \ --rows_per_batch=1 \ --skip_scans Results on master before this patch series: 344086.766163 task-clock#5.659 CPUs utilized 14,435,389 context-switches #0.042 M/sec 91,225 cpu-migrations#0.265 K/sec 112,036 page-faults #0.326 K/sec 956,637,266,040 cycles#2.780 GHz stalled-cycles-frontend stalled-cycles-backend 577,637,169,921 instructions #0.60 insns per cycle 109,720,205,884 branches # 318.874 M/sec 730,807,372 branch-misses #0.67% of all branches 60.807013290 seconds time elapsed Results on master after this patch series: 328574.272742 task-clock#5.932 CPUs utilized 13,820,330 context-switches #0.042 M/sec 170,370 cpu-migrations#0.519 K/sec 111,997 page-faults #0.341 K/sec 911,264,247,114 cycles#2.773 GHz stalled-cycles-frontend stalled-cycles-backend 572,898,176,369 instructions #0.63 insns per cycle 108,872,113,035 branches # 331.347 M/sec 728,934,228 branch-misses #0.67% of all branches 55.387672052 seconds time elapsed === mt-tablet-test run config: bin/mt-tablet-test \ --gtest_filter=*0*DoTestAllAtOnce* \ --num_counter_threads=0 \ --num_slowreader_threads=0 \ --flusher_backoff=1.0 \ --flusher_initial_frequency_ms=1 \ --inserts_per_thread=100 \ --num_summer_threads=0 \ --gtest_repeat=3 \ --tablet_test_flush_threshold_mb=2000 \ --minloglevel=10 Results on master before this patch series: 618894.806683 task-clock#7.716 CPUs utilized 7,243,713 context-switches #0.012 M/sec 782 cpu-migrations#0.001 K/sec 1,457,677 page-faults #0.002 M/sec 1,794,712,092,284 cycles#2.900 GHz stalled-cycles-frontend stalled-cycles-backend 648,757,335,519 instructions #0.36 insns per cycle 128,766,989,780 branches # 208.060 M/sec 790,345,583 branch-misses #0.61% of all branches 80.204388663 seconds time elapsed Result on master after this patch series: 620594.911121 task-clock#7.012 CPUs utilized 17,645,922 context-switches #0.028 M/sec 1,163 cpu-migrations#0.002 K/sec 1,252,812 page-faults #0.002 M/sec 1,775,937,664,119 cycles#2.862 GHz stalled-cycles-frontend stalled-cycles-backend 757,882,564,990 instructions #0.43 insns per cycle 155,500,757,477 branches # 250.567 M/sec 982,110,478 branch-misses #0.63% of all branches 88.500973260 seconds time elapsed Change-Id: I6cd0b8aa52e4b0f5c0f4872d806e76387f6b9538 --- M src/kudu/tablet/compaction-test.cc M src/kudu/tablet/deltamemstore-test.cc M src/kudu/tablet/diskrowset-test-base.h M src/kudu/tablet/di
[kudu-CR] KUDU-798 (part 2) Remove automatic safe time adjustment from mvcc
Todd Lipcon has posted comments on this change.
Change subject: KUDU-798 (part 2) Remove automatic safe time adjustment from
mvcc
..
Patch Set 10:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/5057/10/src/kudu/tablet/mvcc-test.cc
File src/kudu/tablet/mvcc-test.cc:
PS10, Line 228: / and committing this transaction this should not advance the
MvccManager
: // 'all_committed_before_' watermark.
not quite a sentence
http://gerrit.cloudera.org:8080/#/c/5057/10/src/kudu/tablet/mvcc.cc
File src/kudu/tablet/mvcc.cc:
PS10, Line 48: // This shouldn't happen in general, so in debug more just
crash.
hrm, maybe this shoudl just be a CHECK then? if this happens can't we get all
sorts of bizarre corruption issues that would be hard to track down, and
crashing would be safer for data?
Line 58: strings::Substitute("There is already a transaction with
timestamp: $0 in flight or "
same, do we ever expect this to happen?
http://gerrit.cloudera.org:8080/#/c/5057/10/src/kudu/tablet/mvcc.h
File src/kudu/tablet/mvcc.h:
PS10, Line 186: 'all_committed_before_
should this be referring to safe_time_ now? seems this all_committed_before_
isn't an actual field name
PS10, Line 220: with timestamp
'with timestamps equal to'
PS10, Line 220: anymore
nit: 'any more'
PS10, Line 360: ID
timestamp
--
To view, visit http://gerrit.cloudera.org:8080/5057
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6cd0b8aa52e4b0f5c0f4872d806e76387f6b9538
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: David Ribeiro Alves
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] KUDU-798 (part 2) Remove automatic safe time adjustment from mvcc
Hello Mike Percy, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5057 to look at the new patch set (#10). Change subject: KUDU-798 (part 2) Remove automatic safe time adjustment from mvcc .. KUDU-798 (part 2) Remove automatic safe time adjustment from mvcc This patch completely removes the APIs that allow automatic timestamp assignment and automatic safe time adjustment from Mvcc. The previous patch took care of making sure these APIs were unused in regular TabletServer operations. This patch completes that by removing the APIs from Mvcc and changing tests appropriately. Because, for non-tserver based tests, timestamp assignment and transaction start are now done under a lock this does introduce a slowdown in tests like mt-tablet-test. I measured about 10% slowdown on an mvcc-stressing config of mt-tablet-test (run config and perf stats in the end of this commit message). However, this patch series does not add any overhead on the actual tablet server write path. In fact, this even simplifies it in some circumstances: for instance, we were advancing safe time twice for leader side txns: one when it was consensus committed and one when the apply was complete. Due to this, there was even a moderate speedup in this more important case, measured with full_stack-insert-scan-test, (run config and perf stats below) of about 9%. === full_stack-insert-scan-test run config: bin/full_stack-insert-scan-test \ --gtest_filter=*MRSOnlyStressTest* \ --inserts_per_client=20 \ --concurrent_inserts=10 \ --rows_per_batch=1 \ --skip_scans Results on master before this patch series: 344086.766163 task-clock#5.659 CPUs utilized 14,435,389 context-switches #0.042 M/sec 91,225 cpu-migrations#0.265 K/sec 112,036 page-faults #0.326 K/sec 956,637,266,040 cycles#2.780 GHz stalled-cycles-frontend stalled-cycles-backend 577,637,169,921 instructions #0.60 insns per cycle 109,720,205,884 branches # 318.874 M/sec 730,807,372 branch-misses #0.67% of all branches 60.807013290 seconds time elapsed Results on master after this patch series: 328574.272742 task-clock#5.932 CPUs utilized 13,820,330 context-switches #0.042 M/sec 170,370 cpu-migrations#0.519 K/sec 111,997 page-faults #0.341 K/sec 911,264,247,114 cycles#2.773 GHz stalled-cycles-frontend stalled-cycles-backend 572,898,176,369 instructions #0.63 insns per cycle 108,872,113,035 branches # 331.347 M/sec 728,934,228 branch-misses #0.67% of all branches 55.387672052 seconds time elapsed === mt-tablet-test run config: bin/mt-tablet-test \ --gtest_filter=*0*DoTestAllAtOnce* \ --num_counter_threads=0 \ --num_slowreader_threads=0 \ --flusher_backoff=1.0 \ --flusher_initial_frequency_ms=1 \ --inserts_per_thread=100 \ --num_summer_threads=0 \ --gtest_repeat=3 \ --tablet_test_flush_threshold_mb=2000 \ --minloglevel=10 Results on master before this patch series: 618894.806683 task-clock#7.716 CPUs utilized 7,243,713 context-switches #0.012 M/sec 782 cpu-migrations#0.001 K/sec 1,457,677 page-faults #0.002 M/sec 1,794,712,092,284 cycles#2.900 GHz stalled-cycles-frontend stalled-cycles-backend 648,757,335,519 instructions #0.36 insns per cycle 128,766,989,780 branches # 208.060 M/sec 790,345,583 branch-misses #0.61% of all branches 80.204388663 seconds time elapsed Result on master after this patch series: 620594.911121 task-clock#7.012 CPUs utilized 17,645,922 context-switches #0.028 M/sec 1,163 cpu-migrations#0.002 K/sec 1,252,812 page-faults #0.002 M/sec 1,775,937,664,119 cycles#2.862 GHz stalled-cycles-frontend stalled-cycles-backend 757,882,564,990 instructions #0.43 insns per cycle 155,500,757,477 branches # 250.567 M/sec 982,110,478 branch-misses #0.63% of all branches 88.500973260 seconds time elapsed Change-Id: I6cd0b8aa52e4b0f5c0f4872d806e76387f6b9538 --- M src/kudu/tablet/compaction-test.cc M src/kudu/tablet/deltamemstore-test.cc M src/kudu/tablet/diskrowset-test-base.h M src/kudu/tablet/di
[kudu-CR] KUDU-798 (part 2) Remove automatic safe time adjustment from mvcc
Hello Mike Percy, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5057 to look at the new patch set (#9). Change subject: KUDU-798 (part 2) Remove automatic safe time adjustment from mvcc .. KUDU-798 (part 2) Remove automatic safe time adjustment from mvcc This patch completely removes the APIs that allow automatic timestamp assignment and automatic safe time adjustment from Mvcc. The previous patch took care of making sure these APIs were unused in regular TabletServer operations. This patch completes that by removing the APIs from Mvcc and changing tests appropriately. Because, for non-tserver based tests, timestamp assignment and transaction start are now done under a lock this does introduce a slowdown in tests like mt-tablet-test. I measured about 10% slowdown on an mvcc-stressing config of mt-tablet-test (run config and perf stats in the end of this commit message). However this patch series does not add any overhead on the actual tablet server write path. In fact, this even simplifies it in some circumstances: for instance, we were advancing safe time twice for leader side txns: one when it was consensus committed and one when the apply was complete. Due to this, there was even a moderate speedup in this more important case, measured with full_stack-insert-scan-test, (run config and perf stats below) of about 9%. === full_stack-insert-scan-test run config: bin/full_stack-insert-scan-test \ --gtest_filter=*MRSOnlyStressTest* \ --inserts_per_client=20 \ --concurrent_inserts=10 \ --rows_per_batch=1 \ --skip_scans Results on master before this patch series: 344086.766163 task-clock#5.659 CPUs utilized 14,435,389 context-switches #0.042 M/sec 91,225 cpu-migrations#0.265 K/sec 112,036 page-faults #0.326 K/sec 956,637,266,040 cycles#2.780 GHz stalled-cycles-frontend stalled-cycles-backend 577,637,169,921 instructions #0.60 insns per cycle 109,720,205,884 branches # 318.874 M/sec 730,807,372 branch-misses #0.67% of all branches 60.807013290 seconds time elapsed Results on master after this patch series: 328574.272742 task-clock#5.932 CPUs utilized 13,820,330 context-switches #0.042 M/sec 170,370 cpu-migrations#0.519 K/sec 111,997 page-faults #0.341 K/sec 911,264,247,114 cycles#2.773 GHz stalled-cycles-frontend stalled-cycles-backend 572,898,176,369 instructions #0.63 insns per cycle 108,872,113,035 branches # 331.347 M/sec 728,934,228 branch-misses #0.67% of all branches 55.387672052 seconds time elapsed === mt-tablet-test run config: bin/mt-tablet-test \ --gtest_filter=*0*DoTestAllAtOnce* \ --num_counter_threads=0 \ --num_slowreader_threads=0 \ --flusher_backoff=1.0 \ --flusher_initial_frequency_ms=1 \ --inserts_per_thread=100 \ --num_summer_threads=0 \ --gtest_repeat=3 \ --tablet_test_flush_threshold_mb=2000 \ --minloglevel=10 Results on master before this patch series: 618894.806683 task-clock#7.716 CPUs utilized 7,243,713 context-switches #0.012 M/sec 782 cpu-migrations#0.001 K/sec 1,457,677 page-faults #0.002 M/sec 1,794,712,092,284 cycles#2.900 GHz stalled-cycles-frontend stalled-cycles-backend 648,757,335,519 instructions #0.36 insns per cycle 128,766,989,780 branches # 208.060 M/sec 790,345,583 branch-misses #0.61% of all branches 80.204388663 seconds time elapsed Result on master after this patch series: 620594.911121 task-clock#7.012 CPUs utilized 17,645,922 context-switches #0.028 M/sec 1,163 cpu-migrations#0.002 K/sec 1,252,812 page-faults #0.002 M/sec 1,775,937,664,119 cycles#2.862 GHz stalled-cycles-frontend stalled-cycles-backend 757,882,564,990 instructions #0.43 insns per cycle 155,500,757,477 branches # 250.567 M/sec 982,110,478 branch-misses #0.63% of all branches 88.500973260 seconds time elapsed Change-Id: I6cd0b8aa52e4b0f5c0f4872d806e76387f6b9538 --- M src/kudu/tablet/compaction-test.cc M src/kudu/tablet/deltamemstore-test.cc M src/kudu/tablet/diskrowset-test-base.h M src/kudu/tablet/disk
[kudu-CR] KUDU-798 (part 2) Remove automatic safe time adjustment from mvcc
Hello Mike Percy, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5057 to look at the new patch set (#8). Change subject: KUDU-798 (part 2) Remove automatic safe time adjustment from mvcc .. KUDU-798 (part 2) Remove automatic safe time adjustment from mvcc This patch completely removes the APIs that allow automatic timestamp assignment and automatic safe time adjustment from Mvcc. The previous patch took care of making sure these APIs were unused in regular TabletServer operations. This patch completes that by removing the APIs from Mvcc and changing tests appropriately. Because for non-tserver based tests timestamp assignment and transaction start are now done under a lock, this does introduce a slowdown in tests like mt-tablet-test. I measured about 10% slowdown on an mvcc-stressing config of mt-tablet-test (perf stats in the end of this commit message). However this patch series does not add any overhead on the actual tablet server write path. In fact, this even simplifies it in some circumstances: for instance we were advancing safe time twice for leader side txns, one when it was consensus committed and one when the apply was complete. Due to this, there was even a moderate speedup in this more important case, measured with full_stack-insert-scan-test, of about 9%. === full_stack-insert-scan-test run config: bin/full_stack-insert-scan-test \ --gtest_filter=*MRSOnlyStressTest* \ --inserts_per_client=20 \ --concurrent_inserts=10 \ --rows_per_batch=1 \ --skip_scans Results on master before this patch series: 344086.766163 task-clock#5.659 CPUs utilized 14,435,389 context-switches #0.042 M/sec 91,225 cpu-migrations#0.265 K/sec 112,036 page-faults #0.326 K/sec 956,637,266,040 cycles#2.780 GHz stalled-cycles-frontend stalled-cycles-backend 577,637,169,921 instructions #0.60 insns per cycle 109,720,205,884 branches # 318.874 M/sec 730,807,372 branch-misses #0.67% of all branches 60.807013290 seconds time elapsed Results on master after this patch series: 328574.272742 task-clock#5.932 CPUs utilized 13,820,330 context-switches #0.042 M/sec 170,370 cpu-migrations#0.519 K/sec 111,997 page-faults #0.341 K/sec 911,264,247,114 cycles#2.773 GHz stalled-cycles-frontend stalled-cycles-backend 572,898,176,369 instructions #0.63 insns per cycle 108,872,113,035 branches # 331.347 M/sec 728,934,228 branch-misses #0.67% of all branches 55.387672052 seconds time elapsed === mt-tablet-test run config: bin/mt-tablet-test \ --gtest_filter=*0*DoTestAllAtOnce* \ --num_counter_threads=0 \ --num_slowreader_threads=0 \ --flusher_backoff=1.0 \ --flusher_initial_frequency_ms=1 \ --inserts_per_thread=100 \ --num_summer_threads=0 \ --gtest_repeat=3 \ --tablet_test_flush_threshold_mb=2000 \ --minloglevel=10 Results on master before this patch series: 618894.806683 task-clock#7.716 CPUs utilized 7,243,713 context-switches #0.012 M/sec 782 cpu-migrations#0.001 K/sec 1,457,677 page-faults #0.002 M/sec 1,794,712,092,284 cycles#2.900 GHz stalled-cycles-frontend stalled-cycles-backend 648,757,335,519 instructions #0.36 insns per cycle 128,766,989,780 branches # 208.060 M/sec 790,345,583 branch-misses #0.61% of all branches 80.204388663 seconds time elapsed Result on master after this patch series: 620594.911121 task-clock#7.012 CPUs utilized 17,645,922 context-switches #0.028 M/sec 1,163 cpu-migrations#0.002 K/sec 1,252,812 page-faults #0.002 M/sec 1,775,937,664,119 cycles#2.862 GHz stalled-cycles-frontend stalled-cycles-backend 757,882,564,990 instructions #0.43 insns per cycle 155,500,757,477 branches # 250.567 M/sec 982,110,478 branch-misses #0.63% of all branches 88.500973260 seconds time elapsed Change-Id: I6cd0b8aa52e4b0f5c0f4872d806e76387f6b9538 --- M src/kudu/tablet/compaction-test.cc M src/kudu/tablet/deltamemstore-test.cc M src/kudu/tablet/diskrowset-test-base.h M src/kudu/tablet/diskrowset-test.cc M src/kudu/tablet/local_tablet_write
[kudu-CR] KUDU-798 (part 2) Remove automatic safe time adjustment from mvcc
Hello Mike Percy, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5057 to look at the new patch set (#7). Change subject: KUDU-798 (part 2) Remove automatic safe time adjustment from mvcc .. KUDU-798 (part 2) Remove automatic safe time adjustment from mvcc This patch completely removes the APIs that allow automatic timestamp assignment and automatic safe time adjustment from Mvcc. The previous patch took care of making sure these APIs were unused in regular TabletServer operations. This patch completes that by removing the APIs from Mvcc and changing tests appropriately. Because for non-tserver based tests timestamp assignment and transaction start are now done under a lock, this does introduce a slowdown in tests like mt-tablet-test. I measured about 10% slowdown on an mvcc-stressing config of mt-tablet-test (perf stats in the end of this commit message). However this patch series does not add any overhead on the actual tablet server write path. In fact, this even simplifies it in some circumstances: for instance we were advancing safe time twice for leader side txns, one when it was consensus committed and one when the apply was complete. Due to this, there was even a moderate speedup in this more important case of about 9%. === full_stack-insert-scan-test run config: bin/full_stack-insert-scan-test \ --gtest_filter=*MRSOnlyStressTest* \ --inserts_per_client=20 \ --concurrent_inserts=10 \ --rows_per_batch=1 \ --skip_scans Results on master before this patch series: 344086.766163 task-clock#5.659 CPUs utilized 14,435,389 context-switches #0.042 M/sec 91,225 cpu-migrations#0.265 K/sec 112,036 page-faults #0.326 K/sec 956,637,266,040 cycles#2.780 GHz stalled-cycles-frontend stalled-cycles-backend 577,637,169,921 instructions #0.60 insns per cycle 109,720,205,884 branches # 318.874 M/sec 730,807,372 branch-misses #0.67% of all branches 60.807013290 seconds time elapsed Results on master after this patch series: 328574.272742 task-clock#5.932 CPUs utilized 13,820,330 context-switches #0.042 M/sec 170,370 cpu-migrations#0.519 K/sec 111,997 page-faults #0.341 K/sec 911,264,247,114 cycles#2.773 GHz stalled-cycles-frontend stalled-cycles-backend 572,898,176,369 instructions #0.63 insns per cycle 108,872,113,035 branches # 331.347 M/sec 728,934,228 branch-misses #0.67% of all branches 55.387672052 seconds time elapsed === mt-tablet-test run config: bin/mt-tablet-test \ --gtest_filter=*0*DoTestAllAtOnce* \ --num_counter_threads=0 \ --num_slowreader_threads=0 \ --flusher_backoff=1.0 \ --flusher_initial_frequency_ms=1 \ --inserts_per_thread=100 \ --num_summer_threads=0 \ --gtest_repeat=3 \ --tablet_test_flush_threshold_mb=2000 \ --minloglevel=10 Results on master before this patch series: 618894.806683 task-clock#7.716 CPUs utilized 7,243,713 context-switches #0.012 M/sec 782 cpu-migrations#0.001 K/sec 1,457,677 page-faults #0.002 M/sec 1,794,712,092,284 cycles#2.900 GHz stalled-cycles-frontend stalled-cycles-backend 648,757,335,519 instructions #0.36 insns per cycle 128,766,989,780 branches # 208.060 M/sec 790,345,583 branch-misses #0.61% of all branches 80.204388663 seconds time elapsed Result on master after this patch series: 620594.911121 task-clock#7.012 CPUs utilized 17,645,922 context-switches #0.028 M/sec 1,163 cpu-migrations#0.002 K/sec 1,252,812 page-faults #0.002 M/sec 1,775,937,664,119 cycles#2.862 GHz stalled-cycles-frontend stalled-cycles-backend 757,882,564,990 instructions #0.43 insns per cycle 155,500,757,477 branches # 250.567 M/sec 982,110,478 branch-misses #0.63% of all branches 88.500973260 seconds time elapsed Change-Id: I6cd0b8aa52e4b0f5c0f4872d806e76387f6b9538 --- M src/kudu/tablet/compaction-test.cc M src/kudu/tablet/deltamemstore-test.cc M src/kudu/tablet/diskrowset-test-base.h M src/kudu/tablet/diskrowset-test.cc M src/kudu/tablet/local_tablet_writer.h M src/kudu/tablet/memrowset-test.cc M sr
[kudu-CR] KUDU-798 (part 2) Remove automatic safe time adjustment from mvcc
David Ribeiro Alves has posted comments on this change. Change subject: KUDU-798 (part 2) Remove automatic safe time adjustment from mvcc .. Patch Set 5: (25 comments) http://gerrit.cloudera.org:8080/#/c/5057/5//COMMIT_MSG Commit Message: PS5, Line 9: This > nit: This --> This patch Done PS5, Line 9: that > nit: that --> the or that --> those or, may be, drop 'that'? Done PS5, Line 14: This patch takes this the rest of the way by > nit: may be 'This patch completes that by ...' Done PS5, Line 10: Previous patches : had taken care of making sure these were unused in regular : TabletServer operations or in tests that use LocalTabletWriter. : : This patch takes this the rest of the way by removing the APIs : from Mvcc completely and changing tests that use it directly : to obtain the timestamps from a clock. > nit: may be, separate this into its own paragraph? simplified, I don't think it needs it now http://gerrit.cloudera.org:8080/#/c/5057/5/src/kudu/tablet/compaction-test.cc File src/kudu/tablet/compaction-test.cc: PS5, Line 174: Timestamp t = clock_->Now(); : ScopedTransaction tx(&mvcc_, t > Here and elsewhere: consider removing the temporary variable Done http://gerrit.cloudera.org:8080/#/c/5057/5/src/kudu/tablet/deltamemstore-test.cc File src/kudu/tablet/deltamemstore-test.cc: PS5, Line 82: Timestamp t = clock_->Now(); : ScopedTransaction tx(&mvcc_, t); > nit: consider instead Done http://gerrit.cloudera.org:8080/#/c/5057/5/src/kudu/tablet/diskrowset-test.cc File src/kudu/tablet/diskrowset-test.cc: PS5, Line 335: Timestamp t = clock_->Now(); : ScopedTransaction tx(&mvcc_, t); > nit: consider Done http://gerrit.cloudera.org:8080/#/c/5057/5/src/kudu/tablet/memrowset-test.cc File src/kudu/tablet/memrowset-test.cc: PS5, Line 110: Timestamp t = clock_->Now(); : ScopedTransaction tx(&mvcc_, t); > Here and elsewhere, consider Done http://gerrit.cloudera.org:8080/#/c/5057/5/src/kudu/tablet/mvcc-test.cc File src/kudu/tablet/mvcc-test.cc: PS5, Line 101: TEST_F(MvccTest, TestMvccMultipleInFlight) > It would be nice to have a concise description for the idea of the test: wh this test is not new nor is it changed in any way funcionally. would prefer to avoid fixing these random backlog things in what are mostly mechanical changes. PS5, Line 105: // Start timestamp 1, timestamp 2 > What does 'Start timestamp' mean? May be, update this comment to clarify t removed PS5, Line 164: mgr.AdjustSafeTime(t3); > Why did it appear in this revision of the test? Does this mean the previou this patch is about removing automatic safe time adjustment apis from mvcc. since we don't have the automatic apis anymore we have to do it "manually" (this would be called within CommitTransaction above). Added a comment to that regard though honestly don't know whether it conveyed much more information than the code statement. PS5, Line 217: TestOfflineTransactions > can you update the terminology and the comment here? we don't use the word Done Line 228: // and committing this transaction "offline" this > same Done Line 546: // coalesce to the latest timestamp, for offline transactions. > same (and elsewhere -- grep for 'offline' in this test) Done PS5, Line 552: CHECK_OK > Why not CHECK_OK(), not ASSERT_OK() ? Done http://gerrit.cloudera.org:8080/#/c/5057/5/src/kudu/tablet/mvcc.cc File src/kudu/tablet/mvcc.cc: PS5, Line 54: There is already a transaction with timestamp: $0 in flight > this error string isn't necessarily accurate in the case that InitTransacti yeah this race can't happen anymore because ts assignment and txn start are done under a lock now, for test. did a sanity check. there was a slowdown of about 10% on mt-tablet-test, but this is not the actual tserver write patch. ran full_stack-insert-scan-test there are there is actually a speedup of 9%. PS5, Line 55: > weird indent Done PS5, Line 125: // If this transaction was the earliest in-flight, we might have to adjust : // the "clean" timestamp. : AdjustCleanTime(); > ah, so my comment in the other review about clean/safe time not having a 's yeah, safe/clean time behave as before. As I mentioned on the other patch (funnily or not :)) until now this patch series doesn't actually change any behavior. We were advancing "safe" time before apply. meaning that the "automatic' advancement on commit for leader txns would only really either move the "clean" time or nothing at all, which is what still happens now. PS5, Line 182: no_new_transactions_at_or_before_ > should we rename this member variable to 'safe_time_'? Done http://gerrit.cloudera.org:8080/#/c/5057/5/src/kudu/tablet/mvcc.h File src/kudu/tablet/mvcc.h: PS5, Line 168: // When a trans
[kudu-CR] KUDU-798 (part 2) Remove automatic safe time adjustment from mvcc
Hello Mike Percy, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5057 to look at the new patch set (#6). Change subject: KUDU-798 (part 2) Remove automatic safe time adjustment from mvcc .. KUDU-798 (part 2) Remove automatic safe time adjustment from mvcc This patch completely removes the APIs that allow automatic timestamp assignment and safe time adjustment from Mvcc. The previous patch had taken care of making sure these APIs were unused in regular TabletServer operations. This patch completes that by removing the APIs from Mvcc and changing tests appropriately. Because timestamp assignment and transaction start are now done under a lock, this does have some impact on some tests like mt-tablet-test. I measured about 10% slowdown on an mvcc-stressing config of mt-tablet-test (perf stats in the end of this commit message). However this patch series does not add any overhead on the actual tablet server write path. In fact this even simplifies it in some circumstances (for instance we were advancing safe time twice for leader side txns, one when it was consensus committed and one when the apply was complete). Due to this there was even a moderate perf gain in this more important case of about 9%. === full_stack-insert-scan-test run config: bin/full_stack-insert-scan-test \ --gtest_filter=*MRSOnlyStressTest* \ --inserts_per_client=20 \ --concurrent_inserts=10 \ --rows_per_batch=1 \ --skip_scans Results on master before this patch series: 344086.766163 task-clock#5.659 CPUs utilized 14,435,389 context-switches #0.042 M/sec 91,225 cpu-migrations#0.265 K/sec 112,036 page-faults #0.326 K/sec 956,637,266,040 cycles#2.780 GHz stalled-cycles-frontend stalled-cycles-backend 577,637,169,921 instructions #0.60 insns per cycle 109,720,205,884 branches # 318.874 M/sec 730,807,372 branch-misses #0.67% of all branches 60.807013290 seconds time elapsed Results on master after this patch series: 328574.272742 task-clock#5.932 CPUs utilized 13,820,330 context-switches #0.042 M/sec 170,370 cpu-migrations#0.519 K/sec 111,997 page-faults #0.341 K/sec 911,264,247,114 cycles#2.773 GHz stalled-cycles-frontend stalled-cycles-backend 572,898,176,369 instructions #0.63 insns per cycle 108,872,113,035 branches # 331.347 M/sec 728,934,228 branch-misses #0.67% of all branches 55.387672052 seconds time elapsed === mt-tablet-test run config: bin/mt-tablet-test \ --gtest_filter=*0*DoTestAllAtOnce* \ --num_counter_threads=0 \ --num_slowreader_threads=0 \ --flusher_backoff=1.0 \ --flusher_initial_frequency_ms=1 \ --inserts_per_thread=100 \ --num_summer_threads=0 \ --gtest_repeat=3 \ --tablet_test_flush_threshold_mb=2000 \ --minloglevel=10 Results on master before this patch series: 618894.806683 task-clock#7.716 CPUs utilized 7,243,713 context-switches #0.012 M/sec 782 cpu-migrations#0.001 K/sec 1,457,677 page-faults #0.002 M/sec 1,794,712,092,284 cycles#2.900 GHz stalled-cycles-frontend stalled-cycles-backend 648,757,335,519 instructions #0.36 insns per cycle 128,766,989,780 branches # 208.060 M/sec 790,345,583 branch-misses #0.61% of all branches 80.204388663 seconds time elapsed Result on master after this patch series: 620594.911121 task-clock#7.012 CPUs utilized 17,645,922 context-switches #0.028 M/sec 1,163 cpu-migrations#0.002 K/sec 1,252,812 page-faults #0.002 M/sec 1,775,937,664,119 cycles#2.862 GHz stalled-cycles-frontend stalled-cycles-backend 757,882,564,990 instructions #0.43 insns per cycle 155,500,757,477 branches # 250.567 M/sec 982,110,478 branch-misses #0.63% of all branches 88.500973260 seconds time elapsed Change-Id: I6cd0b8aa52e4b0f5c0f4872d806e76387f6b9538 --- M src/kudu/tablet/compaction-test.cc M src/kudu/tablet/deltamemstore-test.cc M src/kudu/tablet/diskrowset-test-base.h M src/kudu/tablet/diskrowset-test.cc M src/kudu/tablet/local_tablet_writer.h M src/kudu/tablet/memrowset-test.cc M src/kudu/tablet/mvcc-test.cc M sr
