[kudu-CR] [compaction] KUDU-1400: Improve rowset compaction policy to consider merging small DRSs
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/11869 )
Change subject: [compaction] KUDU-1400: Improve rowset compaction policy to
consider merging small DRSs
..
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11869/4/src/kudu/tablet/compaction_policy-test.cc
File src/kudu/tablet/compaction_policy-test.cc:
http://gerrit.cloudera.org:8080/#/c/11869/4/src/kudu/tablet/compaction_policy-test.cc@381
PS4, Line 381: if (divisor > 2) {
: ASSERT_EQ(rowsets.size(), picked.size());
> Ah I thought you were pondering the general case, rather than 3 specificall
Yep, I understand the general case. That was more about having explicit
scenarios for the cases which are closer to the
'switch-the-compaction-behavior' boundaries. If those alpha and gamma
constants are to be updated at some point, I think it's better have a snapshot
on the expected behavior to spot as many regressions as possible.
http://gerrit.cloudera.org:8080/#/c/11869/5/src/kudu/tablet/compaction_policy-test.cc
File src/kudu/tablet/compaction_policy-test.cc:
http://gerrit.cloudera.org:8080/#/c/11869/5/src/kudu/tablet/compaction_policy-test.cc@362
PS5, Line 362: { 32, 16, 8, 4, 2, 1 }
So, no need to have the coverage for the divisor of 3?
--
To view, visit http://gerrit.cloudera.org:8080/11869
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b421c6ed77d28ebab9b91a4d6fcb1e825997e6c
Gerrit-Change-Number: 11869
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Will Berkeley
Gerrit-Comment-Date: Sat, 17 Nov 2018 02:47:19 +
Gerrit-HasComments: Yes
[kudu-CR] [tests] fix flake in TestRandomHistoryGCWorkload
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11945 ) Change subject: [tests] fix flake in TestRandomHistoryGCWorkload .. [tests] fix flake in TestRandomHistoryGCWorkload This patch fixes a flake most prominent in TSAN builds for the RandomizedTabletHistoryGcITest.TestRandomHistoryGCWorkload scenario of the tablet_history_gc-itest suite. Before: dist_test --stress_cpu_threads=16: 256 out of 256 failed http://dist-test.cloudera.org/job?job_id=aserbin.1542394875.56611 After: dist_test --stress_cpu_threads=16: 0 out of 256 failed http://dist-test.cloudera.org/job?job_id=aserbin.1542397028.71131 In addition, this patch also contains a minor code clean up of the tablet_history_gc-itest suite to take advantage of the 'using' declarations. Change-Id: I8d146d4b83c8d488d3c1766a53fe4a4d322b6590 Reviewed-on: http://gerrit.cloudera.org:8080/11945 Reviewed-by: Adar Dembo Tested-by: Kudu Jenkins --- M src/kudu/integration-tests/tablet_history_gc-itest.cc 1 file changed, 15 insertions(+), 13 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/11945 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I8d146d4b83c8d488d3c1766a53fe4a4d322b6590 Gerrit-Change-Number: 11945 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [tests] fix flake in TestRandomHistoryGCWorkload
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/11945 )
Change subject: [tests] fix flake in TestRandomHistoryGCWorkload
..
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11945/1/src/kudu/integration-tests/tablet_history_gc-itest.cc
File src/kudu/integration-tests/tablet_history_gc-itest.cc:
http://gerrit.cloudera.org:8080/#/c/11945/1/src/kudu/integration-tests/tablet_history_gc-itest.cc@519
PS1, Line 519: # if !defined(THREAD_SANITIZER)
: OverrideFlagForSlowTests("test_num_rounds",
:Substitute("$0",
FLAGS_test_num_rounds * 5));
: # endif
> So before this change, in TSAN mode, the test would time out due to the hig
Yes -- the workload we use in this scenario and manual compaction/flushing
leads to a situation where too many rowsets are accumulated and it takes longer
and longer for write operation (especially update) to complete. Eventually, it
gets really ugly:
W1116 19:03:13.326668 7226 rpcz_store.cc:253] Call
kudu.tserver.TabletServerService.Write from 127.0.0.1:46428 (ReqId={client:
ae42f5eb2c2344068da4093cccfe156c, seq_no=38, attempt_no=0}) took 19083 ms (19.1
s). Client timeout 1 ms (20 s)
W1116 19:03:13.329354 7226 rpcz_store.cc:259] Trace:
1116 19:02:54.243665 (+ 0us) service_pool.cc:162] Inserting onto call queue
1116 19:02:54.243908 (+ 243us) service_pool.cc:221] Handling call
1116 19:03:13.326412 (+19082504us) inbound_call.cc:162] Queueing success
response
Related trace 'txn':
1116 19:02:54.251341 (+ 0us) write_transaction.cc:100] PREPARE: Starting
1116 19:02:54.251519 (+ 178us) write_transaction.cc:267] Acquiring schema
lock in shared mode
1116 19:02:54.251572 (+53us) write_transaction.cc:270] Acquired schema lock
1116 19:02:54.251607 (+35us) tablet.cc:437] PREPARE: Decoding operations
1116 19:02:54.282224 (+ 30617us) tablet.cc:459] PREPARE: Acquiring locks for
626 operations
1116 19:02:54.390295 (+108071us) tablet.cc:463] PREPARE: locks acquired
1116 19:02:54.390316 (+21us) write_transaction.cc:125] PREPARE: finished.
1116 19:02:54.390479 (+ 163us) write_transaction.cc:135] Start()
1116 19:02:54.398643 (+ 8164us) write_transaction.cc:140] Timestamp: P:
24 usec, L: 179
1116 19:02:54.399618 (+ 975us) log.cc:587] Serialized 4555 byte log entry
1116 19:02:54.411726 (+ 12108us) write_transaction.cc:148] APPLY: Starting
1116 19:03:13.305541 (+18893815us) tablet_metrics.cc:371] ProbeStats:
bloom_lookups=1269,key_file_lookups=1263,delta_file_lookups=3457,mrs_lookups=0
1116 19:03:13.310397 (+ 4856us) log.cc:587] Serialized 5027 byte log entry
1116 19:03:13.311747 (+ 1350us) write_transaction.cc:308] Releasing row and
schema locks
1116 19:03:13.324116 (+ 12369us) write_transaction.cc:276] Released schema lock
1116 19:03:13.326006 (+ 1890us) write_transaction.cc:195] FINISH: updating
metrics
--
To view, visit http://gerrit.cloudera.org:8080/11945
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8d146d4b83c8d488d3c1766a53fe4a4d322b6590
Gerrit-Change-Number: 11945
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 16 Nov 2018 21:57:04 +
Gerrit-HasComments: Yes
[kudu-CR] [compaction] KUDU-1400: Improve rowset compaction policy to consider merging small DRSs
Andrew Wong has posted comments on this change. (
http://gerrit.cloudera.org:8080/11869 )
Change subject: [compaction] KUDU-1400: Improve rowset compaction policy to
consider merging small DRSs
..
Patch Set 5: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11869/4/src/kudu/tablet/compaction_policy-test.cc
File src/kudu/tablet/compaction_policy-test.cc:
http://gerrit.cloudera.org:8080/#/c/11869/4/src/kudu/tablet/compaction_policy-test.cc@381
PS4, Line 381: if (divisor > 2) {
: ASSERT_EQ(rowsets.size(), picked.size());
> Yep, I was asking about 3 adjacent rowsets of size 32/3 MB each. The idea
Ah I thought you were pondering the general case, rather than 3 specifically.
--
To view, visit http://gerrit.cloudera.org:8080/11869
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b421c6ed77d28ebab9b91a4d6fcb1e825997e6c
Gerrit-Change-Number: 11869
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Will Berkeley
Gerrit-Comment-Date: Fri, 16 Nov 2018 21:29:03 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2038: Support bitmap index
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11722 ) Change subject: KUDU-2038: Support bitmap index .. Patch Set 6: Here's a design doc that LiFu shared with me via Slack. It was originally a docx, but I've uploaded it as a gdoc to facilitate comments. If there's trouble accessing it, I can funnel comments from the gdoc to gerrit. https://docs.google.com/document/d/1U_RFs8piw30K7fjyl0m08v2f-eiIjOUOkJRGGXrCoVY/edit?usp=sharing -- To view, visit http://gerrit.cloudera.org:8080/11722 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0edaa0ef1dba2dbce85ebf15f0a731e4939a7860 Gerrit-Change-Number: 11722 Gerrit-PatchSet: 6 Gerrit-Owner: helifu Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: helifu Gerrit-Comment-Date: Fri, 16 Nov 2018 21:19:05 + Gerrit-HasComments: No
[kudu-CR] [tests] fix flake in TestRandomHistoryGCWorkload
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/11945 )
Change subject: [tests] fix flake in TestRandomHistoryGCWorkload
..
Patch Set 1: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11945/1/src/kudu/integration-tests/tablet_history_gc-itest.cc
File src/kudu/integration-tests/tablet_history_gc-itest.cc:
http://gerrit.cloudera.org:8080/#/c/11945/1/src/kudu/integration-tests/tablet_history_gc-itest.cc@519
PS1, Line 519: # if !defined(THREAD_SANITIZER)
: OverrideFlagForSlowTests("test_num_rounds",
:Substitute("$0",
FLAGS_test_num_rounds * 5));
: # endif
So before this change, in TSAN mode, the test would time out due to the high
number of rounds?
--
To view, visit http://gerrit.cloudera.org:8080/11945
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8d146d4b83c8d488d3c1766a53fe4a4d322b6590
Gerrit-Change-Number: 11945
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 16 Nov 2018 21:13:26 +
Gerrit-HasComments: Yes
[kudu-CR] [compaction] KUDU-1400: Improve rowset compaction policy to consider merging small DRSs
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/11869 )
Change subject: [compaction] KUDU-1400: Improve rowset compaction policy to
consider merging small DRSs
..
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11869/4/src/kudu/tablet/compaction_policy-test.cc
File src/kudu/tablet/compaction_policy-test.cc:
http://gerrit.cloudera.org:8080/#/c/11869/4/src/kudu/tablet/compaction_policy-test.cc@381
PS4, Line 381: if (divisor > 2) {
: ASSERT_EQ(rowsets.size(), picked.size());
> You're asking if 3 adjacent rowsets of size 32/3 MB each should be compacte
Yep, I was asking about 3 adjacent rowsets of size 32/3 MB each. The idea was:
if we expect those to be compacted, why not to add that divisor (i.e. 3) into
the list and have that case covered by this scenario as well.
--
To view, visit http://gerrit.cloudera.org:8080/11869
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b421c6ed77d28ebab9b91a4d6fcb1e825997e6c
Gerrit-Change-Number: 11869
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Will Berkeley
Gerrit-Comment-Date: Fri, 16 Nov 2018 21:11:27 +
Gerrit-HasComments: Yes
[kudu-CR] [tests] fix flake in TestRandomHistoryGCWorkload
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11945 Change subject: [tests] fix flake in TestRandomHistoryGCWorkload .. [tests] fix flake in TestRandomHistoryGCWorkload This patch fixes a flake most prominent in TSAN builds for the RandomizedTabletHistoryGcITest.TestRandomHistoryGCWorkload scenario of the tablet_history_gc-itest suite. Before: dist_test --stress_cpu_threads=16: 256 out of 256 failed http://dist-test.cloudera.org/job?job_id=aserbin.1542394875.56611 After: dist_test --stress_cpu_threads=16: 0 out of 256 failed http://dist-test.cloudera.org/job?job_id=aserbin.1542397028.71131 In addition, this patch also contains a minor code clean up of the tablet_history_gc-itest suite to take advantage of the 'using' declarations. Change-Id: I8d146d4b83c8d488d3c1766a53fe4a4d322b6590 --- M src/kudu/integration-tests/tablet_history_gc-itest.cc 1 file changed, 15 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/45/11945/1 -- To view, visit http://gerrit.cloudera.org:8080/11945 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I8d146d4b83c8d488d3c1766a53fe4a4d322b6590 Gerrit-Change-Number: 11945 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
[kudu-CR] [compaction] KUDU-1400: Improve rowset compaction policy to consider merging small DRSs
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11869 to look at the new patch set (#5). Change subject: [compaction] KUDU-1400: Improve rowset compaction policy to consider merging small DRSs .. [compaction] KUDU-1400: Improve rowset compaction policy to consider merging small DRSs This implements the small rowset compaction scheme explained in the KUDU-1400 design doc [1]. While the implementation is simple, the reasoning behind it is nuanced, and I'll defer the explanations of them to the design doc rather than repeating them here. The three relevant constant factors: --compaction_minimum_improvement_score --compaction_small_rowset_tradeoff kSupportAdjust have been tuned to work well with each other. See the relevant tests for reasoning on why, and validation of what the results should be. Since compaction policy's performance is important, I ran the YCSB benchmark before and after the change using 'perf stat' to compare the performance: Command: Before: Performance counter stats for 'bin/compaction_policy-test --gtest_filter=*Ycsb*' (10 runs): 1231.095442 task-clock#0.997 CPUs utilized ( +- 2.00% ) 148 context-switches #0.120 K/sec ( +- 2.43% ) 9 cpu-migrations#0.007 K/sec ( +- 16.14% ) 3,630 page-faults #0.003 M/sec ( +- 0.00% ) 3,251,530,478 cycles#2.641 GHz ( +- 2.04% ) stalled-cycles-frontend stalled-cycles-backend 5,772,319,429 instructions #1.78 insns per cycle ( +- 0.01% ) 1,070,627,520 branches # 869.654 M/sec ( +- 0.01% ) 13,583,368 branch-misses #1.27% of all branches ( +- 0.10% ) 1.235037947 seconds time elapsed ( +- 2.00% ) After: Performance counter stats for 'bin/compaction_policy-test --gtest_filter=*Ycsb*' (10 runs): 1297.749333 task-clock#0.994 CPUs utilized ( +- 2.17% ) 158 context-switches #0.122 K/sec ( +- 3.01% ) 14 cpu-migrations#0.011 K/sec ( +- 17.48% ) 3,636 page-faults #0.003 M/sec 3,509,480,140 cycles#2.704 GHz ( +- 2.65% ) stalled-cycles-frontend stalled-cycles-backend 6,800,316,126 instructions #1.94 insns per cycle ( +- 0.01% ) 1,073,111,416 branches # 826.902 M/sec ( +- 0.01% ) 13,574,780 branch-misses #1.26% of all branches ( +- 0.15% ) 1.305058206 seconds time elapsed ( +- 2.16% ) A follow up will integrate the below design doc with the existing one, docs/design-docs/compaction-policy.md. [1]: https://docs.google.com/document/d/1yTfxt0_2p5EfIjCnjJCt3o-nB9xk-Kl2O8yKTA1LQrQ/edit?usp=sharing Change-Id: I7b421c6ed77d28ebab9b91a4d6fcb1e825997e6c --- M src/kudu/tablet/compaction_policy-test.cc M src/kudu/tablet/compaction_policy.cc M src/kudu/tablet/rowset_info.cc M src/kudu/tablet/rowset_info.h 4 files changed, 297 insertions(+), 45 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/69/11869/5 -- To view, visit http://gerrit.cloudera.org:8080/11869 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7b421c6ed77d28ebab9b91a4d6fcb1e825997e6c Gerrit-Change-Number: 11869 Gerrit-PatchSet: 5 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] [compaction] KUDU-1400: Improve rowset compaction policy to consider merging small DRSs
Will Berkeley has posted comments on this change. (
http://gerrit.cloudera.org:8080/11869 )
Change subject: [compaction] KUDU-1400: Improve rowset compaction policy to
consider merging small DRSs
..
Patch Set 4:
(14 comments)
http://gerrit.cloudera.org:8080/#/c/11869/4/src/kudu/tablet/compaction_policy-test.cc
File src/kudu/tablet/compaction_policy-test.cc:
http://gerrit.cloudera.org:8080/#/c/11869/4/src/kudu/tablet/compaction_policy-test.cc@321
PS4, Line 321: if doing so
: // doesn't actually reduce the number of rowsets
> Does it make sense to add a test to see how the compaction algorithm behave
An original test, TestBudgetedSelection, tests the case of a small number of
overlapping rowsets. I don't think having many rowsets overlapping tests the
code in any new way beyond having 3.
http://gerrit.cloudera.org:8080/#/c/11869/4/src/kudu/tablet/compaction_policy-test.cc@336
PS4, Line 336: [0 - 1]
> nit: it seems this one is extra -- the index starts with 1. Instead, it se
Done
http://gerrit.cloudera.org:8080/#/c/11869/4/src/kudu/tablet/compaction_policy-test.cc@345
PS4, Line 345: ASSERT_EQ(quality, 0.0);
> nit: the expected values comes first for XXX_EQ() test macros
Done. Here and elsewhere.
http://gerrit.cloudera.org:8080/#/c/11869/4/src/kudu/tablet/compaction_policy-test.cc@344
PS4, Line 344: ASSERT_TRUE(picked.empty());
: ASSERT_EQ(quality, 0.0);
> nit: think it's worth using EXPECT* here so we can run over all rowsets, ev
No, because the expectation for the loops is that if it fails one one
iteration, it fails for all subsequent ones, so the extra EXPECT_* failures
would just be noise.
http://gerrit.cloudera.org:8080/#/c/11869/4/src/kudu/tablet/compaction_policy-test.cc@381
PS4, Line 381: if (divisor > 2) {
: ASSERT_EQ(rowsets.size(), picked.size());
> I think that isn't what we want. As the number of rowsets increases, the (N
I think Alexey is asking about something else.
http://gerrit.cloudera.org:8080/#/c/11869/4/src/kudu/tablet/compaction_policy-test.cc@381
PS4, Line 381: if (divisor > 2) {
: ASSERT_EQ(rowsets.size(), picked.size());
> Should it also hold if the divisor is 3 in case of 3 rowsets? Or it doesn'
You're asking if 3 adjacent rowsets of size 32/3 MB each should be compacted
together. That's a decision about how important we think small rowset
compaction is. Maybe they should be, maybe not, but the various choices I'm
enforcing in this test have already exhausted most of my ability to tune the
weights and coefficients, and think they represent reasonable choices, so
whatever actually does happen is fine with me.
http://gerrit.cloudera.org:8080/#/c/11869/4/src/kudu/tablet/compaction_policy-test.cc@391
PS4, Line 391: FLAGS_budgeted_compaction_target_rowset_size
> nit: does it make sense to use 'target_size_bytes' as well?
Done
http://gerrit.cloudera.org:8080/#/c/11869/4/src/kudu/tablet/compaction_policy.cc
File src/kudu/tablet/compaction_policy.cc:
http://gerrit.cloudera.org:8080/#/c/11869/4/src/kudu/tablet/compaction_policy.cc@126
PS4, Line 126: const RowSetInfo*
> nit: not related to this particular patch, but if item_type is typedef'ed,
Done
http://gerrit.cloudera.org:8080/#/c/11869/4/src/kudu/tablet/compaction_policy.cc@268
PS4, Line 268: double MaybePenalizeWideSolution(double sum_width, double
union_width) {
: return sum_width <= union_width ? sum_width - union_width :
: sum_width - kSupportAdjust *
union_width;
: }
> This is worth a comment. My attempt:
Done
http://gerrit.cloudera.org:8080/#/c/11869/4/src/kudu/tablet/rowset_info.h
File src/kudu/tablet/rowset_info.h:
http://gerrit.cloudera.org:8080/#/c/11869/4/src/kudu/tablet/rowset_info.h@87
PS4, Line 87: by this RowSet
> nit: not your fault, but mind tweaking this to be "is spanned by the min/ma
Done
http://gerrit.cloudera.org:8080/#/c/11869/4/src/kudu/tablet/rowset_info.cc
File src/kudu/tablet/rowset_info.cc:
http://gerrit.cloudera.org:8080/#/c/11869/4/src/kudu/tablet/rowset_info.cc@65
PS4, Line 65: compaction_small_rowset_tradeoff
> +1, same above?
Done
http://gerrit.cloudera.org:8080/#/c/11869/4/src/kudu/tablet/rowset_info.cc@65
PS4, Line 65: compaction_small_rowset_tradeoff
> nit: is it worth marking this flag with the 'runtime' tag?
Done
http://gerrit.cloudera.org:8080/#/c/11869/4/src/kudu/tablet/rowset_info.cc@90
PS4, Line 90: --compaction_small_rowset_tradeoff=$0 must be less than "
: "--compaction_minimum_improvement=$1 in order to prevent
pointless "
: "compactions; if you know what you are doing, pass "
: "--compaction_force_small_rowset_tradeoff to permit this",
> nit: consistent -flag_dashes with the above flag?
Done
http://gerrit.cloudera.org:8080/#/c/11869/4/src/kudu/tablet/rowset_info.cc@230
[kudu-CR] Fix some issues in HMS and Sentry test fixtures
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11943 ) Change subject: Fix some issues in HMS and Sentry test fixtures .. Fix some issues in HMS and Sentry test fixtures 1. The HMS or Sentry client object may be null in TearDown. If we don't check, we'll deref a null pointer if there's an error during SetUp() (e.g. if the HMS or Sentry server failed to start). 2. The HMSCatalogTest fixture should properly chain to its superclass. Change-Id: Ib0376b972fe6add6d9312aea6944c9ab1a03f25f Reviewed-on: http://gerrit.cloudera.org:8080/11943 Reviewed-by: Hao Hao Tested-by: Kudu Jenkins --- M src/kudu/hms/hms_catalog-test.cc M src/kudu/sentry/sentry-test-base.h 2 files changed, 8 insertions(+), 2 deletions(-) Approvals: Hao Hao: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/11943 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ib0376b972fe6add6d9312aea6944c9ab1a03f25f Gerrit-Change-Number: 11943 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] Fix some issues in HMS and Sentry test fixtures
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11943 ) Change subject: Fix some issues in HMS and Sentry test fixtures .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11943 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib0376b972fe6add6d9312aea6944c9ab1a03f25f Gerrit-Change-Number: 11943 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 16 Nov 2018 18:12:43 + Gerrit-HasComments: No
[kudu-CR] Fix some issues in HMS and Sentry test fixtures
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11943 ) Change subject: Fix some issues in HMS and Sentry test fixtures .. Patch Set 1: Here's problem #1 in the wild: http://dist-test.cloudera.org:8080/diagnose?key=bad20370-e4da-11e8-9b35-0242ac110002 -- To view, visit http://gerrit.cloudera.org:8080/11943 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib0376b972fe6add6d9312aea6944c9ab1a03f25f Gerrit-Change-Number: 11943 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 16 Nov 2018 18:07:57 + Gerrit-HasComments: No
[kudu-CR] Fix some issues in HMS and Sentry test fixtures
Hello Andrew Wong, Hao Hao, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/11943 to review the following change. Change subject: Fix some issues in HMS and Sentry test fixtures .. Fix some issues in HMS and Sentry test fixtures 1. The HMS or Sentry client object may be null in TearDown. If we don't check, we'll deref a null pointer if there's an error during SetUp() (e.g. if the HMS or Sentry server failed to start). 2. The HMSCatalogTest fixture should properly chain to its superclass. Change-Id: Ib0376b972fe6add6d9312aea6944c9ab1a03f25f --- M src/kudu/hms/hms_catalog-test.cc M src/kudu/sentry/sentry-test-base.h 2 files changed, 8 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/43/11943/1 -- To view, visit http://gerrit.cloudera.org:8080/11943 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ib0376b972fe6add6d9312aea6944c9ab1a03f25f Gerrit-Change-Number: 11943 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao
[kudu-CR](gh-pages) [blog] Call for Posts
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/11941 ) Change subject: [blog] Call for Posts .. Patch Set 2: here's a rendering of the post: http://bukor.me/kudu/2018/11/16/call-for-posts.html This should be merged only after https://gerrit.cloudera.org/c/11940/ is merged and the new docs have been generated and merged as this post refers to an anchor on the contributing page that doesn't yet exist. -- To view, visit http://gerrit.cloudera.org:8080/11941 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: comment Gerrit-Change-Id: Icd801585b1be525c635b9def937f9dd0d28fdec8 Gerrit-Change-Number: 11941 Gerrit-PatchSet: 2 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Mike Percy Gerrit-Comment-Date: Fri, 16 Nov 2018 15:51:52 + Gerrit-HasComments: No
[kudu-CR](gh-pages) [blog] Call for Posts
Hello Mike Percy, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11941 to look at the new patch set (#2). Change subject: [blog] Call for Posts .. [blog] Call for Posts This commit adds a call for guest posts as agreed on the latest virtual meetup on Slack Change-Id: Icd801585b1be525c635b9def937f9dd0d28fdec8 --- A _posts/2018-11-16-call-for-posts.md 1 file changed, 20 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/11941/2 -- To view, visit http://gerrit.cloudera.org:8080/11941 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: newpatchset Gerrit-Change-Id: Icd801585b1be525c635b9def937f9dd0d28fdec8 Gerrit-Change-Number: 11941 Gerrit-PatchSet: 2 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Mike Percy
[kudu-CR](gh-pages) [blog] Call for Posts
Hello Mike Percy, Grant Henke, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/11941 to review the following change. Change subject: [blog] Call for Posts .. [blog] Call for Posts This commit adds a call for guest posts as agreed on the latest virtual meetup on Slack Change-Id: Icd801585b1be525c635b9def937f9dd0d28fdec8 --- A _posts/2018-11-16-call-for-posts.md 1 file changed, 20 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/11941/1 -- To view, visit http://gerrit.cloudera.org:8080/11941 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: newchange Gerrit-Change-Id: Icd801585b1be525c635b9def937f9dd0d28fdec8 Gerrit-Change-Number: 11941 Gerrit-PatchSet: 1 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Mike Percy
[kudu-CR] [docs] Contributing to blog
Hello Mike Percy, Grant Henke, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/11940 to review the following change. Change subject: [docs] Contributing to blog .. [docs] Contributing to blog Submitting blog posts are not straightforward, especially if someone hasn't used Jekyll and/or Gerrit. This commit adds a "blog posts" section to our contributing docs. Change-Id: Ifd8ccae4b15b1ad8b679e0d2d8eabdf5fb5e3a09 --- M docs/contributing.adoc 1 file changed, 74 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/11940/1 -- To view, visit http://gerrit.cloudera.org:8080/11940 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ifd8ccae4b15b1ad8b679e0d2d8eabdf5fb5e3a09 Gerrit-Change-Number: 11940 Gerrit-PatchSet: 1 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Mike Percy
