[kudu-CR] [docs] update the upgrade documentation
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13820 ) Change subject: [docs] update the upgrade documentation .. Patch Set 3: Code-Review+1 (1 comment) Would be good for someone else to review too. http://gerrit.cloudera.org:8080/#/c/13820/3/docs/installation.adoc File docs/installation.adoc: http://gerrit.cloudera.org:8080/#/c/13820/3/docs/installation.adoc@652 PS3, Line 652: restarted tablet servers "Replicas hosted on restarted tablet servers" -- To view, visit http://gerrit.cloudera.org:8080/13820 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6b3e5c549dc05c3388c0b0dd628d205a356da344 Gerrit-Change-Number: 13820 Gerrit-PatchSet: 3 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Priyanka Chheda Gerrit-Reviewer: helifu Gerrit-Comment-Date: Thu, 11 Jul 2019 05:55:09 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-1938 Add support for CHAR/VARCHAR pt 1
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/13760 )
Change subject: KUDU-1938 Add support for CHAR/VARCHAR pt 1
..
Patch Set 16:
(15 comments)
http://gerrit.cloudera.org:8080/#/c/13760/16//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13760/16//COMMIT_MSG@10
PS16, Line 10: Follow up commits will add integration to other clients. The
CHAR and
Mind separating the C++ client piece into a separate commit too? I'd like to
review the server side pieces without any distractions if it's not much work.
http://gerrit.cloudera.org:8080/#/c/13760/16/src/kudu/common/partial_row.h
File src/kudu/common/partial_row.h:
http://gerrit.cloudera.org:8080/#/c/13760/16/src/kudu/common/partial_row.h@148
PS16, Line 148: /// @name Setters for binary/string columns by name (copying).
Update.
http://gerrit.cloudera.org:8080/#/c/13760/16/src/kudu/common/partial_row.h@171
PS16, Line 171: /// @name Setters for binary/string columns by index
(copying).
Update.
http://gerrit.cloudera.org:8080/#/c/13760/16/src/kudu/common/partial_row.h@432
PS16, Line 432: Status GetCharVarchar(const Slice& col_name, Slice* val)
const WARN_UNUSED_RESULT;
Shouldn't we have separate GetChar and GetVarchar getters, as you did for
setters?
http://gerrit.cloudera.org:8080/#/c/13760/16/src/kudu/common/partial_row.cc
File src/kudu/common/partial_row.cc:
http://gerrit.cloudera.org:8080/#/c/13760/16/src/kudu/common/partial_row.cc@239
PS16, Line 239: if (col.type_info()->type() == BINARY) {
: dst = schema_->ExtractColumnFromRow(row, col_idx);
: } else if (col.type_info()->type() == CHAR) {
: dst = schema_->ExtractColumnFromRow(row, col_idx);
: } else if (col.type_info()->type() == VARCHAR) {
: dst = schema_->ExtractColumnFromRow(row,
col_idx);
: } else {
: CHECK(col.type_info()->type() == STRING);
: dst = schema_->ExtractColumnFromRow(row, col_idx);
: }
Convert into a switch.
http://gerrit.cloudera.org:8080/#/c/13760/16/src/kudu/common/partial_row.cc@440
PS16, Line 440: size_t length = col.type_attributes().length;
This is only used in one place; consider not using a local for it.
http://gerrit.cloudera.org:8080/#/c/13760/16/src/kudu/common/partial_row.cc@441
PS16, Line 441: resize
It's late but I have no idea what resize() is.
http://gerrit.cloudera.org:8080/#/c/13760/16/src/kudu/common/partial_row.cc@793
PS16, Line 793: decimal
Not decimal
http://gerrit.cloudera.org:8080/#/c/13760/16/src/kudu/common/schema.h
File src/kudu/common/schema.h:
http://gerrit.cloudera.org:8080/#/c/13760/16/src/kudu/common/schema.h@123
PS16, Line 123: uint16_t length;
Why uint16_t? What does this implicitly say about the maximum length of a CHAR
or VARCHAR?
http://gerrit.cloudera.org:8080/#/c/13760/16/src/kudu/util/char_util.h
File src/kudu/util/char_util.h:
http://gerrit.cloudera.org:8080/#/c/13760/16/src/kudu/util/char_util.h@28
PS16, Line 28: namespace kudu {
These function names are a little too generic (especially resize). Plus they
should use CamelCase naming.
http://gerrit.cloudera.org:8080/#/c/13760/16/src/kudu/util/char_util.h@38
PS16, Line 38: /// Check the UTF8 and character length of the slice up to a
maximum length.
This isn't part of the public API, so you don't need to use Doxygen style
comments.
http://gerrit.cloudera.org:8080/#/c/13760/16/src/kudu/util/char_util.h@60
PS16, Line 60: bool expand
Define a two-state enum instead; it'll be more readable.
http://gerrit.cloudera.org:8080/#/c/13760/16/src/kudu/util/char_util.cc
File src/kudu/util/char_util.cc:
http://gerrit.cloudera.org:8080/#/c/13760/16/src/kudu/util/char_util.cc@27
PS16, Line 27: void utf8length(const Slice& slice, size_t* utf8_length,
Does this duplicate functionality from gutil/utf?
http://gerrit.cloudera.org:8080/#/c/13760/16/src/kudu/util/char_util.cc@46
PS16, Line 46: char_length += length - utf8_length;
It's a little confusing as to why we need three different kinds of length here.
Could you add some comments?
http://gerrit.cloudera.org:8080/#/c/13760/16/src/kudu/util/char_util.cc@66
PS16, Line 66: std::s
Add a "using std::string" at the top and drop this.
--
To view, visit http://gerrit.cloudera.org:8080/13760
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I998982dba93831db91c43a97ce30d3e68c2a4a54
Gerrit-Change-Number: 13760
Gerrit-PatchSet: 16
Gerrit-Owner: Attila Bukor
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Will
[kudu-CR] [docs] update the upgrade documentation
Hello Kudu Jenkins, Adar Dembo, Priyanka Chheda, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13820 to look at the new patch set (#3). Change subject: [docs] update the upgrade documentation .. [docs] update the upgrade documentation The process of upgrading the cluster has been added to the installation.adoc. Change-Id: I6b3e5c549dc05c3388c0b0dd628d205a356da344 --- M docs/installation.adoc 1 file changed, 52 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/13820/3 -- To view, visit http://gerrit.cloudera.org:8080/13820 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6b3e5c549dc05c3388c0b0dd628d205a356da344 Gerrit-Change-Number: 13820 Gerrit-PatchSet: 3 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Priyanka Chheda Gerrit-Reviewer: helifu
[kudu-CR] KUDU-2722 (table level): Support new 'write enabled' table extra config
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/13796 )
Change subject: KUDU-2722 (table level): Support new 'write_enabled' table
extra config
..
Patch Set 7:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/13796/2/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:
http://gerrit.cloudera.org:8080/#/c/13796/2/src/kudu/tserver/tablet_service.cc@1084
PS2, Line 1084: boost::optional extra_config =
tablet->metadata()->extra_config();
> Looks like you missed this.
Still missed this.
http://gerrit.cloudera.org:8080/#/c/13796/7/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:
http://gerrit.cloudera.org:8080/#/c/13796/7/src/kudu/tserver/tablet_service.cc@1146
PS7, Line 1146: Status s = Status::InvalidArgument("rejecting write
request: tablet is not writable");
: SetupErrorAndRespond(resp->mutable_error(), s,
Nit: why not combine these as before?
--
To view, visit http://gerrit.cloudera.org:8080/13796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd3768eda36d9574be9c41e7cd3dd81cc5ae8f3a
Gerrit-Change-Number: 13796
Gerrit-PatchSet: 7
Gerrit-Owner: XiaokaiWang
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: XiaokaiWang
Gerrit-Reviewer: Yao Xu
Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
Gerrit-Comment-Date: Thu, 11 Jul 2019 05:29:28 +
Gerrit-HasComments: Yes
[kudu-CR](branch-1.10.x) [docs] Update the site build to use the release directory
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13840 ) Change subject: [docs] Update the site build to use the release directory .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13840 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.10.x Gerrit-MessageType: comment Gerrit-Change-Id: I174de2d7041bba20cdc3dcacd3efbb5e736ecb63 Gerrit-Change-Number: 13840 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 11 Jul 2019 05:29:13 + Gerrit-HasComments: No
[kudu-CR](branch-1.10.x) Fix KuduScanTokenBuilder::SetDiffScan docs
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13839 ) Change subject: Fix KuduScanTokenBuilder::SetDiffScan docs .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13839 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.10.x Gerrit-MessageType: comment Gerrit-Change-Id: I448c00b78f9d14685e87d154e1bbee0eaa8e8e24 Gerrit-Change-Number: 13839 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 11 Jul 2019 05:29:06 + Gerrit-HasComments: No
[kudu-CR] KUDU-2722 (table level): Support new 'write enabled' table extra config
Hello Tidy Bot, Yingchun Lai, Kudu Jenkins, Yao Xu, Adar Dembo, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13796 to look at the new patch set (#7). Change subject: KUDU-2722 (table level): Support new 'write_enabled' table extra config .. KUDU-2722 (table level): Support new 'write_enabled' table extra config Change-Id: Ibd3768eda36d9574be9c41e7cd3dd81cc5ae8f3a --- M src/kudu/client/client-test.cc M src/kudu/common/common.proto M src/kudu/common/wire_protocol.cc M src/kudu/tablet/tablet_metadata-test.cc M src/kudu/tools/kudu-admin-test.cc M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/ts_tablet_manager-test.cc M src/kudu/tserver/tserver.proto 8 files changed, 127 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/96/13796/7 -- To view, visit http://gerrit.cloudera.org:8080/13796 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibd3768eda36d9574be9c41e7cd3dd81cc5ae8f3a Gerrit-Change-Number: 13796 Gerrit-PatchSet: 7 Gerrit-Owner: XiaokaiWang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: XiaokaiWang Gerrit-Reviewer: Yao Xu Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
[kudu-CR] Separate benchmark to single test
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13832 ) Change subject: Separate benchmark to single test .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13832 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I980234bcdf76a7d2978f49ed59dff21184abf892 Gerrit-Change-Number: 13832 Gerrit-PatchSet: 3 Gerrit-Owner: ZhangYao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: ZhangYao Gerrit-Comment-Date: Thu, 11 Jul 2019 05:12:06 + Gerrit-HasComments: No
[kudu-CR] Separate benchmark to single test
ZhangYao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13832 ) Change subject: Separate benchmark to single test .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/13832/2/src/kudu/common/wire_protocol-test.cc File src/kudu/common/wire_protocol-test.cc: http://gerrit.cloudera.org:8080/#/c/13832/2/src/kudu/common/wire_protocol-test.cc@64 PS2, Line 64: // Used for benchmark, int corresponds to the number of columns, : // double corresponds to the selection rate. > Nit: use C++-style commenting: Done :) -- To view, visit http://gerrit.cloudera.org:8080/13832 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I980234bcdf76a7d2978f49ed59dff21184abf892 Gerrit-Change-Number: 13832 Gerrit-PatchSet: 3 Gerrit-Owner: ZhangYao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: ZhangYao Gerrit-Comment-Date: Thu, 11 Jul 2019 05:10:25 + Gerrit-HasComments: Yes
[kudu-CR] Separate benchmark to single test
Hello Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13832 to look at the new patch set (#3). Change subject: Separate benchmark to single test .. Separate benchmark to single test Separate the total benchmark to single test with paraments so they all can be run in test time limit. Change-Id: I980234bcdf76a7d2978f49ed59dff21184abf892 --- M src/kudu/common/CMakeLists.txt M src/kudu/common/wire_protocol-test.cc 2 files changed, 14 insertions(+), 14 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/13832/3 -- To view, visit http://gerrit.cloudera.org:8080/13832 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I980234bcdf76a7d2978f49ed59dff21184abf892 Gerrit-Change-Number: 13832 Gerrit-PatchSet: 3 Gerrit-Owner: ZhangYao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: ZhangYao
[kudu-CR] [maintenance] Add extra config for maintenance manager task priority
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13659 ) Change subject: [maintenance] Add extra config for maintenance manager task priority .. Patch Set 3: Code-Review+2 Test failure looks unrelated. -- To view, visit http://gerrit.cloudera.org:8080/13659 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I966ee626ef85ce56ba4517b9e494f3ac5b044867 Gerrit-Change-Number: 13659 Gerrit-PatchSet: 3 Gerrit-Owner: Yingchun Lai <405403...@qq.com> Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Will Berkeley Gerrit-Reviewer: XiaokaiWang Gerrit-Reviewer: Yao Xu Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Comment-Date: Thu, 11 Jul 2019 05:01:52 + Gerrit-HasComments: No
[kudu-CR] KUDU-2855 Lazy-create DeltaMemStore on first update
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13821 ) Change subject: KUDU-2855 Lazy-create DeltaMemStore on first update .. KUDU-2855 Lazy-create DeltaMemStore on first update This patch supports lazy-create DeltaMemStore on first update to save memory and to fast-path out any DMS-related code. The created DeltaMemStore will be destroyed on the following flush. Change-Id: Ie0c565d86647d5144266b30aa6e8572d42db48c6 Reviewed-on: http://gerrit.cloudera.org:8080/13821 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo --- M src/kudu/tablet/delta_tracker.cc M src/kudu/tablet/delta_tracker.h M src/kudu/tablet/diskrowset-test-base.h M src/kudu/tablet/diskrowset-test.cc 4 files changed, 72 insertions(+), 43 deletions(-) Approvals: Kudu Jenkins: Verified Adar Dembo: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/13821 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ie0c565d86647d5144266b30aa6e8572d42db48c6 Gerrit-Change-Number: 13821 Gerrit-PatchSet: 5 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu
[kudu-CR] KUDU-2855 Lazy-create DeltaMemStore on first update
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13821 ) Change subject: KUDU-2855 Lazy-create DeltaMemStore on first update .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13821 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie0c565d86647d5144266b30aa6e8572d42db48c6 Gerrit-Change-Number: 13821 Gerrit-PatchSet: 4 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu Gerrit-Comment-Date: Thu, 11 Jul 2019 04:58:44 + Gerrit-HasComments: No
[kudu-CR] Separate benchmark to single test
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13832 ) Change subject: Separate benchmark to single test .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/13832/2/src/kudu/common/wire_protocol-test.cc File src/kudu/common/wire_protocol-test.cc: http://gerrit.cloudera.org:8080/#/c/13832/2/src/kudu/common/wire_protocol-test.cc@64 PS2, Line 64: /*Used for benchmark, int corresponds to the number of columns, : * double corresponds to the selection rate.*/ Nit: use C++-style commenting: // Used for benchmark, int corresponds to the number of columns, // double corresponds to the selection rate. -- To view, visit http://gerrit.cloudera.org:8080/13832 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I980234bcdf76a7d2978f49ed59dff21184abf892 Gerrit-Change-Number: 13832 Gerrit-PatchSet: 2 Gerrit-Owner: ZhangYao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: ZhangYao Gerrit-Comment-Date: Thu, 11 Jul 2019 04:55:07 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2823 Place tablet replicas based on dimension
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13632 ) Change subject: KUDU-2823 Place tablet replicas based on dimension .. Patch Set 16: (1 comment) http://gerrit.cloudera.org:8080/#/c/13632/12/src/kudu/master/master.proto File src/kudu/master/master.proto: http://gerrit.cloudera.org:8080/#/c/13632/12/src/kudu/master/master.proto@365 PS12, Line 365: // Indicates that the tablet server needs to report the number of tablets > > Still not clear on why this is necessary. Why not have tservers The docs are helpful, but I still don't fully understand the motivation. Can you make a reasonable argument that the code complexity of making this behavior optional is worth the optimization? -- To view, visit http://gerrit.cloudera.org:8080/13632 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I48a225e221eb42ef2f5489687e80a151d8dc1a42 Gerrit-Change-Number: 13632 Gerrit-PatchSet: 16 Gerrit-Owner: Yao Xu 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: Yao Xu Gerrit-Comment-Date: Thu, 11 Jul 2019 04:45:57 + Gerrit-HasComments: Yes
[kudu-CR] [maintenance] Add extra config for maintenance manager task priority
Yingchun Lai has posted comments on this change. (
http://gerrit.cloudera.org:8080/13659 )
Change subject: [maintenance] Add extra config for maintenance manager task
priority
..
Patch Set 3:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/13659/2/src/kudu/common/common.proto
File src/kudu/common/common.proto:
PS2:
> Do you think we should update all of the tests that xiaokai updated (https:
I don't think it's necessary, we should assume that 'table extra config' is a
stable feature, not needed to test it again and again. However, functional test
for the new added config is needed.
http://gerrit.cloudera.org:8080/#/c/13659/2/src/kudu/common/common.proto@449
PS2, Line 449: ance, it will be clamped into
: // range [-FLAGS_max_priority_range, FLAGS_max_priority_ran
> Hmm, but --max_priority_range can change. What's the expected behavior if i
Done
http://gerrit.cloudera.org:8080/#/c/13659/2/src/kudu/common/wire_protocol.h
File src/kudu/common/wire_protocol.h:
http://gerrit.cloudera.org:8080/#/c/13659/2/src/kudu/common/wire_protocol.h@154
PS2, Line 154: Int
> Nit: Int32 to be clearer about the size.
Done
http://gerrit.cloudera.org:8080/#/c/13659/2/src/kudu/common/wire_protocol.cc
File src/kudu/common/wire_protocol.cc:
http://gerrit.cloudera.org:8080/#/c/13659/2/src/kudu/common/wire_protocol.cc@666
PS2, Line 666: Status ParseInt32Config(const string& name, const string& value,
int32_t* result) {
> No need for std:: prefixes here.
Done
http://gerrit.cloudera.org:8080/#/c/13659/2/src/kudu/common/wire_protocol.cc@669
PS2, Line 669: unable
> Nit: "unable to..." because we often prepend other messages to a status.
Done
http://gerrit.cloudera.org:8080/#/c/13659/2/src/kudu/tablet/tablet_mm_ops.cc
File src/kudu/tablet/tablet_mm_ops.cc:
http://gerrit.cloudera.org:8080/#/c/13659/2/src/kudu/tablet/tablet_mm_ops.cc@96
PS2, Line 96: const auto& extra_config = tablet_->metadata()->extra_config();
: if (extra_config && extra_config->has_maintenance_priority()) {
: priority = extra_config->maintenance_priority();
: }
> This pattern (and the equivalent in tablet_replica_mm_ops.cc) doesn't seem
I'll add an unit test to check OPs priority after alter this extra config, but
I don't think it's necessary to repro this race, because there is no race after
the prior patch set.
--
To view, visit http://gerrit.cloudera.org:8080/13659
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I966ee626ef85ce56ba4517b9e494f3ac5b044867
Gerrit-Change-Number: 13659
Gerrit-PatchSet: 3
Gerrit-Owner: Yingchun Lai <405403...@qq.com>
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley
Gerrit-Reviewer: XiaokaiWang
Gerrit-Reviewer: Yao Xu
Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
Gerrit-Comment-Date: Thu, 11 Jul 2019 04:22:24 +
Gerrit-HasComments: Yes
[kudu-CR] [maintenance] Add extra config for maintenance manager task priority
Hello Will Berkeley, Tidy Bot, Kudu Jenkins, Andrew Wong, Yao Xu, Adar Dembo, XiaokaiWang, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13659 to look at the new patch set (#3). Change subject: [maintenance] Add extra config for maintenance manager task priority .. [maintenance] Add extra config for maintenance manager task priority While set and update priorities for multiply tables by GFlags is a compromise, now we can improve it by extra config of tables. Change-Id: I966ee626ef85ce56ba4517b9e494f3ac5b044867 --- M src/kudu/common/common.proto M src/kudu/common/wire_protocol.cc M src/kudu/common/wire_protocol.h M src/kudu/integration-tests/alter_table-test.cc M src/kudu/tablet/tablet.h M src/kudu/tablet/tablet_mm_ops.cc M src/kudu/tablet/tablet_mm_ops.h M src/kudu/tablet/tablet_replica.h M src/kudu/tablet/tablet_replica_mm_ops.cc M src/kudu/tablet/tablet_replica_mm_ops.h M src/kudu/util/maintenance_manager-test.cc M src/kudu/util/maintenance_manager.cc M src/kudu/util/maintenance_manager.h 13 files changed, 116 insertions(+), 96 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/59/13659/3 -- To view, visit http://gerrit.cloudera.org:8080/13659 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I966ee626ef85ce56ba4517b9e494f3ac5b044867 Gerrit-Change-Number: 13659 Gerrit-PatchSet: 3 Gerrit-Owner: Yingchun Lai <405403...@qq.com> Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Will Berkeley Gerrit-Reviewer: XiaokaiWang Gerrit-Reviewer: Yao Xu Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
[kudu-CR] KUDU-2855 Lazy-create DeltaMemStore on first update
Hello Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13821 to look at the new patch set (#4). Change subject: KUDU-2855 Lazy-create DeltaMemStore on first update .. KUDU-2855 Lazy-create DeltaMemStore on first update This patch supports lazy-create DeltaMemStore on first update to save memory and to fast-path out any DMS-related code. The created DeltaMemStore will be destroyed on the following flush. Change-Id: Ie0c565d86647d5144266b30aa6e8572d42db48c6 --- M src/kudu/tablet/delta_tracker.cc M src/kudu/tablet/delta_tracker.h M src/kudu/tablet/diskrowset-test-base.h M src/kudu/tablet/diskrowset-test.cc 4 files changed, 72 insertions(+), 43 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/21/13821/4 -- To view, visit http://gerrit.cloudera.org:8080/13821 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie0c565d86647d5144266b30aa6e8572d42db48c6 Gerrit-Change-Number: 13821 Gerrit-PatchSet: 4 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu
[kudu-CR] KUDU-2881 Support create/drop range partition by command line
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13833 ) Change subject: KUDU-2881 Support create/drop range partition by command line .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/13833/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13833/1//COMMIT_MSG@13 PS1, Line 13: 1. kudu table create_range_partition : [-lower_bound_type] [-upper_bound_type] : 2. kudu table drop_range_partition : [-lower_bound_type] [-upper_bound_type] Also have you considered how we would want to handle unbounded range partitions? -- To view, visit http://gerrit.cloudera.org:8080/13833 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I647d9c3cf01807bd8ae9f8cf4091568ea6f1161c Gerrit-Change-Number: 13833 Gerrit-PatchSet: 1 Gerrit-Owner: honeyhexin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yao Xu Gerrit-Comment-Date: Thu, 11 Jul 2019 03:04:18 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2881 Support create/drop range partition by command line
Andrew Wong has posted comments on this change. (
http://gerrit.cloudera.org:8080/13833 )
Change subject: KUDU-2881 Support create/drop range partition by command line
..
Patch Set 1:
(17 comments)
Thanks for the contribution! Looks good overall, many stylistic nit-picky
comments.
http://gerrit.cloudera.org:8080/#/c/13833/1/src/kudu/common/partition.h
File src/kudu/common/partition.h:
http://gerrit.cloudera.org:8080/#/c/13833/1/src/kudu/common/partition.h@260
PS1, Line 260: void GetRangeSchemaColumnIds(const Schema& schema,
std::vector& range_column_idxs) const;
> warning: non-const reference parameter 'range_column_idxs', make it const o
Alternatively, how about having this return vector?
http://gerrit.cloudera.org:8080/#/c/13833/1/src/kudu/common/partition.cc
File src/kudu/common/partition.cc:
http://gerrit.cloudera.org:8080/#/c/13833/1/src/kudu/common/partition.cc@1242
PS1, Line 1242: GetRangeSchemaColumnIds
Seems like is actually GetRangeSchemaColumnIndexes
http://gerrit.cloudera.org:8080/#/c/13833/1/src/kudu/integration-tests/ts_itest-base.h
File src/kudu/integration-tests/ts_itest-base.h:
http://gerrit.cloudera.org:8080/#/c/13833/1/src/kudu/integration-tests/ts_itest-base.h@158
PS1, Line 158: void InsertTestRows(client::KuduTable* table, int num_rows,
int first_row);
nit: can you document what this does and what the arguments do? Similar to the
other functions. Should also note any assumptions that a caller should be aware
of (e.g. is the schema of 'table' important?)
http://gerrit.cloudera.org:8080/#/c/13833/1/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:
http://gerrit.cloudera.org:8080/#/c/13833/1/src/kudu/tools/kudu-admin-test.cc@2345
PS1, Line 2345: TEST_F(AdminCliTest, TestCreateAndDropRangePartition) {
There's a decent amount of functionality that isn't tested by this, e.g. making
sure that all types are covered when parsing, making sure that we get the
expected behavior when passing correct or incorrect "exclusive"/"inclusive"
arguments.
http://gerrit.cloudera.org:8080/#/c/13833/1/src/kudu/tools/kudu-admin-test.cc@2378
PS1, Line 2378:
: //Insert 100
nit: Can you keep the comments consistent in terms of spacing? I.e.
// Insert 100 lines...
We try to keep the style of comments consistent, following the C++ Google style
guide https://google.github.io/styleguide/cppguide.html#Comment_Style
http://gerrit.cloudera.org:8080/#/c/13833/1/src/kudu/tools/kudu-admin-test.cc@2380
PS1, Line 2380: ASSERT_NO_FATAL_FAILURE
nit: For brevity, you can just do:
NO_FATALS(InsertTestRows(...));
Same in other places.
http://gerrit.cloudera.org:8080/#/c/13833/1/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
http://gerrit.cloudera.org:8080/#/c/13833/1/src/kudu/tools/tool_action_table.cc@53
PS1, Line 53: #include "kudu/common/partial_row.h"
nit: Can you sort this alphabetically?
http://gerrit.cloudera.org:8080/#/c/13833/1/src/kudu/tools/tool_action_table.cc@75
PS1, Line 75: using kudu::client::KuduTableCreator;
This too. Could you sort this alphabetically? It makes it easier to verify what
is present and what isn't.
Can you do the same in other files too?
http://gerrit.cloudera.org:8080/#/c/13833/1/src/kudu/tools/tool_action_table.cc@484
PS1, Line 484: /*field=*/nullptr,
: ));
nit: Could you add an extra space to align these with reader.root()?
http://gerrit.cloudera.org:8080/#/c/13833/1/src/kudu/tools/tool_action_table.cc@488
PS1, Line 488: auto &
nit: reverse the space order
const auto& partit...
http://gerrit.cloudera.org:8080/#/c/13833/1/src/kudu/tools/tool_action_table.cc@494
PS1, Line 494: key_indexes.size(),
: values.size()));
nit: Could you align the spacing here?
http://gerrit.cloudera.org:8080/#/c/13833/1/src/kudu/tools/tool_action_table.cc@500
PS1, Line 500: to = table->schema().Column(key_index);
: const auto
nit: spacing for `const auto&` here too
http://gerrit.cloudera.org:8080/#/c/13833/1/src/kudu/tools/tool_action_table.cc@509
PS1, Line 509: col_name,
: K
nit: Spacing alignment here and below.
http://gerrit.cloudera.org:8080/#/c/13833/1/src/kudu/tools/tool_action_table.cc@518
PS1, Line 518: "unable to parse value for column '$0' of type $1"
nit: maybe pull this out into a constant so we wouldn't have to change it so
many places if we wanted to?
Also how about showing the value, something like:
Substitute("unable to parse value '$0' for column '$1' of type $2", values[i],
col_name, type);
http://gerrit.cloudera.org:8080/#/c/13833/1/src/kudu/tools/tool_action_table.cc@639
PS1, Line 639: FLAGS_lower_bound_type
Should we validate this up-front that it is either "INCLUSIVE_BOUND" or
"EXCLUSIVE_BOUND" and raise an error of not?
Also, what do you think about using
[kudu-CR] Separate benchmark to single test
ZhangYao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13832 ) Change subject: Separate benchmark to single test .. Patch Set 2: (1 comment) > (1 comment) > > How slow are these tests in DEBUG mode? If I run with total 3 * 4 test, it will exceed the Jenkins unit test time limit in Debug mode. So I just seperate them and after that if someone want to extend the parameters of benchmark it can also work. http://gerrit.cloudera.org:8080/#/c/13832/1/src/kudu/common/wire_protocol-test.cc File src/kudu/common/wire_protocol-test.cc: http://gerrit.cloudera.org:8080/#/c/13832/1/src/kudu/common/wire_protocol-test.cc@63 PS1, Line 63: class WireProtocolTest : public KuduTest, > Doc up here that the int corresponds to the number of columns and the doubl Done :) -- To view, visit http://gerrit.cloudera.org:8080/13832 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I980234bcdf76a7d2978f49ed59dff21184abf892 Gerrit-Change-Number: 13832 Gerrit-PatchSet: 2 Gerrit-Owner: ZhangYao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: ZhangYao Gerrit-Comment-Date: Thu, 11 Jul 2019 02:44:12 + Gerrit-HasComments: Yes
[kudu-CR] Separate benchmark to single test
Hello Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13832 to look at the new patch set (#2). Change subject: Separate benchmark to single test .. Separate benchmark to single test Separate the total benchmark to single test with paraments so they all can be run in test time limit. Change-Id: I980234bcdf76a7d2978f49ed59dff21184abf892 --- M src/kudu/common/CMakeLists.txt M src/kudu/common/wire_protocol-test.cc 2 files changed, 14 insertions(+), 14 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/13832/2 -- To view, visit http://gerrit.cloudera.org:8080/13832 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I980234bcdf76a7d2978f49ed59dff21184abf892 Gerrit-Change-Number: 13832 Gerrit-PatchSet: 2 Gerrit-Owner: ZhangYao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2823 Place tablet replicas based on dimension
Yao Xu has posted comments on this change. ( http://gerrit.cloudera.org:8080/13632 ) Change subject: KUDU-2823 Place tablet replicas based on dimension .. Patch Set 16: (2 comments) http://gerrit.cloudera.org:8080/#/c/13632/15/src/kudu/client/client.h File src/kudu/client/client.h: http://gerrit.cloudera.org:8080/#/c/13632/15/src/kudu/client/client.h@879 PS15, Line 879: /// @note By default (or if the cluster is configured without : /// '--master_place_tablet_replicas_based_on_dimension'), the master will try to place : /// newly created tablet replicas on tablet servers with a small number of tablet replicas. : /// If the dimension label is provided, newly created replicas will be evenly distributed : /// in the cluster based on the dimension label. In other words, the master will try t > Thanks, this is useful. I'd rewrite it though: Done http://gerrit.cloudera.org:8080/#/c/13632/12/src/kudu/master/master.proto File src/kudu/master/master.proto: http://gerrit.cloudera.org:8080/#/c/13632/12/src/kudu/master/master.proto@365 PS12, Line 365: // Indicates that the tablet server needs to report the number of tablets > Still not clear on why this is necessary. Why not have tservers > always report this information? I added some docs, I hope this will be clearer. -- To view, visit http://gerrit.cloudera.org:8080/13632 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I48a225e221eb42ef2f5489687e80a151d8dc1a42 Gerrit-Change-Number: 13632 Gerrit-PatchSet: 16 Gerrit-Owner: Yao Xu 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: Yao Xu Gerrit-Comment-Date: Thu, 11 Jul 2019 02:34:17 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2823 Place tablet replicas based on dimension
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/13632 to look at the new patch set (#16). Change subject: KUDU-2823 Place tablet replicas based on dimension .. KUDU-2823 Place tablet replicas based on dimension When we add a new range to the fact table, we expect replicas of the newly created tablet to be evenly distributed across all available tablet servers. This is especially important when we use time as the range key. The more recent the data, the hotter it gets. We expect hot tablets on the cluster to be evenly distributed. Unfortunately, after we add some new tablet servers to the cluster, creating a new tablet replica will prioritize the new tablet server for placement according to the current placement policy. This is because we prefer to select tablet server which a smaller number of tablet replicas. This will cause hot tablets to be concentrated on these new tablet servers. So, I added a new placement policy. When creating a new tablet replica, we prefer to select tablet server which a smaller number of tablet replicas in the dimension. You can set dimensions when creating tables and adding partitions, this will ensure that the new tablets are evenly distributed in the cluster based on dimension. You can decide whether to use this feature by setting '--master_place_tablet_replicas_based_on_dimension'. Change-Id: I48a225e221eb42ef2f5489687e80a151d8dc1a42 --- M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/table_alterer-internal.cc M src/kudu/client/table_alterer-internal.h M src/kudu/client/table_creator-internal.h M src/kudu/integration-tests/create-table-itest.cc M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master.proto M src/kudu/master/master_service.cc M src/kudu/master/placement_policy-test.cc M src/kudu/master/placement_policy.cc M src/kudu/master/placement_policy.h M src/kudu/master/sys_catalog.cc M src/kudu/master/ts_descriptor.cc M src/kudu/master/ts_descriptor.h M src/kudu/tablet/metadata.proto M src/kudu/tablet/tablet-harness.h M src/kudu/tablet/tablet_bootstrap-test.cc M src/kudu/tablet/tablet_metadata.cc M src/kudu/tablet/tablet_metadata.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tserver/heartbeater.cc M src/kudu/tserver/mini_tablet_server.cc M src/kudu/tserver/tablet_copy_client.cc M src/kudu/tserver/tablet_server-test.cc M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/ts_tablet_manager-test.cc M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/ts_tablet_manager.h M src/kudu/tserver/tserver_admin.proto 31 files changed, 593 insertions(+), 74 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/13632/16 -- To view, visit http://gerrit.cloudera.org:8080/13632 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I48a225e221eb42ef2f5489687e80a151d8dc1a42 Gerrit-Change-Number: 13632 Gerrit-PatchSet: 16 Gerrit-Owner: Yao Xu 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: Yao Xu
[kudu-CR] [doxygen] fix KuduScanTokenBuilder::SetDiffScan (take two)
Alexey Serbin has submitted this change and it was merged. (
http://gerrit.cloudera.org:8080/13841 )
Change subject: [doxygen] fix KuduScanTokenBuilder::SetDiffScan (take two)
..
[doxygen] fix KuduScanTokenBuilder::SetDiffScan (take two)
Since both {KuduScanner,KuduScanTokenBuilder}::SetDiffScan() are both
under the same PRIVATE_API conditional, it's possible to use the
@copydoc to avoid the duplication of the description.
This is a follow-up to 0b4948ea3.
Change-Id: I2de45bf62bd8ef240b0d0e0cd3f887ef3d40cd04
Reviewed-on: http://gerrit.cloudera.org:8080/13841
Tested-by: Kudu Jenkins
Reviewed-by: Grant Henke
---
M src/kudu/client/client.h
1 file changed, 1 insertion(+), 16 deletions(-)
Approvals:
Kudu Jenkins: Verified
Grant Henke: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/13841
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I2de45bf62bd8ef240b0d0e0cd3f887ef3d40cd04
Gerrit-Change-Number: 13841
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [doxygen] fix KuduScanTokenBuilder::SetDiffScan (take two)
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13841 ) Change subject: [doxygen] fix KuduScanTokenBuilder::SetDiffScan (take two) .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13841 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2de45bf62bd8ef240b0d0e0cd3f887ef3d40cd04 Gerrit-Change-Number: 13841 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 11 Jul 2019 01:06:48 + Gerrit-HasComments: No
[kudu-CR] [doxygen] fix KuduScanTokenBuilder::SetDiffScan (take two)
Alexey Serbin has uploaded this change for review. (
http://gerrit.cloudera.org:8080/13841
Change subject: [doxygen] fix KuduScanTokenBuilder::SetDiffScan (take two)
..
[doxygen] fix KuduScanTokenBuilder::SetDiffScan (take two)
Since both {KuduScanner,KuduScanTokenBuilder}::SetDiffScan() are both
under the same PRIVATE_API conditional, it's possible to use the
@copydoc to avoid the duplication of the description.
This is a follow-up to 0b4948ea3.
Change-Id: I2de45bf62bd8ef240b0d0e0cd3f887ef3d40cd04
---
M src/kudu/client/client.h
1 file changed, 1 insertion(+), 16 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/13841/1
--
To view, visit http://gerrit.cloudera.org:8080/13841
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2de45bf62bd8ef240b0d0e0cd3f887ef3d40cd04
Gerrit-Change-Number: 13841
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin
[kudu-CR] Fix KuduScanTokenBuilder::SetDiffScan docs
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/13837 ) Change subject: Fix KuduScanTokenBuilder::SetDiffScan docs .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/13837/2/src/kudu/client/client.h File src/kudu/client/client.h: http://gerrit.cloudera.org:8080/#/c/13837/2/src/kudu/client/client.h@2436 PS2, Line 2436: /// Set the start and end timestamp for a diff scan. The timestamps should be : /// encoded HT timestamps. : /// : /// Additionally sets any other scan properties required by diff scans. : /// : /// Private API. : /// : /// @param [in] start_timestamp : /// Start timestamp to set in raw encoded form : /// (i.e. as returned by a previous call to a server). : /// @param [in] end_timestamp : /// End timestamp to set in raw encoded form : /// (i.e. as returned by a previous call to a server). : /// @return Operation result status. nit: given this update puts the KuduScanTokenBuilder::SetDiffScan into the priviate API section, the easier way would be just adding the @cond/@endcond since the source of the reference is under the same conditional. I'll put up a patch to address this. -- To view, visit http://gerrit.cloudera.org:8080/13837 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I448c00b78f9d14685e87d154e1bbee0eaa8e8e24 Gerrit-Change-Number: 13837 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 10 Jul 2019 23:48:24 + Gerrit-HasComments: Yes
[kudu-CR] [docs] update the upgrade documentation
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13820 ) Change subject: [docs] update the upgrade documentation .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/13820/2/docs/installation.adoc File docs/installation.adoc: http://gerrit.cloudera.org:8080/#/c/13820/2/docs/installation.adoc@647 PS2, Line 647: and the gflag above should be reset after every reboot. > The 'reset' here means setting to 7200. Gotcha. Can you add this to the documentation? There's another catch: depending on the size of the deployment, it could take some time until the tserver starts responding to RPCs. For example, I think the block manager has to fully load first, and the tserver has to finish waiting for NTP to synchronize (if it's not already synchronized). -- To view, visit http://gerrit.cloudera.org:8080/13820 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6b3e5c549dc05c3388c0b0dd628d205a356da344 Gerrit-Change-Number: 13820 Gerrit-PatchSet: 2 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Priyanka Chheda Gerrit-Reviewer: helifu Gerrit-Comment-Date: Wed, 10 Jul 2019 23:00:39 + Gerrit-HasComments: Yes
[kudu-CR] [docs] update the upgrade documentation
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/13820 ) Change subject: [docs] update the upgrade documentation .. Patch Set 2: (9 comments) http://gerrit.cloudera.org:8080/#/c/13820/2/docs/installation.adoc File docs/installation.adoc: http://gerrit.cloudera.org:8080/#/c/13820/2/docs/installation.adoc@635 PS2, Line 635: for the scenario of compiling from source code. > "relevant when building from source code". Done http://gerrit.cloudera.org:8080/#/c/13820/2/docs/installation.adoc@640 PS2, Line 640: 2hours > "2 hours" Done http://gerrit.cloudera.org:8080/#/c/13820/2/docs/installation.adoc@640 PS2, Line 640: kudu-tserver > "tablet server" Done http://gerrit.cloudera.org:8080/#/c/13820/2/docs/installation.adoc@645 PS2, Line 645: --force > Shouldn't need this; the flag is tagged as 'runtime'. Ah, Grant has tagged the follower_unavailable_considered_failed_sec flag as runtime. http://gerrit.cloudera.org:8080/#/c/13820/2/docs/installation.adoc@647 PS2, Line 647: kudu-tserver > "the tablet servers" Done http://gerrit.cloudera.org:8080/#/c/13820/2/docs/installation.adoc@647 PS2, Line 647: and the gflag above should be reset after every reboot. > Since step 4 restores the gflag value, do we really need this step? The 'reset' here means setting to 7200. Let me describe it more clearly. If there are 3 tablet servers A, B, C: 1.Set the unavailable time for every kudu-tserver to a large value: ./kudu tserver set_flag A follower_unavailable_considered_failed_sec 7200 ./kudu tserver set_flag B follower_unavailable_considered_failed_sec 7200 ./kudu tserver set_flag C follower_unavailable_considered_failed_sec 7200 2.Rolling restart kudu-tserver and the gflag above should be reset after every reboot: restart A ./kudu tserver set_flag A follower_unavailable_considered_failed_sec 7200 restart B ./kudu tserver set_flag B follower_unavailable_considered_failed_sec 7200 restart C ./kudu tserver set_flag C follower_unavailable_considered_failed_sec 7200 3.Restore the default gflag value (5 minutes) for every kudu-tserver: ./kudu tserver set_flag A follower_unavailable_considered_failed_sec 300 ./kudu tserver set_flag B follower_unavailable_considered_failed_sec 300 ./kudu tserver set_flag C follower_unavailable_considered_failed_sec 300 http://gerrit.cloudera.org:8080/#/c/13820/2/docs/installation.adoc@649 PS2, Line 649: [NOTE] > Shouldn't this be just after "Set the unavailable time..." so users underst The note is used to emphasize the 'reset' is very important, not the first time set. http://gerrit.cloudera.org:8080/#/c/13820/2/docs/installation.adoc@651 PS2, Line 651: If the gflag is not reset, kudu-tserver which is restarting would be possibly evicted from the cluster. > "If the unavailable time is not extended, restarted tablet servers could be Done http://gerrit.cloudera.org:8080/#/c/13820/2/docs/installation.adoc@654 PS2, Line 654: kudu-master > "the masters" Done -- To view, visit http://gerrit.cloudera.org:8080/13820 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6b3e5c549dc05c3388c0b0dd628d205a356da344 Gerrit-Change-Number: 13820 Gerrit-PatchSet: 2 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Priyanka Chheda Gerrit-Reviewer: helifu Gerrit-Comment-Date: Wed, 10 Jul 2019 22:48:20 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2855 Lazy-create DeltaMemStore on first update
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/13821 ) Change subject: KUDU-2855 Lazy-create DeltaMemStore on first update .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.h File src/kudu/tablet/delta_tracker.h: http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.h@336 PS2, Line 336: const scoped_refptr log_anchor_registry_; > Those test-only cases look like lazy programming. Let's fix them, and then No, I will fix them. ^_^ By the way, a raw pointer here has a potential risk for the next users or test cases if he or she doesn't know how log_anchor_registry_ works. -- To view, visit http://gerrit.cloudera.org:8080/13821 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie0c565d86647d5144266b30aa6e8572d42db48c6 Gerrit-Change-Number: 13821 Gerrit-PatchSet: 3 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu Gerrit-Comment-Date: Wed, 10 Jul 2019 22:27:17 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2855 Lazy-create DeltaMemStore on first update
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13821 ) Change subject: KUDU-2855 Lazy-create DeltaMemStore on first update .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.h File src/kudu/tablet/delta_tracker.h: http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.h@336 PS2, Line 336: const scoped_refptr log_anchor_registry_; > Yes, when log_anchor_registry_ was a raw pointer, it pointed to the scoped_ Those test-only cases look like lazy programming. Let's fix them, and then a raw pointer should be 100% safe here? -- To view, visit http://gerrit.cloudera.org:8080/13821 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie0c565d86647d5144266b30aa6e8572d42db48c6 Gerrit-Change-Number: 13821 Gerrit-PatchSet: 3 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu Gerrit-Comment-Date: Wed, 10 Jul 2019 22:17:26 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2855 Lazy-create DeltaMemStore on first update
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/13821 ) Change subject: KUDU-2855 Lazy-create DeltaMemStore on first update .. Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/13821/3/src/kudu/tablet/delta_tracker.h File src/kudu/tablet/delta_tracker.h: http://gerrit.cloudera.org:8080/#/c/13821/3/src/kudu/tablet/delta_tracker.h@374 PS3, Line 374: // Number of deleted rows for a DMS that is currently being flushed. > Nit: what changed here? There is a ^M at the end. http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.h File src/kudu/tablet/delta_tracker.h: http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.h@336 PS2, Line 336: const scoped_refptr log_anchor_registry_; > I don't understand your explanation. When log_anchor_registry_ was a raw po Yes, when log_anchor_registry_ was a raw pointer, it pointed to the scoped_refptr owned by Tablet. But, in test cases, this raw pointer is not very safe to use: https://github.com/apache/kudu/blob/3cbc0d4fbe295748d6ffdf1e5e7edeaf94ef0911/src/kudu/tablet/diskrowset-test-base.h#L330 https://github.com/apache/kudu/blob/09e089bfafb9a1fa2099bd43cd0bd786dad0e771/src/kudu/tablet/diskrowset-test.cc#L759 http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.cc File src/kudu/tablet/delta_tracker.cc: http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.cc@78 PS2, Line 78: DEFINE_bool(dms_lazy_create, true, : "Allow lazily creating of DeltaMemStore"); : TAG_FLAG(dms_lazy_create, advanced > I'd discard it; it doesn't seem important enough to expose. ok. -- To view, visit http://gerrit.cloudera.org:8080/13821 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie0c565d86647d5144266b30aa6e8572d42db48c6 Gerrit-Change-Number: 13821 Gerrit-PatchSet: 3 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu Gerrit-Comment-Date: Wed, 10 Jul 2019 22:05:02 + Gerrit-HasComments: Yes
[kudu-CR] [docs] Update the site build to use the release directory
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13835 ) Change subject: [docs] Update the site build to use the release directory .. [docs] Update the site build to use the release directory Updates the make_site.sh script and make_docs.sh script to generate docs in the versioned release directory. This matches the new symbolic linking style we have been using for the docs and avoids the need to manually copy the docs during release. An additional flag to skip updating the symbolic links (`—no-link`) was added. Change-Id: I174de2d7041bba20cdc3dcacd3efbb5e736ecb63 Reviewed-on: http://gerrit.cloudera.org:8080/13835 Reviewed-by: Andrew Wong Tested-by: Andrew Wong --- M docs/support/scripts/make_docs.sh M docs/support/scripts/make_site.sh 2 files changed, 54 insertions(+), 11 deletions(-) Approvals: Andrew Wong: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/13835 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I174de2d7041bba20cdc3dcacd3efbb5e736ecb63 Gerrit-Change-Number: 13835 Gerrit-PatchSet: 4 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR](branch-1.10.x) Fix KuduScanTokenBuilder::SetDiffScan docs
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13839 Change subject: Fix KuduScanTokenBuilder::SetDiffScan docs .. Fix KuduScanTokenBuilder::SetDiffScan docs It looks like Doxygen can’t use `@copydoc` when the doc being copied is in a `@cond PRIVATE_API`. Change-Id: I448c00b78f9d14685e87d154e1bbee0eaa8e8e24 Reviewed-on: http://gerrit.cloudera.org:8080/13837 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo (cherry picked from commit 0b4948ea3beac43a0dcd2ae2933440d089e337fd) --- M src/kudu/client/client.h 1 file changed, 18 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/39/13839/1 -- To view, visit http://gerrit.cloudera.org:8080/13839 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.10.x Gerrit-MessageType: newchange Gerrit-Change-Id: I448c00b78f9d14685e87d154e1bbee0eaa8e8e24 Gerrit-Change-Number: 13839 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke
[kudu-CR](branch-1.10.x) [docs] Update the site build to use the release directory
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13840 Change subject: [docs] Update the site build to use the release directory .. [docs] Update the site build to use the release directory Updates the make_site.sh script and make_docs.sh script to generate docs in the versioned release directory. This matches the new symbolic linking style we have been using for the docs and avoids the need to manually copy the docs during release. An additional flag to skip updating the symbolic links (`—no-link`) was added. Change-Id: I174de2d7041bba20cdc3dcacd3efbb5e736ecb63 Reviewed-on: http://gerrit.cloudera.org:8080/13835 Reviewed-by: Andrew Wong Tested-by: Andrew Wong (cherry picked from commit 819a85fbb1d836f70a50c136dfcdaebc37faf7a8) --- M docs/support/scripts/make_docs.sh M docs/support/scripts/make_site.sh 2 files changed, 54 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/13840/1 -- To view, visit http://gerrit.cloudera.org:8080/13840 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.10.x Gerrit-MessageType: newchange Gerrit-Change-Id: I174de2d7041bba20cdc3dcacd3efbb5e736ecb63 Gerrit-Change-Number: 13840 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke
[kudu-CR] Fix KuduScanTokenBuilder::SetDiffScan docs
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13837 ) Change subject: Fix KuduScanTokenBuilder::SetDiffScan docs .. Fix KuduScanTokenBuilder::SetDiffScan docs It looks like Doxygen can’t use `@copydoc` when the doc being copied is in a `@cond PRIVATE_API`. Change-Id: I448c00b78f9d14685e87d154e1bbee0eaa8e8e24 Reviewed-on: http://gerrit.cloudera.org:8080/13837 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo --- M src/kudu/client/client.h 1 file changed, 18 insertions(+), 1 deletion(-) Approvals: Kudu Jenkins: Verified Adar Dembo: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/13837 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I448c00b78f9d14685e87d154e1bbee0eaa8e8e24 Gerrit-Change-Number: 13837 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [docs] Update the site build to use the release directory
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13835 ) Change subject: [docs] Update the site build to use the release directory .. Patch Set 3: Verified+1 Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13835 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I174de2d7041bba20cdc3dcacd3efbb5e736ecb63 Gerrit-Change-Number: 13835 Gerrit-PatchSet: 3 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 10 Jul 2019 21:42:07 + Gerrit-HasComments: No
[kudu-CR] [docs] Update the site build to use the release directory
Andrew Wong has removed a vote on this change. Change subject: [docs] Update the site build to use the release directory .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/13835 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I174de2d7041bba20cdc3dcacd3efbb5e736ecb63 Gerrit-Change-Number: 13835 Gerrit-PatchSet: 3 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] Fix KuduScanTokenBuilder::SetDiffScan docs
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13837 ) Change subject: Fix KuduScanTokenBuilder::SetDiffScan docs .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13837 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I448c00b78f9d14685e87d154e1bbee0eaa8e8e24 Gerrit-Change-Number: 13837 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 10 Jul 2019 21:29:40 + Gerrit-HasComments: No
[kudu-CR](branch-1.10.x) Remove experimental tags from NVM block cache gflags
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13838 Change subject: Remove experimental tags from NVM block cache gflags .. Remove experimental tags from NVM block cache gflags The block cache has supported non-volatile memory for almost four years now, though one needed to configure some experimental gflags in order to use it. The implementation was recently changed to use the newer memkind library and, in the process, underwent additional testing on NVM hardware. As such, I think it's time to remove the experimental tags to communicate to users that the underlying code is supported upstream. Note: I'm not marking them as 'stable' so that we retain the flexibility to modify the configuration surface itself, if we find we need that. Change-Id: Ie6b58118116ecc11b2eaa5cec4a9e72c29ac27f8 Reviewed-on: http://gerrit.cloudera.org:8080/13830 Reviewed-by: Greg Solovyev Reviewed-by: Andrew Wong Tested-by: Kudu Jenkins (cherry picked from commit daf2572ad107bc50bb8a4e534a5e1414aa71bbe6) --- M src/kudu/cfile/block_cache.cc M src/kudu/util/nvm_cache.cc 2 files changed, 1 insertion(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/38/13838/1 -- To view, visit http://gerrit.cloudera.org:8080/13838 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.10.x Gerrit-MessageType: newchange Gerrit-Change-Id: Ie6b58118116ecc11b2eaa5cec4a9e72c29ac27f8 Gerrit-Change-Number: 13838 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo
[kudu-CR] [docs] Update the site build to use the release directory
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13835 ) Change subject: [docs] Update the site build to use the release directory .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/13835/1/src/kudu/client/client.h File src/kudu/client/client.h: PS1: > I see. Yeah, I'd prefer breaking it out, as trivial as it is. Kind of surpr I broke it out. -- To view, visit http://gerrit.cloudera.org:8080/13835 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I174de2d7041bba20cdc3dcacd3efbb5e736ecb63 Gerrit-Change-Number: 13835 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 10 Jul 2019 21:06:23 + Gerrit-HasComments: Yes
[kudu-CR] Fix KuduScanTokenBuilder::SetDiffScan docs
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13837 Change subject: Fix KuduScanTokenBuilder::SetDiffScan docs .. Fix KuduScanTokenBuilder::SetDiffScan docs It looks like Doxygen can’t use `@copydoc` when the doc being copied is in a `@cond PRIVATE_API`. Change-Id: I448c00b78f9d14685e87d154e1bbee0eaa8e8e24 --- M src/kudu/client/client.h 1 file changed, 18 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/37/13837/1 -- To view, visit http://gerrit.cloudera.org:8080/13837 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I448c00b78f9d14685e87d154e1bbee0eaa8e8e24 Gerrit-Change-Number: 13837 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke
[kudu-CR] [docs] Update the site build to use the release directory
Hello Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13835 to look at the new patch set (#3). Change subject: [docs] Update the site build to use the release directory .. [docs] Update the site build to use the release directory Updates the make_site.sh script and make_docs.sh script to generate docs in the versioned release directory. This matches the new symbolic linking style we have been using for the docs and avoids the need to manually copy the docs during release. An additional flag to skip updating the symbolic links (`—no-link`) was added. Change-Id: I174de2d7041bba20cdc3dcacd3efbb5e736ecb63 --- M docs/support/scripts/make_docs.sh M docs/support/scripts/make_site.sh 2 files changed, 54 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/35/13835/3 -- To view, visit http://gerrit.cloudera.org:8080/13835 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I174de2d7041bba20cdc3dcacd3efbb5e736ecb63 Gerrit-Change-Number: 13835 Gerrit-PatchSet: 3 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR](branch-1.10.x) [web] add HMS docs to the side menu
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13836 ) Change subject: [web] add HMS docs to the side menu .. [web] add HMS docs to the side menu Change-Id: Ic3aaf36954ca0b63f04f5923b93cddd03813337a Reviewed-on: http://gerrit.cloudera.org:8080/13831 Tested-by: Kudu Jenkins Reviewed-by: Grant Henke (cherry picked from commit b72d6448a15a518af6d271d95067647df8d0cd95) Reviewed-on: http://gerrit.cloudera.org:8080/13836 --- M docs/support/jekyll-templates/document.html.erb 1 file changed, 1 insertion(+), 0 deletions(-) Approvals: Grant Henke: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/13836 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.10.x Gerrit-MessageType: merged Gerrit-Change-Id: Ic3aaf36954ca0b63f04f5923b93cddd03813337a Gerrit-Change-Number: 13836 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [docs] Update the site build to use the release directory
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13835 ) Change subject: [docs] Update the site build to use the release directory .. Patch Set 2: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/13835/1/src/kudu/client/client.h File src/kudu/client/client.h: PS1: > Doxygen was failing on this because it couldn't generate docs for SetDiffSc I see. Yeah, I'd prefer breaking it out, as trivial as it is. Kind of surprising to find it here, even if it's noted in the body of the commit message. -- To view, visit http://gerrit.cloudera.org:8080/13835 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I174de2d7041bba20cdc3dcacd3efbb5e736ecb63 Gerrit-Change-Number: 13835 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 10 Jul 2019 19:41:22 + Gerrit-HasComments: Yes
[kudu-CR](branch-1.10.x) [docs] Make the quickstart link more obvious
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13834 ) Change subject: [docs] Make the quickstart link more obvious .. [docs] Make the quickstart link more obvious Adjusts the sidebar to make finding the quickstart more straightforward. Currently it’s not clear the “Getting Started” link means quickstart. Change-Id: Ica5831dd0871be4e146777fdc1a178cc397d9593 Reviewed-on: http://gerrit.cloudera.org:8080/13828 Tested-by: Kudu Jenkins Reviewed-by: Andrew Wong Reviewed-by: Greg Solovyev (cherry picked from commit 0892c19425bd1a9e3eb293d93a43e1ce38dc3d2d) Reviewed-on: http://gerrit.cloudera.org:8080/13834 --- M docs/support/jekyll-templates/document.html.erb 1 file changed, 1 insertion(+), 2 deletions(-) Approvals: Kudu Jenkins: Verified Andrew Wong: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/13834 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.10.x Gerrit-MessageType: merged Gerrit-Change-Id: Ica5831dd0871be4e146777fdc1a178cc397d9593 Gerrit-Change-Number: 13834 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR](branch-1.10.x) [web] add HMS docs to the side menu
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13836 ) Change subject: [web] add HMS docs to the side menu .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13836 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.10.x Gerrit-MessageType: comment Gerrit-Change-Id: Ic3aaf36954ca0b63f04f5923b93cddd03813337a Gerrit-Change-Number: 13836 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 10 Jul 2019 19:22:32 + Gerrit-HasComments: No
[kudu-CR] [docs] Update the site build to use the release directory
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13835 ) Change subject: [docs] Update the site build to use the release directory .. Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/13835/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13835/1//COMMIT_MSG@13 PS1, Line 13: during > nit: during Done http://gerrit.cloudera.org:8080/#/c/13835/1/docs/support/scripts/make_docs.sh File docs/support/scripts/make_docs.sh: http://gerrit.cloudera.org:8080/#/c/13835/1/docs/support/scripts/make_docs.sh@34 PS1, Line 34: echo usage: "$0 --build_root --output_subdir [--site Can you update this too? Done http://gerrit.cloudera.org:8080/#/c/13835/1/src/kudu/client/client.h File src/kudu/client/client.h: PS1: > I'm a little confused. Is this at all related to the other changes? Doxygen was failing on this because it couldn't generate docs for SetDiffScan. I included it as a commit note. I can break it out if you want. It just seemed to be trivial given it was a copy/paste. -- To view, visit http://gerrit.cloudera.org:8080/13835 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I174de2d7041bba20cdc3dcacd3efbb5e736ecb63 Gerrit-Change-Number: 13835 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 10 Jul 2019 19:21:49 + Gerrit-HasComments: Yes
[kudu-CR] [docs] Update the site build to use the release directory
Hello Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13835 to look at the new patch set (#2). Change subject: [docs] Update the site build to use the release directory .. [docs] Update the site build to use the release directory Updates the make_site.sh script and make_docs.sh script to generate docs in the versioned release directory. This matches the new symbolic linking style we have been using for the docs and avoids the need to manually copy the docs during release. An additional flag to skip updating the symbolic links (`—no-link`) was added. Additionally included a Doxygen fix for `SetDiffScan`. It looks like Doxygen can’t use `@copydoc` when the doc being copied is in a `@cond PRIVATE_API`. Change-Id: I174de2d7041bba20cdc3dcacd3efbb5e736ecb63 --- M docs/support/scripts/make_docs.sh M docs/support/scripts/make_site.sh M src/kudu/client/client.h 3 files changed, 72 insertions(+), 12 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/35/13835/2 -- To view, visit http://gerrit.cloudera.org:8080/13835 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I174de2d7041bba20cdc3dcacd3efbb5e736ecb63 Gerrit-Change-Number: 13835 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [web] add HMS docs to the side menu
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13831 ) Change subject: [web] add HMS docs to the side menu .. [web] add HMS docs to the side menu Change-Id: Ic3aaf36954ca0b63f04f5923b93cddd03813337a Reviewed-on: http://gerrit.cloudera.org:8080/13831 Tested-by: Kudu Jenkins Reviewed-by: Grant Henke --- M docs/support/jekyll-templates/document.html.erb 1 file changed, 1 insertion(+), 0 deletions(-) Approvals: Kudu Jenkins: Verified Grant Henke: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/13831 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ic3aaf36954ca0b63f04f5923b93cddd03813337a Gerrit-Change-Number: 13831 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR](branch-1.10.x) [web] add HMS docs to the side menu
Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13836 Change subject: [web] add HMS docs to the side menu .. [web] add HMS docs to the side menu Change-Id: Ic3aaf36954ca0b63f04f5923b93cddd03813337a Reviewed-on: http://gerrit.cloudera.org:8080/13831 Tested-by: Kudu Jenkins Reviewed-by: Grant Henke (cherry picked from commit b72d6448a15a518af6d271d95067647df8d0cd95) --- M docs/support/jekyll-templates/document.html.erb 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/13836/1 -- To view, visit http://gerrit.cloudera.org:8080/13836 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.10.x Gerrit-MessageType: newchange Gerrit-Change-Id: Ic3aaf36954ca0b63f04f5923b93cddd03813337a Gerrit-Change-Number: 13836 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong
[kudu-CR] [web] add HMS docs to the side menu
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13831 ) Change subject: [web] add HMS docs to the side menu .. Patch Set 1: > Patch Set 1: Code-Review+2 > > I am a +2 on this. A side thought is that we may want to break out an > "integrations" section of the site and move some content there. Or something > similar. Our side bar is fairly long. I agree, but I haven't come to a conclusion of how I'd rather organize it yet. "Integrations" is good to showcase the ecosystem, but functionally it's awkward since the HMS isn't a read/write integration like our other integrations are. Maybe that's fine though. -- To view, visit http://gerrit.cloudera.org:8080/13831 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic3aaf36954ca0b63f04f5923b93cddd03813337a Gerrit-Change-Number: 13831 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 10 Jul 2019 19:19:06 + Gerrit-HasComments: No
[kudu-CR] [docs] Update the site build to use the release directory
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13835 ) Change subject: [docs] Update the site build to use the release directory .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/13835/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13835/1//COMMIT_MSG@13 PS1, Line 13: durring nit: during http://gerrit.cloudera.org:8080/#/c/13835/1/docs/support/scripts/make_docs.sh File docs/support/scripts/make_docs.sh: http://gerrit.cloudera.org:8080/#/c/13835/1/docs/support/scripts/make_docs.sh@34 PS1, Line 34: echo usage: "$0 --build_root [--site ]" Can you update this too? http://gerrit.cloudera.org:8080/#/c/13835/1/src/kudu/client/client.h File src/kudu/client/client.h: PS1: I'm a little confused. Is this at all related to the other changes? -- To view, visit http://gerrit.cloudera.org:8080/13835 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I174de2d7041bba20cdc3dcacd3efbb5e736ecb63 Gerrit-Change-Number: 13835 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 10 Jul 2019 19:16:02 + Gerrit-HasComments: Yes
[kudu-CR](branch-1.10.x) [docs] Make the quickstart link more obvious
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13834 ) Change subject: [docs] Make the quickstart link more obvious .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13834 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.10.x Gerrit-MessageType: comment Gerrit-Change-Id: Ica5831dd0871be4e146777fdc1a178cc397d9593 Gerrit-Change-Number: 13834 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 10 Jul 2019 19:16:12 + Gerrit-HasComments: No
[kudu-CR] [docs] Update the site build to use the release directory
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13835 Change subject: [docs] Update the site build to use the release directory .. [docs] Update the site build to use the release directory Updates the make_site.sh script and make_docs.sh script to generate docs in the versioned release directory. This matches the new symbolic linking style we have been using for the docs and avoids the need to manually copy the docs durring release. An additional flag to skip updating the symbolic links (`—no-link`) was added. Additionally included a Doxygen fix for `SetDiffScan`. It looks like Doxygen can’t use `@copydoc` when the doc being copied is in a `@cond PRIVATE_API`. Change-Id: I174de2d7041bba20cdc3dcacd3efbb5e736ecb63 --- M docs/support/scripts/make_docs.sh M docs/support/scripts/make_site.sh M src/kudu/client/client.h 3 files changed, 71 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/35/13835/1 -- To view, visit http://gerrit.cloudera.org:8080/13835 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I174de2d7041bba20cdc3dcacd3efbb5e736ecb63 Gerrit-Change-Number: 13835 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke
[kudu-CR] KUDU-2722 (table level): Support new 'write enabled' table extra config
Hello Tidy Bot, Yingchun Lai, Kudu Jenkins, Yao Xu, Adar Dembo, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13796 to look at the new patch set (#6). Change subject: KUDU-2722 (table level): Support new 'write_enabled' table extra config .. KUDU-2722 (table level): Support new 'write_enabled' table extra config Change-Id: Ibd3768eda36d9574be9c41e7cd3dd81cc5ae8f3a --- M src/kudu/client/client-test.cc M src/kudu/common/common.proto M src/kudu/common/wire_protocol.cc M src/kudu/tablet/tablet_metadata-test.cc M src/kudu/tools/kudu-admin-test.cc M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/ts_tablet_manager-test.cc M src/kudu/tserver/tserver.proto 8 files changed, 128 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/96/13796/6 -- To view, visit http://gerrit.cloudera.org:8080/13796 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibd3768eda36d9574be9c41e7cd3dd81cc5ae8f3a Gerrit-Change-Number: 13796 Gerrit-PatchSet: 6 Gerrit-Owner: XiaokaiWang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: XiaokaiWang Gerrit-Reviewer: Yao Xu Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
[kudu-CR] KUDU-2722 (table level): Support new 'write enabled' table extra config
Hello Tidy Bot, Yingchun Lai, Kudu Jenkins, Yao Xu, Adar Dembo, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13796 to look at the new patch set (#5). Change subject: KUDU-2722 (table level): Support new 'write_enabled' table extra config .. KUDU-2722 (table level): Support new 'write_enabled' table extra config Change-Id: Ibd3768eda36d9574be9c41e7cd3dd81cc5ae8f3a --- M src/kudu/client/client-test.cc M src/kudu/common/common.proto M src/kudu/common/wire_protocol.cc M src/kudu/tablet/tablet_metadata-test.cc M src/kudu/tools/kudu-admin-test.cc M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/ts_tablet_manager-test.cc M src/kudu/tserver/tserver.proto 8 files changed, 128 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/96/13796/5 -- To view, visit http://gerrit.cloudera.org:8080/13796 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibd3768eda36d9574be9c41e7cd3dd81cc5ae8f3a Gerrit-Change-Number: 13796 Gerrit-PatchSet: 5 Gerrit-Owner: XiaokaiWang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: XiaokaiWang Gerrit-Reviewer: Yao Xu Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
[kudu-CR] KUDU-2722 (table level): Support new 'write enabled' table extra config
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/13796 )
Change subject: KUDU-2722 (table level): Support new 'write_enabled' table
extra config
..
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/13796/2/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:
http://gerrit.cloudera.org:8080/#/c/13796/2/src/kudu/tserver/tablet_service.cc@1083
PS2, Line 1083: bool TabletIsWritable(shared_ptr tablet) {
> warning: the parameter 'tablet' is copied for each invocation but only used
This is also a good suggestion that you should adopt.
http://gerrit.cloudera.org:8080/#/c/13796/2/src/kudu/tserver/tablet_service.cc@1084
PS2, Line 1084: boost::optional extra_config =
tablet->metadata()->extra_config();
> This makes a copy, so consider storing a cref:
Looks like you missed this.
--
To view, visit http://gerrit.cloudera.org:8080/13796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd3768eda36d9574be9c41e7cd3dd81cc5ae8f3a
Gerrit-Change-Number: 13796
Gerrit-PatchSet: 4
Gerrit-Owner: XiaokaiWang
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: XiaokaiWang
Gerrit-Reviewer: Yao Xu
Gerrit-Comment-Date: Wed, 10 Jul 2019 16:21:16 +
Gerrit-HasComments: Yes
[kudu-CR] [maintenance] Add extra config for maintenance manager task priority
XiaokaiWang has posted comments on this change. ( http://gerrit.cloudera.org:8080/13659 ) Change subject: [maintenance] Add extra config for maintenance manager task priority .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/13659/2/src/kudu/common/common.proto File src/kudu/common/common.proto: PS2: > Do you think we should update all of the tests that xiaokai updated (https: Do we need to confirm that it works on all respects? I think it is needed. -- To view, visit http://gerrit.cloudera.org:8080/13659 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I966ee626ef85ce56ba4517b9e494f3ac5b044867 Gerrit-Change-Number: 13659 Gerrit-PatchSet: 2 Gerrit-Owner: Yingchun Lai <405403...@qq.com> Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Will Berkeley Gerrit-Reviewer: XiaokaiWang Gerrit-Reviewer: Yao Xu Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Comment-Date: Wed, 10 Jul 2019 15:59:32 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2855 Lazy-create DeltaMemStore on first update
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13821 ) Change subject: KUDU-2855 Lazy-create DeltaMemStore on first update .. Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/13821/3/src/kudu/tablet/delta_tracker.h File src/kudu/tablet/delta_tracker.h: http://gerrit.cloudera.org:8080/#/c/13821/3/src/kudu/tablet/delta_tracker.h@374 PS3, Line 374: // Number of deleted rows for a DMS that is currently being flushed. Nit: what changed here? http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.h File src/kudu/tablet/delta_tracker.h: http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.h@336 PS2, Line 336: const scoped_refptr log_anchor_registry_; > The "log_anchor_registry_" will be a wild pointer if we use a raw point her I don't understand your explanation. When log_anchor_registry_ was a raw pointer, it pointed to the scoped_refptr owned by Tablet. That's safe because we can't destroy Tablet without first destroying all of its DiskRowSets and DeltaTrackers. This is a fundamental tablet lifecycle property that doesn't change even when DMSes are lazily constructed. It's frustrating that LogAnchorRegistry isn't treated consistently as a shared object, but I think that's partly done for performance reasons. When Foo has a scoped_refptr instead of a Bar*, Bar's reference count is incremented when Foo is constructed and decremented when Foo is destructed. For LogAnchorRegistry, we know that a Tablet will outlive all of its sub-objects, and because there can be many of those sub-objects (i.e. a handful per rowset), storing LogAnchorRegistry* in them means avoiding excessive increments and decrements without sacrificing safety. http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.cc File src/kudu/tablet/delta_tracker.cc: http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.cc@78 PS2, Line 78: DEFINE_bool(dms_lazy_create, true, : "Allow lazily creating of DeltaMemStore"); : TAG_FLAG(dms_lazy_create, advanced > Should I keep this gflag or just discard it? If yes, maybe some test cases I'd discard it; it doesn't seem important enough to expose. -- To view, visit http://gerrit.cloudera.org:8080/13821 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie0c565d86647d5144266b30aa6e8572d42db48c6 Gerrit-Change-Number: 13821 Gerrit-PatchSet: 3 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu Gerrit-Comment-Date: Wed, 10 Jul 2019 15:58:18 + Gerrit-HasComments: Yes
[kudu-CR] [web] add HMS docs to the side menu
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13831 ) Change subject: [web] add HMS docs to the side menu .. Patch Set 1: Code-Review+2 I am a +2 on this. A side thought is that we may want to break out an "integrations" section of the site and move some content there. Or something similar. Our side bar is fairly long. -- To view, visit http://gerrit.cloudera.org:8080/13831 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic3aaf36954ca0b63f04f5923b93cddd03813337a Gerrit-Change-Number: 13831 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 10 Jul 2019 15:56:38 + Gerrit-HasComments: No
[kudu-CR] KUDU-2722 (table level): Support new 'write enabled' table extra config
Hello Tidy Bot, Kudu Jenkins, Yao Xu, Adar Dembo, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13796 to look at the new patch set (#4). Change subject: KUDU-2722 (table level): Support new 'write_enabled' table extra config .. KUDU-2722 (table level): Support new 'write_enabled' table extra config Change-Id: Ibd3768eda36d9574be9c41e7cd3dd81cc5ae8f3a --- M src/kudu/client/client-test.cc M src/kudu/common/common.proto M src/kudu/common/wire_protocol.cc M src/kudu/tablet/tablet_metadata-test.cc M src/kudu/tools/kudu-admin-test.cc M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/ts_tablet_manager-test.cc M src/kudu/tserver/tserver.proto 8 files changed, 128 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/96/13796/4 -- To view, visit http://gerrit.cloudera.org:8080/13796 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibd3768eda36d9574be9c41e7cd3dd81cc5ae8f3a Gerrit-Change-Number: 13796 Gerrit-PatchSet: 4 Gerrit-Owner: XiaokaiWang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: XiaokaiWang Gerrit-Reviewer: Yao Xu
[kudu-CR](branch-1.10.x) [docs] Make the quickstart link more obvious
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13834 Change subject: [docs] Make the quickstart link more obvious .. [docs] Make the quickstart link more obvious Adjusts the sidebar to make finding the quickstart more straightforward. Currently it’s not clear the “Getting Started” link means quickstart. Change-Id: Ica5831dd0871be4e146777fdc1a178cc397d9593 Reviewed-on: http://gerrit.cloudera.org:8080/13828 Tested-by: Kudu Jenkins Reviewed-by: Andrew Wong Reviewed-by: Greg Solovyev (cherry picked from commit 0892c19425bd1a9e3eb293d93a43e1ce38dc3d2d) --- M docs/support/jekyll-templates/document.html.erb 1 file changed, 1 insertion(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/34/13834/1 -- To view, visit http://gerrit.cloudera.org:8080/13834 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.10.x Gerrit-MessageType: newchange Gerrit-Change-Id: Ica5831dd0871be4e146777fdc1a178cc397d9593 Gerrit-Change-Number: 13834 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke
[kudu-CR] KUDU-2722 (table level): Support new 'write enabled' table extra config
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13796 ) Change subject: KUDU-2722 (table level): Support new 'write_enabled' table extra config .. Patch Set 2: (2 comments) Yingchun has also been adding another extra config (https://gerrit.cloudera.org/c/13659/). In his review I asked whether we should update these unit tests every time a new extra config is added. I'm not sure it's necessary; could you take a look at that review comment in his review and add your thoughts? > > I'm curious about how to implement the read-only range partition? > > For the read-only range partitions, it seems that the tablet-based > > configuration might be better, not a table. :) > > Hello Yao Xu, talking about this with Grant before. Yeah, table level config > is not appropriate. An RPC would be used to set the tablet state to READ_ONLY > (https://github.com/apache/kudu/blob/master/src/kudu/tablet/metadata.proto#L58). > What do you think? @Adar Hmm. Ideally all tablet configuration would be mediated by the master, so that clients/tools need only connect to masters, not to individual tservers. Moreover, the configuration should be persisted, otherwise it'd need to be reapplied whenever a tserver reboots. What's tricky here is that we try hard to avoid exposing tablets to users; they normally think about tables instead. And for this feature we'd want to refer to tablets by partition key range (or by start partition key) rather than by tablet ID. Maybe solicit input from Grant, who filed the ticket originally? http://gerrit.cloudera.org:8080/#/c/13796/2/src/kudu/tserver/tablet_service.cc File src/kudu/tserver/tablet_service.cc: http://gerrit.cloudera.org:8080/#/c/13796/2/src/kudu/tserver/tablet_service.cc@1084 PS2, Line 1084: boost::optional extra_config = tablet->metadata()->extra_config(); This makes a copy, so consider storing a cref: const auto& extra_config = tablet->metadata()->extra_config(). http://gerrit.cloudera.org:8080/#/c/13796/2/src/kudu/util/status.h File src/kudu/util/status.h: http://gerrit.cloudera.org:8080/#/c/13796/2/src/kudu/util/status.h@450 PS2, Line 450: kNotEnabled = 19, We're very very hesitant to introduce new Status codes. In this case, perhaps IllegalState would be OK? -- To view, visit http://gerrit.cloudera.org:8080/13796 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibd3768eda36d9574be9c41e7cd3dd81cc5ae8f3a Gerrit-Change-Number: 13796 Gerrit-PatchSet: 2 Gerrit-Owner: XiaokaiWang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: XiaokaiWang Gerrit-Reviewer: Yao Xu Gerrit-Comment-Date: Wed, 10 Jul 2019 15:28:05 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2722 (table level): Support new 'write enabled' table extra config
Hello Tidy Bot, Kudu Jenkins, Yao Xu, Adar Dembo, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13796 to look at the new patch set (#3). Change subject: KUDU-2722 (table level): Support new 'write_enabled' table extra config .. KUDU-2722 (table level): Support new 'write_enabled' table extra config Change-Id: Ibd3768eda36d9574be9c41e7cd3dd81cc5ae8f3a --- M src/kudu/client/client-test.cc M src/kudu/common/common.proto M src/kudu/common/wire_protocol.cc M src/kudu/tablet/tablet_metadata-test.cc M src/kudu/tools/kudu-admin-test.cc M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/ts_tablet_manager-test.cc M src/kudu/tserver/tserver.proto M src/kudu/util/status.cc M src/kudu/util/status.h 10 files changed, 136 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/96/13796/3 -- To view, visit http://gerrit.cloudera.org:8080/13796 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibd3768eda36d9574be9c41e7cd3dd81cc5ae8f3a Gerrit-Change-Number: 13796 Gerrit-PatchSet: 3 Gerrit-Owner: XiaokaiWang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: XiaokaiWang Gerrit-Reviewer: Yao Xu
[kudu-CR] KUDU-2823 Place tablet replicas based on dimension
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13632 ) Change subject: KUDU-2823 Place tablet replicas based on dimension .. Patch Set 15: (2 comments) http://gerrit.cloudera.org:8080/#/c/13632/15/src/kudu/client/client.h File src/kudu/client/client.h: http://gerrit.cloudera.org:8080/#/c/13632/15/src/kudu/client/client.h@879 PS15, Line 879: /// @note By default or masters doesn't set '--master_place_tablet_replicas_based_on_dimension', : /// it will select a tablet server with a smaller number of tablet replicas to place newly : /// created tablet replicas. If set the dimension label, the newly created tablet will be : /// evenly distributed in the cluster distribution based on dimension. In other words, it will : /// select a tablet server with a smaller number of tablet replicas in this dimension. Thanks, this is useful. I'd rewrite it though: 'By default (or if the cluster is configured without '--master_place...'), the master will try to place newly created tablet replicas on tablet servers with a small number of tablet replicas. If the dimension label is provided, newly created replicas will be evenly distributed in the cluster based on the dimension label. In other words, the master will try to place newly created tablet replicas on tablet servers with a small number of tablet replicas belonging to this dimension label.' http://gerrit.cloudera.org:8080/#/c/13632/12/src/kudu/master/master.proto File src/kudu/master/master.proto: http://gerrit.cloudera.org:8080/#/c/13632/12/src/kudu/master/master.proto@365 PS12, Line 365: // Indicates that the tablet server needs to report the number of tablets > Done Still not clear on why this is necessary. Why not have tservers always report this information? -- To view, visit http://gerrit.cloudera.org:8080/13632 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I48a225e221eb42ef2f5489687e80a151d8dc1a42 Gerrit-Change-Number: 13632 Gerrit-PatchSet: 15 Gerrit-Owner: Yao Xu 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: Yao Xu Gerrit-Comment-Date: Wed, 10 Jul 2019 14:44:36 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2722 (table level): Support new 'write enabled' table extra config
XiaokaiWang has posted comments on this change. ( http://gerrit.cloudera.org:8080/13796 ) Change subject: KUDU-2722 (table level): Support new 'write_enabled' table extra config .. Patch Set 2: > I'm curious about how to implement the read-only range partition? > For the read-only range partitions, it seems that the tablet-based > configuration might be better, not a table. :) Hello Yao Xu, talking about this with Grant before. Yeah, table level config is not appropriate. An RPC would be used to set the tablet state to READ_ONLY (https://github.com/apache/kudu/blob/master/src/kudu/tablet/metadata.proto#L58). What do you think? @Adar -- To view, visit http://gerrit.cloudera.org:8080/13796 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibd3768eda36d9574be9c41e7cd3dd81cc5ae8f3a Gerrit-Change-Number: 13796 Gerrit-PatchSet: 2 Gerrit-Owner: XiaokaiWang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: XiaokaiWang Gerrit-Reviewer: Yao Xu Gerrit-Comment-Date: Wed, 10 Jul 2019 14:36:14 + Gerrit-HasComments: No
[kudu-CR] KUDU-2722 (table level): Support new 'write enabled' table extra config
XiaokaiWang has posted comments on this change. ( http://gerrit.cloudera.org:8080/13796 ) Change subject: KUDU-2722 (table level): Support new 'write_enabled' table extra config .. Patch Set 2: > (14 comments) Thanks for your comments, Adar. I update the code for your all comments, please take a review again. -- To view, visit http://gerrit.cloudera.org:8080/13796 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibd3768eda36d9574be9c41e7cd3dd81cc5ae8f3a Gerrit-Change-Number: 13796 Gerrit-PatchSet: 2 Gerrit-Owner: XiaokaiWang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: XiaokaiWang Gerrit-Reviewer: Yao Xu Gerrit-Comment-Date: Wed, 10 Jul 2019 14:28:50 + Gerrit-HasComments: No
[kudu-CR] KUDU-2722 (table level): Support new 'write enabled' table extra config
Hello Tidy Bot, Kudu Jenkins, Yao Xu, Adar Dembo, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13796 to look at the new patch set (#2). Change subject: KUDU-2722 (table level): Support new 'write_enabled' table extra config .. KUDU-2722 (table level): Support new 'write_enabled' table extra config Change-Id: Ibd3768eda36d9574be9c41e7cd3dd81cc5ae8f3a --- M src/kudu/client/client-test.cc M src/kudu/common/common.proto M src/kudu/common/wire_protocol.cc M src/kudu/tablet/tablet_metadata-test.cc M src/kudu/tools/kudu-admin-test.cc M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/ts_tablet_manager-test.cc M src/kudu/tserver/tserver.proto M src/kudu/util/status.h 9 files changed, 133 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/96/13796/2 -- To view, visit http://gerrit.cloudera.org:8080/13796 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibd3768eda36d9574be9c41e7cd3dd81cc5ae8f3a Gerrit-Change-Number: 13796 Gerrit-PatchSet: 2 Gerrit-Owner: XiaokaiWang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: XiaokaiWang Gerrit-Reviewer: Yao Xu
[kudu-CR] Separate benchmark to single test
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13832 ) Change subject: Separate benchmark to single test .. Patch Set 1: (1 comment) How slow are these tests in DEBUG mode? http://gerrit.cloudera.org:8080/#/c/13832/1/src/kudu/common/wire_protocol-test.cc File src/kudu/common/wire_protocol-test.cc: http://gerrit.cloudera.org:8080/#/c/13832/1/src/kudu/common/wire_protocol-test.cc@63 PS1, Line 63: class WireProtocolTest : public KuduTest, Doc up here that the int corresponds to the number of columns and the double to the selection rate. -- To view, visit http://gerrit.cloudera.org:8080/13832 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I980234bcdf76a7d2978f49ed59dff21184abf892 Gerrit-Change-Number: 13832 Gerrit-PatchSet: 1 Gerrit-Owner: ZhangYao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 10 Jul 2019 14:21:21 + Gerrit-HasComments: Yes
[kudu-CR] [docs] update the upgrade documentation
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13820 ) Change subject: [docs] update the upgrade documentation .. Patch Set 2: (11 comments) http://gerrit.cloudera.org:8080/#/c/13820/2/docs/installation.adoc File docs/installation.adoc: http://gerrit.cloudera.org:8080/#/c/13820/2/docs/installation.adoc@635 PS2, Line 635: for the scenario of compiling from source code. "relevant when building from source code". http://gerrit.cloudera.org:8080/#/c/13820/2/docs/installation.adoc@640 PS2, Line 640: kudu-tserver "tablet server" http://gerrit.cloudera.org:8080/#/c/13820/2/docs/installation.adoc@640 PS2, Line 640: 2hours "2 hours" http://gerrit.cloudera.org:8080/#/c/13820/2/docs/installation.adoc@645 PS2, Line 645: --force Shouldn't need this; the flag is tagged as 'runtime'. http://gerrit.cloudera.org:8080/#/c/13820/2/docs/installation.adoc@647 PS2, Line 647: and the gflag above should be reset after every reboot. Since step 4 restores the gflag value, do we really need this step? http://gerrit.cloudera.org:8080/#/c/13820/2/docs/installation.adoc@647 PS2, Line 647: kudu-tserver "the tablet servers" http://gerrit.cloudera.org:8080/#/c/13820/2/docs/installation.adoc@649 PS2, Line 649: [NOTE] Shouldn't this be just after "Set the unavailable time..." so users understand why it's important to change the gflag value before the rolling restart? http://gerrit.cloudera.org:8080/#/c/13820/2/docs/installation.adoc@651 PS2, Line 651: If the gflag is not reset, kudu-tserver which is restarting would be possibly evicted from the cluster. "If the unavailable time is not extended, restarted tablet servers could be evicted from the cluster." http://gerrit.cloudera.org:8080/#/c/13820/2/docs/installation.adoc@654 PS2, Line 654: Rolling restart We should define what "rolling restart" means too. http://gerrit.cloudera.org:8080/#/c/13820/2/docs/installation.adoc@654 PS2, Line 654: kudu-master "the masters" http://gerrit.cloudera.org:8080/#/c/13820/2/docs/installation.adoc@655 PS2, Line 655: . Restore the default gflag value (5 minutes) for every kudu-tserver. : + : [source,bash] : : ./kudu tserver set_flag follower_unavailable_considered_failed_sec 300 --force : Is this actually necessary? Won't restarting the process restore the default value of all of its gflags? Come to think of it, how does the gflag value overriding work if restarting a tserver restores the original value? Seems like we'd need to restart the tserver AND THEN override follower_unavailable_considered_failed_sec. -- To view, visit http://gerrit.cloudera.org:8080/13820 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6b3e5c549dc05c3388c0b0dd628d205a356da344 Gerrit-Change-Number: 13820 Gerrit-PatchSet: 2 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Priyanka Chheda Gerrit-Reviewer: helifu Gerrit-Comment-Date: Wed, 10 Jul 2019 14:19:13 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2722 (table level): Support table extraConfig 'write enabled' conf
XiaokaiWang has posted comments on this change. ( http://gerrit.cloudera.org:8080/13796 ) Change subject: KUDU-2722 (table level): Support table extraConfig 'write_enabled' conf .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/13796/1/src/kudu/client/client-test.cc File src/kudu/client/client-test.cc: http://gerrit.cloudera.org:8080/#/c/13796/1/src/kudu/client/client-test.cc@2415 PS1, Line 2415: session->SetTimeoutMillis(1); > Is this strictly necessary for the test to pass? The session only insert one row, it's not necessary. I will delete it. -- To view, visit http://gerrit.cloudera.org:8080/13796 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibd3768eda36d9574be9c41e7cd3dd81cc5ae8f3a Gerrit-Change-Number: 13796 Gerrit-PatchSet: 1 Gerrit-Owner: XiaokaiWang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: XiaokaiWang Gerrit-Reviewer: Yao Xu Gerrit-Comment-Date: Wed, 10 Jul 2019 13:44:50 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2881 Support create/drop range partition by command line
honeyhexin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13833 Change subject: KUDU-2881 Support create/drop range partition by command line .. KUDU-2881 Support create/drop range partition by command line Sometimes we need to drop the range partition and then recreate it in order to rewrite in case that the partition was written wrongly. This change supports to drop/create range partition by command line. The usage is: 1. kudu table create_range_partition [-lower_bound_type] [-upper_bound_type] 2. kudu table drop_range_partition [-lower_bound_type] [-upper_bound_type] The parameters of lower_bound_type and upper_bound_type is optional. If these two parameters are not specified, the default value are INCLUSIVE_BOUND and EXCLUSIVE_BOUND respectively. Change-Id: I647d9c3cf01807bd8ae9f8cf4091568ea6f1161c --- M src/kudu/common/partition.cc M src/kudu/common/partition.h M src/kudu/integration-tests/ts_itest-base.cc M src/kudu/integration-tests/ts_itest-base.h M src/kudu/tools/kudu-admin-test.cc M src/kudu/tools/tool_action_table.cc 6 files changed, 412 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/33/13833/1 -- To view, visit http://gerrit.cloudera.org:8080/13833 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I647d9c3cf01807bd8ae9f8cf4091568ea6f1161c Gerrit-Change-Number: 13833 Gerrit-PatchSet: 1 Gerrit-Owner: honeyhexin
[kudu-CR] KUDU-2855 Lazy-create DeltaMemStore on first update
Hello Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13821 to look at the new patch set (#3). Change subject: KUDU-2855 Lazy-create DeltaMemStore on first update .. KUDU-2855 Lazy-create DeltaMemStore on first update This patch supports lazy-create DeltaMemStore on first update to save memory and to fast-path out any DMS-related code. The created DeltaMemStore will be destroyed on the following flush. Change-Id: Ie0c565d86647d5144266b30aa6e8572d42db48c6 --- M src/kudu/tablet/delta_tracker.cc M src/kudu/tablet/delta_tracker.h M src/kudu/tablet/diskrowset-test.cc 3 files changed, 87 insertions(+), 47 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/21/13821/3 -- To view, visit http://gerrit.cloudera.org:8080/13821 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie0c565d86647d5144266b30aa6e8572d42db48c6 Gerrit-Change-Number: 13821 Gerrit-PatchSet: 3 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu
[kudu-CR] KUDU-2855 Lazy-create DeltaMemStore on first update
helifu has posted comments on this change. (
http://gerrit.cloudera.org:8080/13821 )
Change subject: KUDU-2855 Lazy-create DeltaMemStore on first update
..
Patch Set 2:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.h
File src/kudu/tablet/delta_tracker.h:
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.h@336
PS2, Line 336: const scoped_refptr
log_anchor_registry_;
> Why can't this remain a raw pointer?
The "log_anchor_registry_" will be a wild pointer if we use a raw point here
because it will be release by "registry_" from class MinLogIndexAnchorer.
For example, TestRowSet.TestCompactStores from diskrowset-test.cc:
1) the object "log::LogAnchorRegistry" is a raw pointer:
https://github.com/apache/kudu/blob/3cbc0d4fbe295748d6ffdf1e5e7edeaf94ef0911/src/kudu/tablet/diskrowset-test-base.h#L330
2) the raw pointer will be released by "registry_" from class
MinLogIndexAnchorer
3) then the next creation for DMS is dangerous.
==
On the other side, yes, we can use a raw pointer indeed. But I think it's
better to use a ref_ptr because we can't ensure that every caller retains its
lifecycle.
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.h@352
PS2, Line 352: dms_exist_
> Nit: dms_exists_
Done
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.h@356
PS2, Line 356: // - Mutators take this lock in exclusive mode while dms_ is
not initialized.
> "Mutators take this lock in exclusive mode if they need to create a new DMS
Done
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.cc
File src/kudu/tablet/delta_tracker.cc:
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.cc@78
PS2, Line 78: DEFINE_bool(dms_lazy_create, true,
: "Allow lazily creating of DeltaMemStore");
: TAG_FLAG(dms_lazy_create, hidden);
> What's the purpose of this if it's hidden?
Should I keep this gflag or just discard it? If yes, maybe some test cases to
cover true or false are necessary.
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.cc@169
PS2, Line 169: Status DeltaTracker::CreateAndInitDMSUnlocked(const
fs::IOContext* io_context) {
> This should only be done with component_lock_ held in exclusive mode, right
Done
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.cc@601
PS2, Line 601: !dms_->Empty()
> This seems like a semantic change in that previously CollectStores returned
Emmm, the DMS will be not returned when dms_exists_ is false either.
And "!dms_->Empty" can help to avoid returning a empty DMS, though that is
unlikely to happen.
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.cc@659
PS2, Line 659: Status s = Status::OK();
> This is the default value of a Status.
Done
http://gerrit.cloudera.org:8080/#/c/13821/2/src/kudu/tablet/delta_tracker.cc@673
PS2, Line 673: operations of the above component_lock_
> "critical sections defined by component_lock_"
Done
--
To view, visit http://gerrit.cloudera.org:8080/13821
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0c565d86647d5144266b30aa6e8572d42db48c6
Gerrit-Change-Number: 13821
Gerrit-PatchSet: 2
Gerrit-Owner: helifu
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: helifu
Gerrit-Comment-Date: Wed, 10 Jul 2019 11:40:33 +
Gerrit-HasComments: Yes
[kudu-CR] Separate benchmark to single test
ZhangYao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13832 Change subject: Separate benchmark to single test .. Separate benchmark to single test Separate the total benchmark to single test with paraments so they all can be run in test time limit. Change-Id: I980234bcdf76a7d2978f49ed59dff21184abf892 --- M src/kudu/common/CMakeLists.txt M src/kudu/common/wire_protocol-test.cc 2 files changed, 12 insertions(+), 14 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/13832/1 -- To view, visit http://gerrit.cloudera.org:8080/13832 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I980234bcdf76a7d2978f49ed59dff21184abf892 Gerrit-Change-Number: 13832 Gerrit-PatchSet: 1 Gerrit-Owner: ZhangYao
[kudu-CR] KUDU-2823 Place tablet replicas based on dimension
Yao Xu has posted comments on this change. (
http://gerrit.cloudera.org:8080/13632 )
Change subject: KUDU-2823 Place tablet replicas based on dimension
..
Patch Set 15:
(12 comments)
> (12 comments)
>
> Just passing through; didn't look at the placement policy or
> testing changes.
>
> Do you intend to roll this out for the Java client API in a
> follow-on patch?
Yes, I will support the Java client API in a follow-on patch.
http://gerrit.cloudera.org:8080/#/c/13632/12/src/kudu/client/client.h
File src/kudu/client/client.h:
http://gerrit.cloudera.org:8080/#/c/13632/12/src/kudu/client/client.h@877
PS12, Line 877: /// Set the dimension label for all tablets created at table
creation time.
> "Set the dimension label for all tablets created at table creation time".
Done
http://gerrit.cloudera.org:8080/#/c/13632/12/src/kudu/client/client.h@879
PS12, Line 879: /// @note By default or masters doesn't set
'--master_place_tablet_replicas_based_on_dimension',
> This needs a more thorough explanation. Reading this, I have no idea what "
Done
http://gerrit.cloudera.org:8080/#/c/13632/12/src/kudu/client/client.h@1217
PS12, Line 1217: KuduPartialRow* lower_bound,
> This is a backwards incompatible change. See
> https://community.kde.org/Policies/Binary_Compatibility_Issues_With_C%2B%2B
> for details on what sort of changes are permitted to retain
> backwards compatibility.
>
> In this case, you'll need to add a new variant of AddRangePartition()
> with the additional argument. You can't add an overload, because
> the existing AddRangePartition did not have any overloads, and
> adding one now would make existing usages of
> ambiguous.
Thanks for pointing this out, I think we need to add a new interface to fix
this issue.
http://gerrit.cloudera.org:8080/#/c/13632/12/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/13632/12/src/kudu/master/catalog_manager.cc@3713
PS12, Line 3713: enable
> enabled
Done
http://gerrit.cloudera.org:8080/#/c/13632/12/src/kudu/master/catalog_manager.cc@3715
PS12, Line 3715: optional Elsewhere you use optional without the boost:: prefix. Pick one approach an
Done
http://gerrit.cloudera.org:8080/#/c/13632/12/src/kudu/master/catalog_manager.cc@3716
PS12, Line 3716: if (FLAGS_master_place_tablet_replicas_based_on_dimension
&&
: tablet_->metadata().state().pb.has_dimension_label()) {
> Combine into one if statement.
Done
http://gerrit.cloudera.org:8080/#/c/13632/12/src/kudu/master/catalog_manager.cc@4668
PS12, Line 4668: one.
> enabled
Done
http://gerrit.cloudera.org:8080/#/c/13632/12/src/kudu/master/catalog_manager.cc@4671
PS12, Line 4671: tablet->metadata().state().pb.has_dimension_label()) {
: dimension = tablet->metadata().state().pb.dimension_label(
> Combine
Done
http://gerrit.cloudera.org:8080/#/c/13632/12/src/kudu/master/master.proto
File src/kudu/master/master.proto:
http://gerrit.cloudera.org:8080/#/c/13632/12/src/kudu/master/master.proto@365
PS12, Line 365: // Indicates that the tablet server needs to report the
number of tablets
> Doc what this means. Also, why does it exist at all? What value does select
Done
http://gerrit.cloudera.org:8080/#/c/13632/12/src/kudu/master/master.proto@472
PS12, Line 472:
> "The dimension label for tablets that were created during table creation."
Done
http://gerrit.cloudera.org:8080/#/c/13632/12/src/kudu/master/ts_descriptor.h
File src/kudu/master/ts_descriptor.h:
http://gerrit.cloudera.org:8080/#/c/13632/12/src/kudu/master/ts_descriptor.h@138
PS12, Line 138: int num_live_replicas(const boost::optional&
dimension = boost::none) const {
> Odd that the optional isn't passed by cref, given that we're not std::move'
Done
http://gerrit.cloudera.org:8080/#/c/13632/12/src/kudu/tserver/ts_tablet_manager.h
File src/kudu/tserver/ts_tablet_manager.h:
http://gerrit.cloudera.org:8080/#/c/13632/12/src/kudu/tserver/ts_tablet_manager.h@205
PS12, Line 205: TabletNumByDimensionMap GetNumLiveTabletsByDimension() const;
> Could we just return the map?
Done
--
To view, visit http://gerrit.cloudera.org:8080/13632
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I48a225e221eb42ef2f5489687e80a151d8dc1a42
Gerrit-Change-Number: 13632
Gerrit-PatchSet: 15
Gerrit-Owner: Yao Xu
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: Yao Xu
Gerrit-Comment-Date: Wed, 10 Jul 2019 10:32:36 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2847: Optimize iteration over selection vector in SerializeRowBlock
Yao Xu has posted comments on this change. ( http://gerrit.cloudera.org:8080/13721 ) Change subject: KUDU-2847: Optimize iteration over selection vector in SerializeRowBlock .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/13721/2/src/kudu/common/wire_protocol.cc File src/kudu/common/wire_protocol.cc: http://gerrit.cloudera.org:8080/#/c/13721/2/src/kudu/common/wire_protocol.cc@935 PS2, Line 935: select_row_idx->resize(select_row_idx->size() + run_size); I think it would be better to save the selected row idx boundary instead of all the selected row idx. http://gerrit.cloudera.org:8080/#/c/13721/2/src/kudu/common/wire_protocol.cc@999 PS2, Line 999: const ColumnBlock column_block const ColumnBlock& column_block -- To view, visit http://gerrit.cloudera.org:8080/13721 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I19917d1875c46fd4cf98ef8a471b0340a76161e7 Gerrit-Change-Number: 13721 Gerrit-PatchSet: 2 Gerrit-Owner: ZhangYao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Yao Xu Gerrit-Reviewer: ZhangYao Gerrit-Comment-Date: Wed, 10 Jul 2019 10:27:47 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2823 Place tablet replicas based on dimension
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/13632 to look at the new patch set (#15). Change subject: KUDU-2823 Place tablet replicas based on dimension .. KUDU-2823 Place tablet replicas based on dimension When we add a new range to the fact table, we expect replicas of the newly created tablet to be evenly distributed across all available tablet servers. This is especially important when we use time as the range key. The more recent the data, the hotter it gets. We expect hot tablets on the cluster to be evenly distributed. Unfortunately, after we add some new tablet servers to the cluster, creating a new tablet replica will prioritize the new tablet server for placement according to the current placement policy. This is because we prefer to select tablet server which a smaller number of tablet replicas. This will cause hot tablets to be concentrated on these new tablet servers. So, I added a new placement policy. When creating a new tablet replica, we prefer to select tablet server which a smaller number of tablet replicas in the dimension. You can set dimensions when creating tables and adding partitions, this will ensure that the new tablets are evenly distributed in the cluster based on dimension. You can decide whether to use this feature by setting '--master_place_tablet_replicas_based_on_dimension'. Change-Id: I48a225e221eb42ef2f5489687e80a151d8dc1a42 --- M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/table_alterer-internal.cc M src/kudu/client/table_alterer-internal.h M src/kudu/client/table_creator-internal.h M src/kudu/integration-tests/create-table-itest.cc M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master.proto M src/kudu/master/master_service.cc M src/kudu/master/placement_policy-test.cc M src/kudu/master/placement_policy.cc M src/kudu/master/placement_policy.h M src/kudu/master/sys_catalog.cc M src/kudu/master/ts_descriptor.cc M src/kudu/master/ts_descriptor.h M src/kudu/tablet/metadata.proto M src/kudu/tablet/tablet-harness.h M src/kudu/tablet/tablet_bootstrap-test.cc M src/kudu/tablet/tablet_metadata.cc M src/kudu/tablet/tablet_metadata.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tserver/heartbeater.cc M src/kudu/tserver/mini_tablet_server.cc M src/kudu/tserver/tablet_copy_client.cc M src/kudu/tserver/tablet_server-test.cc M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/ts_tablet_manager-test.cc M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/ts_tablet_manager.h M src/kudu/tserver/tserver_admin.proto 31 files changed, 586 insertions(+), 74 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/13632/15 -- To view, visit http://gerrit.cloudera.org:8080/13632 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I48a225e221eb42ef2f5489687e80a151d8dc1a42 Gerrit-Change-Number: 13632 Gerrit-PatchSet: 15 Gerrit-Owner: Yao Xu 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: Yao Xu
[kudu-CR] KUDU-2847: Optimize iteration over selection vector in SerializeRowBlock
ZhangYao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13721 ) Change subject: KUDU-2847: Optimize iteration over selection vector in SerializeRowBlock .. Patch Set 2: Here is the result(Before/After), I only pick the real time in the form below. total row count:1 Schema size 3 30300 Select rate 1 0 0.002/0.002 0.085/0.082 1.617/1.656(maybe all selected need futher optimization) 0.8 0.003/0.002 0.072/0.064 1.457/1.266 0.5 0.004/0.002 0.054/0.041 1.044/0.825 0.2 0.002/0.001 0.027/0.017 0.509/0.280 We can see it have some optimization althought no that much in some case :) The cpu occupation benchmark will be update later. -- To view, visit http://gerrit.cloudera.org:8080/13721 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I19917d1875c46fd4cf98ef8a471b0340a76161e7 Gerrit-Change-Number: 13721 Gerrit-PatchSet: 2 Gerrit-Owner: ZhangYao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Yao Xu Gerrit-Reviewer: ZhangYao Gerrit-Comment-Date: Wed, 10 Jul 2019 09:47:04 + Gerrit-HasComments: No
[kudu-CR] KUDU-2847: Optimize iteration over selection vector in SerializeRowBlock
Hello Kudu Jenkins, Yao Xu, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13721 to look at the new patch set (#2). Change subject: KUDU-2847: Optimize iteration over selection vector in SerializeRowBlock .. KUDU-2847: Optimize iteration over selection vector in SerializeRowBlock Convert the bitmap into vector before serialize row block, thereby it avoid traverse bitmap for each column. Change-Id: I19917d1875c46fd4cf98ef8a471b0340a76161e7 --- M src/kudu/common/wire_protocol.cc 1 file changed, 58 insertions(+), 42 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/21/13721/2 -- To view, visit http://gerrit.cloudera.org:8080/13721 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I19917d1875c46fd4cf98ef8a471b0340a76161e7 Gerrit-Change-Number: 13721 Gerrit-PatchSet: 2 Gerrit-Owner: ZhangYao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Yao Xu Gerrit-Reviewer: ZhangYao
[kudu-CR] [docs] update the upgrade documentation
Hello Kudu Jenkins, Adar Dembo, Priyanka Chheda, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13820 to look at the new patch set (#2). Change subject: [docs] update the upgrade documentation .. [docs] update the upgrade documentation The process of upgrading the cluster has been added to the installation.adoc. Change-Id: I6b3e5c549dc05c3388c0b0dd628d205a356da344 --- M docs/installation.adoc 1 file changed, 26 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/13820/2 -- To view, visit http://gerrit.cloudera.org:8080/13820 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6b3e5c549dc05c3388c0b0dd628d205a356da344 Gerrit-Change-Number: 13820 Gerrit-PatchSet: 2 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Priyanka Chheda Gerrit-Reviewer: helifu
[kudu-CR] [docs] update the upgrade documentation
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/13820 ) Change subject: [docs] update the upgrade documentation .. Patch Set 1: (7 comments) Here is a link to display the rendered form: https://github.com/helifu/kudu/blob/master/docs/installation.adoc#upgrade And I use "asciidoctor installation.adoc -b html" to verify it locally. http://gerrit.cloudera.org:8080/#/c/13820/1/docs/installation.adoc File docs/installation.adoc: http://gerrit.cloudera.org:8080/#/c/13820/1/docs/installation.adoc@635 PS1, Line 635: . Prepare the software. > This recommendation is only necessary if you're actually building from sour Done http://gerrit.cloudera.org:8080/#/c/13820/1/docs/installation.adoc@636 PS1, Line 636: building > build Done http://gerrit.cloudera.org:8080/#/c/13820/1/docs/installation.adoc@638 PS1, Line 638: big > large Done http://gerrit.cloudera.org:8080/#/c/13820/1/docs/installation.adoc@645 PS1, Line 645: - Rolling restart `kudu-tserver` and the gflag above should be reset after every rebooting. > Could you provide specific instructions for restoring the default gflag val Done http://gerrit.cloudera.org:8080/#/c/13820/1/docs/installation.adoc@645 PS1, Line 645: rebooting > reboot Done http://gerrit.cloudera.org:8080/#/c/13820/1/docs/installation.adoc@648 PS1, Line 648: . Restore the original gflags. > Likewise, this duplicates the suggestion on L645. Done http://gerrit.cloudera.org:8080/#/c/13820/1/docs/installation.adoc@650 PS1, Line 650: WARNING: To prevent the restarted `tserver` from being evicted from the cluster, the gflag should be reset. > Could you roll this into the rolling restart instructions? No need to dupli Done -- To view, visit http://gerrit.cloudera.org:8080/13820 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6b3e5c549dc05c3388c0b0dd628d205a356da344 Gerrit-Change-Number: 13820 Gerrit-PatchSet: 1 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Priyanka Chheda Gerrit-Reviewer: helifu Gerrit-Comment-Date: Wed, 10 Jul 2019 08:26:27 + Gerrit-HasComments: Yes
[kudu-CR] [metrics] Merge metrics by the same attribute
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/13580 )
Change subject: [metrics] Merge metrics by the same attribute
..
Patch Set 5:
(37 comments)
I didn't review metrics-test.cc yet.
http://gerrit.cloudera.org:8080/#/c/13580/5/src/kudu/server/default_path_handlers.cc
File src/kudu/server/default_path_handlers.cc:
http://gerrit.cloudera.org:8080/#/c/13580/5/src/kudu/server/default_path_handlers.cc@353
PS5, Line 353: if (entity_type == "tablet") {
: // Entities in type of 'tablet' who have the same
attribute of 'table_name' will be merged
: // to a new entity with type 'table' and id the value of
'table_name'.
: EmplaceIfNotPresent(_attributes_by_entity_type,
entity_type,
: new MetricJsonOptions::MergeAttributes("table",
"table_name"));
: }
This leaks table/tablet concepts into the server/ module, which, like util/
should remain generic.
http://gerrit.cloudera.org:8080/#/c/13580/5/src/kudu/util/hdr_histogram-test.cc
File src/kudu/util/hdr_histogram-test.cc:
http://gerrit.cloudera.org:8080/#/c/13580/5/src/kudu/util/hdr_histogram-test.cc@133
PS5, Line 133: hist.Merge(other);
Should ASSERT_TRUE here.
http://gerrit.cloudera.org:8080/#/c/13580/5/src/kudu/util/hdr_histogram-test.cc@137
PS5, Line 137: ASSERT_EQ(hist.MinValue(), std::min(old.MinValue(),
other.MinValue()));
: ASSERT_EQ(hist.MaxValue(), std::max(old.MaxValue(),
other.MaxValue()));
Maybe just test for 1 and 250 here instead? Would be clearer.
http://gerrit.cloudera.org:8080/#/c/13580/5/src/kudu/util/hdr_histogram-test.cc@139
PS5, Line 139: ASSERT_LE(hist.MeanValue() - (1 + 250) / 2.0, 1e-6);
Maybe ASSERT_NEAR would help here (and maybe below too)?
http://gerrit.cloudera.org:8080/#/c/13580/5/src/kudu/util/hdr_histogram.h
File src/kudu/util/hdr_histogram.h:
http://gerrit.cloudera.org:8080/#/c/13580/5/src/kudu/util/hdr_histogram.h@174
PS5, Line 174: bool Merge(const HdrHistogram& other);
Doc.
http://gerrit.cloudera.org:8080/#/c/13580/5/src/kudu/util/hdr_histogram.cc
File src/kudu/util/hdr_histogram.cc:
http://gerrit.cloudera.org:8080/#/c/13580/5/src/kudu/util/hdr_histogram.cc@348
PS5, Line 348: if (PREDICT_FALSE(highest_trackable_value_ !=
other.highest_trackable_value())) {
: LOG(WARNING) << "highest_trackable_value_ not match.";
: return false;
: }
:
: if (PREDICT_FALSE(num_significant_digits_ !=
other.num_significant_digits())) {
: LOG(WARNING) << "num_significant_digits_ not match.";
: return false;
: }
Should these DCHECK/CHECK instead, like L358-363? Under what circumstances
would you expect to see this case at runtime?
http://gerrit.cloudera.org:8080/#/c/13580/5/src/kudu/util/hdr_histogram.cc@368
PS5, Line 368: // Update min, if needed.
: {
: Atomic64 min_val;
: while (other.min_value_ < (min_val = MinValue())) {
: Atomic64 old_val = NoBarrier_CompareAndSwap(_value_,
min_val, other.min_value_);
: if (PREDICT_TRUE(old_val == min_val)) break; // CAS
success.
: }
: }
:
: // Update max, if needed.
: {
: Atomic64 max_val;
: while (other.max_value_ > (max_val = MaxValue())) {
: Atomic64 old_val = NoBarrier_CompareAndSwap(_value_,
max_val, other.max_value_);
: if (PREDICT_TRUE(old_val == max_val)) break; // CAS
success.
: }
: }
:
: for (int i = 0; i < counts_array_length_; i++) {
: Atomic64 count = NoBarrier_Load(_[i]);
: if (count > 0) {
: NoBarrier_AtomicIncrement(_[i], count);
: }
: }
Could you share this with IncrementBy rather than copying it?
http://gerrit.cloudera.org:8080/#/c/13580/5/src/kudu/util/metrics.h
File src/kudu/util/metrics.h:
http://gerrit.cloudera.org:8080/#/c/13580/5/src/kudu/util/metrics.h@226
PS5, Line 226: #include
Nit: prefer
http://gerrit.cloudera.org:8080/#/c/13580/5/src/kudu/util/metrics.h@445
PS5, Line 445: set
setting
http://gerrit.cloudera.org:8080/#/c/13580/5/src/kudu/util/metrics.h@467
PS5, Line 467: struct MergeAttributes : public
RefCountedThreadSafe {
Not clear why this needs to be RefCountedThreadSafe.
http://gerrit.cloudera.org:8080/#/c/13580/5/src/kudu/util/metrics.h@474
PS5, Line 474: merge
merged
http://gerrit.cloudera.org:8080/#/c/13580/5/src/kudu/util/metrics.h@477
PS5, Line 477: in
Nit: "is in"
http://gerrit.cloudera.org:8080/#/c/13580/5/src/kudu/util/metrics.h@478
PS5, Line 478: to acknowledge
Nit: replace with "for"
[kudu-CR] KUDU-2797: the master aggregates tablet statistics
Hello Mike Percy, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13426 to look at the new patch set (#20). Change subject: KUDU-2797: the master aggregates tablet statistics .. KUDU-2797: the master aggregates tablet statistics There are some jiras are talking about metrics: KUDU-1067, KUDU-1373, KUDU-2019, KUDU-2797. In this patch, it makes the following improvements: 1) live row count of the tablet is exposed as metrics on the tablet server; 2) live row count of the tablet is exposed on the tablet server's Web-UI; 3) disk size and live row count of all the replicas are aggregated on the master server (only the ones that are the leadership roles are aggregated); 4) disk size and live row count of the table are exposed as metrics on the master server; 5) disk size and live row count of the table are exposed on the master server's Web-UI; Change-Id: I74406ab7cca7c22fda455c328b8ee9989a6b2d99 --- M src/kudu/integration-tests/tablet_replacement-itest.cc M src/kudu/integration-tests/ts_tablet_manager-itest.cc M src/kudu/master/CMakeLists.txt M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master.proto M src/kudu/master/master_path_handlers.cc A src/kudu/master/table_metrics.cc A src/kudu/master/table_metrics.h M src/kudu/tablet/local_tablet_writer.h M src/kudu/tablet/metadata.proto M src/kudu/tablet/tablet_replica.cc M src/kudu/tablet/tablet_replica.h M src/kudu/tserver/heartbeater.cc M src/kudu/tserver/heartbeater.h M src/kudu/tserver/ts_tablet_manager-test.cc M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/ts_tablet_manager.h M src/kudu/tserver/tserver_path_handlers.cc M www/table.mustache M www/tablet.mustache 21 files changed, 560 insertions(+), 34 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/13426/20 -- To view, visit http://gerrit.cloudera.org:8080/13426 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I74406ab7cca7c22fda455c328b8ee9989a6b2d99 Gerrit-Change-Number: 13426 Gerrit-PatchSet: 20 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: helifu
[kudu-CR] KUDU-2722 (table level): Support table extraConfig 'write enabled' conf
Yao Xu has posted comments on this change. ( http://gerrit.cloudera.org:8080/13796 ) Change subject: KUDU-2722 (table level): Support table extraConfig 'write_enabled' conf .. Patch Set 1: I'm curious about how to implement the read-only range partition? For the read-only range partitions, it seems that the tablet-based configuration might be better, not a table. :) -- To view, visit http://gerrit.cloudera.org:8080/13796 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibd3768eda36d9574be9c41e7cd3dd81cc5ae8f3a Gerrit-Change-Number: 13796 Gerrit-PatchSet: 1 Gerrit-Owner: XiaokaiWang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yao Xu Gerrit-Comment-Date: Wed, 10 Jul 2019 06:16:21 + Gerrit-HasComments: No
