[kudu-CR] [tablet] Support accurate count of rows
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/13456 ) Change subject: [tablet] Support accurate count of rows .. Patch Set 10: (2 comments) http://gerrit.cloudera.org:8080/#/c/13456/10/src/kudu/tablet/compaction-test.cc File src/kudu/tablet/compaction-test.cc: http://gerrit.cloudera.org:8080/#/c/13456/10/src/kudu/tablet/compaction-test.cc@1206 PS10, Line 1206: ASSERT_NO_FATAL_FAILURE(); > Since you've wrapped function calls that can ASSERT with NO_FATALS(), you d Done http://gerrit.cloudera.org:8080/#/c/13456/10/src/kudu/tablet/diskrowset.cc File src/kudu/tablet/diskrowset.cc: http://gerrit.cloudera.org:8080/#/c/13456/10/src/kudu/tablet/diskrowset.cc@205 PS10, Line 205: if (rowset_metadata_->tablet_metadata()->supports_live_row_count() && > After reviewing this a couple times, I think it'd be clearer if IncrementLi Yea, very good advice! -- To view, visit http://gerrit.cloudera.org:8080/13456 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2e6378e289bb85024c29e96c2b153fc417ed6412 Gerrit-Change-Number: 13456 Gerrit-PatchSet: 10 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu Gerrit-Comment-Date: Tue, 04 Jun 2019 05:50:33 + Gerrit-HasComments: Yes
[kudu-CR] [tablet] Support accurate count of rows
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/13456 )
Change subject: [tablet] Support accurate count of rows
..
Patch Set 10:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/13456/10/src/kudu/tablet/compaction-test.cc
File src/kudu/tablet/compaction-test.cc:
http://gerrit.cloudera.org:8080/#/c/13456/10/src/kudu/tablet/compaction-test.cc@1206
PS10, Line 1206: ASSERT_NO_FATAL_FAILURE();
Since you've wrapped function calls that can ASSERT with NO_FATALS(), you don't
need the standalone ASSERT_NO_FATAL_FAILURE() calls.
http://gerrit.cloudera.org:8080/#/c/13456/10/src/kudu/tablet/diskrowset.cc
File src/kudu/tablet/diskrowset.cc:
http://gerrit.cloudera.org:8080/#/c/13456/10/src/kudu/tablet/diskrowset.cc@205
PS10, Line 205: if
(rowset_metadata_->tablet_metadata()->supports_live_row_count() &&
After reviewing this a couple times, I think it'd be clearer if
IncrementLiveRows() were responsible for checking supports_live_row_count(),
and if it's false, just ignoring the request.
That applies here as well as in DeltaTracker.
http://gerrit.cloudera.org:8080/#/c/13456/9/src/kudu/tserver/tablet_copy-test-base.h
File src/kudu/tserver/tablet_copy-test-base.h:
http://gerrit.cloudera.org:8080/#/c/13456/9/src/kudu/tserver/tablet_copy-test-base.h@113
PS9, Line 113: virtual void GenerateTestData() {
> I want to inject the modification of the 'supports_live_row_count' through
Ah, missed that addition. Thanks for clarifying.
--
To view, visit http://gerrit.cloudera.org:8080/13456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e6378e289bb85024c29e96c2b153fc417ed6412
Gerrit-Change-Number: 13456
Gerrit-PatchSet: 10
Gerrit-Owner: helifu
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: helifu
Gerrit-Comment-Date: Tue, 04 Jun 2019 05:24:27 +
Gerrit-HasComments: Yes
[kudu-CR] Support SPNEGO for web server
Hello Thomas Marshall, Tidy Bot, Lars Volker, Alexey Serbin, Kudu Jenkins, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13341 to look at the new patch set (#2). Change subject: Support SPNEGO for web server .. Support SPNEGO for web server SPNEGO is a protocol for securing HTTP requests with Kerberos by passing negotiation through HTTP headers. It's supported by most major browsers and also by most of the Java-based Hadoop components. Notably, it's also the typical way in which Apache Knox authenticates itself to Hadoop components in the "trusted proxy" mode, allowing them to be secured behind Knox's SSO and other policies. This patch implements the SPNEGO protocol by driving GSSAPI, and integrates it into the webserver when configured by a new --webserver_require_spnego flag. The new test verifies this end-to-end using curl's SPNEGO authentication support. Along the way I had to cross-port a small change to the Base64 functions in gutil to avoid a UBSAN error. I found the fix in abseil-cpp's copy of the same file. Change-Id: I9449ac610aa7d11bbf320d9178a6d73684ff15f7 --- M src/kudu/gutil/strings/escaping.cc M src/kudu/rpc/client_negotiation.cc M src/kudu/security/CMakeLists.txt A src/kudu/security/gssapi.cc A src/kudu/security/gssapi.h M src/kudu/security/test/mini_kdc.cc M src/kudu/security/test/mini_kdc.h M src/kudu/server/CMakeLists.txt M src/kudu/server/webserver-test.cc M src/kudu/server/webserver.cc M src/kudu/server/webserver_options.cc M src/kudu/server/webserver_options.h M src/kudu/util/curl_util.cc M src/kudu/util/curl_util.h M src/kudu/util/test_macros.h M thirdparty/build-definitions.sh 16 files changed, 604 insertions(+), 62 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/13341/2 -- To view, visit http://gerrit.cloudera.org:8080/13341 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9449ac610aa7d11bbf320d9178a6d73684ff15f7 Gerrit-Change-Number: 13341 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [tablet] Support accurate count of rows
Hello Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13456 to look at the new patch set (#10). Change subject: [tablet] Support accurate count of rows .. [tablet] Support accurate count of rows A tablet is consisted of one MRS and a group of DRS. And the group of DRS makes up a RowSetTree. Thus, the number of live rows in a tablet comes from MRS and RowSetTree. At the same time, a new capability is added to the tablet superblock to ensure that only newly created tablets enable the capability. 1.At the beginning, new rows will be written (insert/delete/ reinsert) into MRS and counter1 holds the number of rows; 2.Next, MRS is being flushed to DRS (flush1): When MRS is being flushed, it will be attached to the RowSetTree, so we can count rows there (still counter1); When MRS is flushed to DRS, new RowSetMetadatas will be created and every one has persistent counter2; 3.Then, there will be updates to DRS and they will be accumulated in DMS. counter3 holds the number of rows; 4.If DMS is flushed (flush2): When DMS is being flushed, it will be attached to the Redo list, and its counter3 will be swapped to DeltaTracker; When DMS is flushed to Redo, the value of counter3 will be added to the counter2 of RowSetMetadata; 5.MinorCompact: No influence since any insertions or deletions come from memory; 6.MajorCompact: No influence, just like step5; 7.Compact: Just like step2. __counter2(persistent) / | __RowSetMetadata __counter3 / / flush1 | | [MRS] ---> [ DRS ... ] -- Undo + Redo + DMS | ^ | \__ |__| counter1 flush2 Change-Id: I2e6378e289bb85024c29e96c2b153fc417ed6412 --- M src/kudu/master/sys_catalog.cc M src/kudu/tablet/compaction-test.cc M src/kudu/tablet/compaction.cc M src/kudu/tablet/delta_tracker.cc M src/kudu/tablet/delta_tracker.h M src/kudu/tablet/deltamemstore-test.cc M src/kudu/tablet/deltamemstore.cc M src/kudu/tablet/deltamemstore.h M src/kudu/tablet/diskrowset-test.cc M src/kudu/tablet/diskrowset.cc M src/kudu/tablet/diskrowset.h M src/kudu/tablet/memrowset-test.cc M src/kudu/tablet/memrowset.cc M src/kudu/tablet/memrowset.h M src/kudu/tablet/metadata.proto M src/kudu/tablet/mock-rowsets.h M src/kudu/tablet/rowset.cc M src/kudu/tablet/rowset.h M src/kudu/tablet/rowset_metadata.cc M src/kudu/tablet/rowset_metadata.h M src/kudu/tablet/tablet-test-util.h M src/kudu/tablet/tablet-test.cc M src/kudu/tablet/tablet.cc M src/kudu/tablet/tablet.h 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/tablet_copy-test-base.h M src/kudu/tserver/tablet_copy_client-test.cc M src/kudu/tserver/tablet_copy_client.cc M src/kudu/tserver/ts_tablet_manager.cc 31 files changed, 393 insertions(+), 33 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/13456/10 -- To view, visit http://gerrit.cloudera.org:8080/13456 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2e6378e289bb85024c29e96c2b153fc417ed6412 Gerrit-Change-Number: 13456 Gerrit-PatchSet: 10 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu
[kudu-CR] [tablet] Support accurate count of rows
helifu has posted comments on this change. (
http://gerrit.cloudera.org:8080/13456 )
Change subject: [tablet] Support accurate count of rows
..
Patch Set 9:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/13456/6//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13456/6//COMMIT_MSG@33
PS6, Line 33: 7.Compact:
: Just like step2.
:
: __counter
> Ah, I see your point: because tablet copy operates at the level of data blo
^_^
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/diskrowset.h
File src/kudu/tablet/diskrowset.h:
PS7:
> Also in RollingDiskRowSetWriter.
Done
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/tablet_metadata.cc
File src/kudu/tablet/tablet_metadata.cc:
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/tablet_metadata.cc@272
PS7, Line 272:const TabletDataState&
tablet_data_state,
> Ah, right. I forgot to remove this suggestion after reviewing that; sorry.
it doesn't matter :)
http://gerrit.cloudera.org:8080/#/c/13456/9/src/kudu/tserver/tablet_copy-test-base.h
File src/kudu/tserver/tablet_copy-test-base.h:
http://gerrit.cloudera.org:8080/#/c/13456/9/src/kudu/tserver/tablet_copy-test-base.h@113
PS9, Line 113: virtual void GenerateTestData() {
> Why does this need to be virtual?
I want to inject the modification of the 'supports_live_row_count' through
polymorphism characteristic. Please look at L147 in
tserver/tablet_copy_client-test.cc. And I think that's a simplest way to
support your first comment in
https://gerrit.cloudera.org/#/c/13456/7/src/kudu/tserver/tablet_copy_client.cc
--
To view, visit http://gerrit.cloudera.org:8080/13456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e6378e289bb85024c29e96c2b153fc417ed6412
Gerrit-Change-Number: 13456
Gerrit-PatchSet: 9
Gerrit-Owner: helifu
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: helifu
Gerrit-Comment-Date: Tue, 04 Jun 2019 04:57:44 +
Gerrit-HasComments: Yes
[kudu-CR] [tablet] Support accurate count of rows
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/13456 )
Change subject: [tablet] Support accurate count of rows
..
Patch Set 9:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/13456/6//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13456/6//COMMIT_MSG@33
PS6, Line 33: 7.Compact:
: Just like step2.
:
: __counter
> No, I mean a replicated tablet can enable the live row counting feature fro
Ah, I see your point: because tablet copy operates at the level of data blocks
and WAL segments, it's difficult to reconstruct the live row count.
This only leaves full recompaction as an option, which I don't think is
realistic. So I guess we'll have to resign ourselves to only having this
feature in truly new tablets.
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/diskrowset.h
File src/kudu/tablet/diskrowset.h:
PS7:
> Done
Also in RollingDiskRowSetWriter.
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/tablet_metadata.cc
File src/kudu/tablet/tablet_metadata.cc:
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/tablet_metadata.cc@272
PS7, Line 272:const TabletDataState&
tablet_data_state,
> No, please look at the code L361 in tablet_copy_client.cc.
Ah, right. I forgot to remove this suggestion after reviewing that; sorry.
http://gerrit.cloudera.org:8080/#/c/13456/9/src/kudu/tserver/tablet_copy-test-base.h
File src/kudu/tserver/tablet_copy-test-base.h:
http://gerrit.cloudera.org:8080/#/c/13456/9/src/kudu/tserver/tablet_copy-test-base.h@113
PS9, Line 113: virtual void GenerateTestData() {
Why does this need to be virtual?
--
To view, visit http://gerrit.cloudera.org:8080/13456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e6378e289bb85024c29e96c2b153fc417ed6412
Gerrit-Change-Number: 13456
Gerrit-PatchSet: 9
Gerrit-Owner: helifu
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: helifu
Gerrit-Comment-Date: Tue, 04 Jun 2019 04:09:30 +
Gerrit-HasComments: Yes
[kudu-CR] [tablet] Support accurate count of rows
Hello Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13456 to look at the new patch set (#9). Change subject: [tablet] Support accurate count of rows .. [tablet] Support accurate count of rows A tablet is consisted of one MRS and a group of DRS. And the group of DRS makes up a RowSetTree. Thus, the number of live rows in a tablet comes from MRS and RowSetTree. At the same time, a new capability is added to the tablet superblock to ensure that only newly created tablets enable the capability. 1.At the beginning, new rows will be written (insert/delete/ reinsert) into MRS and counter1 holds the number of rows; 2.Next, MRS is being flushed to DRS (flush1): When MRS is being flushed, it will be attached to the RowSetTree, so we can count rows there (still counter1); When MRS is flushed to DRS, new RowSetMetadatas will be created and every one has persistent counter2; 3.Then, there will be updates to DRS and they will be accumulated in DMS. counter3 holds the number of rows; 4.If DMS is flushed (flush2): When DMS is being flushed, it will be attached to the Redo list, and its counter3 will be swapped to DeltaTracker; When DMS is flushed to Redo, the value of counter3 will be added to the counter2 of RowSetMetadata; 5.MinorCompact: No influence since any insertions or deletions come from memory; 6.MajorCompact: No influence, just like step5; 7.Compact: Just like step2. __counter2(persistent) / | __RowSetMetadata __counter3 / / flush1 | | [MRS] ---> [ DRS ... ] -- Undo + Redo + DMS | ^ | \__ |__| counter1 flush2 Change-Id: I2e6378e289bb85024c29e96c2b153fc417ed6412 --- M src/kudu/master/sys_catalog.cc M src/kudu/tablet/compaction-test.cc M src/kudu/tablet/compaction.cc M src/kudu/tablet/delta_tracker.cc M src/kudu/tablet/delta_tracker.h M src/kudu/tablet/deltamemstore-test.cc M src/kudu/tablet/deltamemstore.cc M src/kudu/tablet/deltamemstore.h M src/kudu/tablet/diskrowset-test.cc M src/kudu/tablet/diskrowset.cc M src/kudu/tablet/diskrowset.h M src/kudu/tablet/memrowset-test.cc M src/kudu/tablet/memrowset.cc M src/kudu/tablet/memrowset.h M src/kudu/tablet/metadata.proto M src/kudu/tablet/mock-rowsets.h M src/kudu/tablet/rowset.cc M src/kudu/tablet/rowset.h M src/kudu/tablet/rowset_metadata.cc M src/kudu/tablet/rowset_metadata.h M src/kudu/tablet/tablet-test-util.h M src/kudu/tablet/tablet-test.cc M src/kudu/tablet/tablet.cc M src/kudu/tablet/tablet.h 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/tablet_copy-test-base.h M src/kudu/tserver/tablet_copy_client-test.cc M src/kudu/tserver/tablet_copy_client.cc M src/kudu/tserver/ts_tablet_manager.cc 31 files changed, 392 insertions(+), 33 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/13456/9 -- To view, visit http://gerrit.cloudera.org:8080/13456 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2e6378e289bb85024c29e96c2b153fc417ed6412 Gerrit-Change-Number: 13456 Gerrit-PatchSet: 9 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu
[kudu-CR] [tablet] Support accurate count of rows
Hello Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13456 to look at the new patch set (#8). Change subject: [tablet] Support accurate count of rows .. [tablet] Support accurate count of rows A tablet is consisted of one MRS and a group of DRS. And the group of DRS makes up a RowSetTree. Thus, the number of live rows in a tablet comes from MRS and RowSetTree. At the same time, a new capability is added to the tablet superblock to ensure that only newly created tablets enable the capability. 1.At the beginning, new rows will be written (insert/delete/ reinsert) into MRS and counter1 holds the number of rows; 2.Next, MRS is being flushed to DRS (flush1): When MRS is being flushed, it will be attached to the RowSetTree, so we can count rows there (still counter1); When MRS is flushed to DRS, new RowSetMetadatas will be created and every one has persistent counter2; 3.Then, there will be updates to DRS and they will be accumulated in DMS. counter3 holds the number of rows; 4.If DMS is flushed (flush2): When DMS is being flushed, it will be attached to the Redo list, and its counter3 will be swapped to DeltaTracker; When DMS is flushed to Redo, the value of counter3 will be added to the counter2 of RowSetMetadata; 5.MinorCompact: No influence since any insertions or deletions come from memory; 6.MajorCompact: No influence, just like step5; 7.Compact: Just like step2. __counter2(persistent) / | __RowSetMetadata __counter3 / / flush1 | | [MRS] ---> [ DRS ... ] -- Undo + Redo + DMS | ^ | \__ |__| counter1 flush2 Change-Id: I2e6378e289bb85024c29e96c2b153fc417ed6412 --- M src/kudu/master/sys_catalog.cc M src/kudu/tablet/compaction-test.cc M src/kudu/tablet/compaction.cc M src/kudu/tablet/delta_tracker.cc M src/kudu/tablet/delta_tracker.h M src/kudu/tablet/deltamemstore-test.cc M src/kudu/tablet/deltamemstore.cc M src/kudu/tablet/deltamemstore.h M src/kudu/tablet/diskrowset-test.cc M src/kudu/tablet/diskrowset.cc M src/kudu/tablet/diskrowset.h M src/kudu/tablet/memrowset-test.cc M src/kudu/tablet/memrowset.cc M src/kudu/tablet/memrowset.h M src/kudu/tablet/metadata.proto M src/kudu/tablet/mock-rowsets.h M src/kudu/tablet/rowset.cc M src/kudu/tablet/rowset.h M src/kudu/tablet/rowset_metadata.cc M src/kudu/tablet/rowset_metadata.h M src/kudu/tablet/tablet-test-util.h M src/kudu/tablet/tablet-test.cc M src/kudu/tablet/tablet.cc M src/kudu/tablet/tablet.h 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/tablet_copy-test-base.h M src/kudu/tserver/tablet_copy_client-test.cc M src/kudu/tserver/tablet_copy_client.cc M src/kudu/tserver/ts_tablet_manager.cc 31 files changed, 392 insertions(+), 33 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/13456/8 -- To view, visit http://gerrit.cloudera.org:8080/13456 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2e6378e289bb85024c29e96c2b153fc417ed6412 Gerrit-Change-Number: 13456 Gerrit-PatchSet: 8 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu
[kudu-CR] [tablet] Support accurate count of rows
helifu has posted comments on this change. (
http://gerrit.cloudera.org:8080/13456 )
Change subject: [tablet] Support accurate count of rows
..
Patch Set 7:
(16 comments)
http://gerrit.cloudera.org:8080/#/c/13456/6//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13456/6//COMMIT_MSG@33
PS6, Line 33: 7.Compact:
: Just like step2.
:
: __counter
> You mean rereplicating all the tablets on a tserver? It's not an explicit "
No, I mean a replicated tablet can enable the live row counting feature from an
ancient tablet? As far as I know the replication is done by copying blocks,
regardless of the number of live rows. That will affect the code L361 in
tablet_copy_client.cc.
Ah, I think I got it: "1. If the tablet didn't support live row counting, it
still doesn't after a copy" ^_^
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/compaction-test.cc
File src/kudu/tablet/compaction-test.cc:
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/compaction-test.cc@235
PS7, Line 235: void DeleteRows(RowSet* rowset, int n_rows) {
> This can chain to the new DeleteRows:
Done
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/compaction-test.cc@1209
PS7, Line 1209: ASSERT_NO_FATAL_FAILURE();
> NO_FATALS() in new code.
Done
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/compaction-test.cc@1251
PS7, Line 1251: > >
> >> (the old syntax is pre-C++11)
Done
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/compaction-test.cc@1252
PS7, Line 1252: push_back
> Use emplace_back for all new code. And std::move the DRS shared pointers.
Done
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/delta_tracker.cc
File src/kudu/tablet/delta_tracker.cc:
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/delta_tracker.cc@721
PS7, Line 721: Old
> old
Done
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/diskrowset.h
File src/kudu/tablet/diskrowset.h:
PS7:
> Please update the AppendBlock docs to explain what live_row_count means.
Done
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/metadata.proto
File src/kudu/tablet/metadata.proto:
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/metadata.proto@138
PS7, Line 138: real-time
> Did you mean 'live' here? Not sure what 'real-time' is supposed to convey i
Done
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/metadata.proto@139
PS7, Line 139: // It's only supported for the newly created ones, not for the
ancient ones.
> I'd add that when false, the 'live_row_count' in every RowSetDataPB is inco
Done
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/metadata.proto@140
PS7, Line 140: support_live_row_count
> Nit: supports_live_row_count.
Done
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/rowset_metadata.cc
File src/kudu/tablet/rowset_metadata.cc:
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/rowset_metadata.cc@124
PS7, Line 124: pb.has_live_row_count())
> I don't think you need this second check; the default value of live_row_cou
Done
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/tablet_metadata.h
File src/kudu/tablet/tablet_metadata.h:
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/tablet_metadata.h@84
PS7, Line 84: bool support_live_row_count,
> supports_live_row_count
Done
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/tablet_metadata.cc
File src/kudu/tablet/tablet_metadata.cc:
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/tablet_metadata.cc@154
PS7, Line 154: true
> To clarify the meaning of true/false in call sites, add an annotation:
Done
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/tablet_metadata.cc@272
PS7, Line 272:bool support_live_row_count)
> Do you actually need it to be configurable via the constructor? Couldn't we
No, please look at the code L361 in tablet_copy_client.cc.
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tserver/tablet_copy_client.cc
File src/kudu/tserver/tablet_copy_client.cc:
PS7:
> Can you add a tablet copy test to check that:
Ok, i will try.
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tserver/tablet_copy_client.cc@355
PS7, Line 355: bool is_support_live_row_count = false;
: if (remote_superblock_->has_support_live_row_count()) {
: is_support_live_row_count =
remote_superblock_->support_live_row_count();
: }
> You can just do:
Done
--
To view, visit http://gerrit.cloudera.org:8080/13456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e6378e289bb85024c29e96c2b153fc417ed6412
Gerrit-Change-Number: 13456
[kudu-CR] [tablet] Support accurate count of rows
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/13456 )
Change subject: [tablet] Support accurate count of rows
..
Patch Set 7:
(16 comments)
http://gerrit.cloudera.org:8080/#/c/13456/6//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13456/6//COMMIT_MSG@33
PS6, Line 33: 7.Compact:
: Just like step2.
:
: __counter
> > need to be compacted?) so naturally the user will turn to other
You mean rereplicating all the tablets on a tserver? It's not an explicit
"feature" per se, but you can do it by stopping a tserver and waiting for the
rereplication timeout (default 5 minutes) to elapse. After that, all of the
tserver's replicas will begin rereplication elsewhere.
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/compaction-test.cc
File src/kudu/tablet/compaction-test.cc:
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/compaction-test.cc@235
PS7, Line 235: void DeleteRows(RowSet* rowset, int n_rows) {
This can chain to the new DeleteRows:
DeleteRows(rowset, n_rows, 0);
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/compaction-test.cc@1209
PS7, Line 1209: ASSERT_NO_FATAL_FAILURE();
NO_FATALS() in new code.
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/compaction-test.cc@1251
PS7, Line 1251: > >
>> (the old syntax is pre-C++11)
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/compaction-test.cc@1252
PS7, Line 1252: push_back
Use emplace_back for all new code. And std::move the DRS shared pointers.
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/delta_tracker.cc
File src/kudu/tablet/delta_tracker.cc:
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/delta_tracker.cc@721
PS7, Line 721: Old
old
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/diskrowset.h
File src/kudu/tablet/diskrowset.h:
PS7:
Please update the AppendBlock docs to explain what live_row_count means.
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/metadata.proto
File src/kudu/tablet/metadata.proto:
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/metadata.proto@138
PS7, Line 138: real-time
Did you mean 'live' here? Not sure what 'real-time' is supposed to convey in
context.
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/metadata.proto@139
PS7, Line 139: // It's only supported for the newly created ones, not for the
ancient ones.
I'd add that when false, the 'live_row_count' in every RowSetDataPB is
incorrect and should be ignored.
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/metadata.proto@140
PS7, Line 140: support_live_row_count
Nit: supports_live_row_count.
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/rowset_metadata.cc
File src/kudu/tablet/rowset_metadata.cc:
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/rowset_metadata.cc@124
PS7, Line 124: pb.has_live_row_count())
I don't think you need this second check; the default value of live_row_count()
is 0, and support_live_row_count() is all we should need to know whether
live_row_count() can be trusted.
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/tablet_metadata.h
File src/kudu/tablet/tablet_metadata.h:
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/tablet_metadata.h@84
PS7, Line 84: bool support_live_row_count,
supports_live_row_count
Elsewhere too.
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/tablet_metadata.cc
File src/kudu/tablet/tablet_metadata.cc:
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/tablet_metadata.cc@154
PS7, Line 154: true
To clarify the meaning of true/false in call sites, add an annotation:
/*supports_live_row_count=*/true
Elsewhere too.
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tablet/tablet_metadata.cc@272
PS7, Line 272:bool support_live_row_count)
Do you actually need it to be configurable via the constructor? Couldn't we
always set it to true in the constructor, then get the correct on-disk value
when we call LoadFromSuperBlock?
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tserver/tablet_copy_client.cc
File src/kudu/tserver/tablet_copy_client.cc:
PS7:
Can you add a tablet copy test to check that:
1. If the tablet didn't support live row counting, it still doesn't after a copy
2, If it did, the value is the same after the copy.
http://gerrit.cloudera.org:8080/#/c/13456/7/src/kudu/tserver/tablet_copy_client.cc@355
PS7, Line 355: bool is_support_live_row_count = false;
: if (remote_superblock_->has_support_live_row_count()) {
: is_support_live_row_count =
remote_superblock_->support_live_row_count();
: }
You can just do:
is_support_live_row_count =
[kudu-CR] external mini cluster: bump start process timeout a bit more
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13484 ) Change subject: external_mini_cluster: bump start process timeout a bit more .. external_mini_cluster: bump start process timeout a bit more Commit 08b916d4b did help reduce the occurrence of NTP desynchronization in tests, but not completely. Worse, tests that now do time out do so in a way that doesn't blame NTP at all. For example: Bad status: Timed out: failed to start masters: Unable to start Master at index 0: Timed out after 60.000s waiting for process (...) to write info file (...) This also affects test result reporting, which tries to avoid reporting test runs where NTP failed to synchronize by searching for the "Clock considered unsynchronized" magic string. Of course, that won't happen if the minicluster, thinking that its subprocesses timed out, kills them before they fail that way. Here's a dumb fix: let's increase the start process timeout a bit. That gives the servers more time with which to reach the NTP desynchronization timeout. Change-Id: I95de5368298b9c7fd1ea0b7166dd6c3cd24e356b Reviewed-on: http://gerrit.cloudera.org:8080/13484 Tested-by: Kudu Jenkins Reviewed-by: Will Berkeley --- M src/kudu/mini-cluster/external_mini_cluster.cc M src/kudu/mini-cluster/external_mini_cluster.h 2 files changed, 2 insertions(+), 2 deletions(-) Approvals: Kudu Jenkins: Verified Will Berkeley: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/13484 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I95de5368298b9c7fd1ea0b7166dd6c3cd24e356b Gerrit-Change-Number: 13484 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley
[kudu-CR] external mini cluster: bump start process timeout a bit more
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/13484 ) Change subject: external_mini_cluster: bump start process timeout a bit more .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13484 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I95de5368298b9c7fd1ea0b7166dd6c3cd24e356b Gerrit-Change-Number: 13484 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 03 Jun 2019 22:25:53 + Gerrit-HasComments: No
[kudu-CR] [hms] Support repairing entries with invalid IDs
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13492 ) Change subject: [hms] Support repairing entries with invalid IDs .. [hms] Support repairing entries with invalid IDs This patch updates the hms tool to support fixing HMS tables with missing or invalid kudu.table_id properties. To support this, the patch changes the HMS plugin to allow tables with unmatched IDs to be altered when `kudu.check_id` is false in the environment context. Change-Id: I437a8b42182dcfa09972f8059ccb6af280925085 Reviewed-on: http://gerrit.cloudera.org:8080/13492 Tested-by: Kudu Jenkins Reviewed-by: Hao Hao --- M java/kudu-hive/src/main/java/org/apache/kudu/hive/metastore/KuduMetastorePlugin.java M java/kudu-hive/src/test/java/org/apache/kudu/hive/metastore/TestKuduMetastorePlugin.java M src/kudu/hms/hms_catalog.cc M src/kudu/hms/hms_catalog.h M src/kudu/hms/hms_client.cc M src/kudu/hms/hms_client.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_hms.cc 8 files changed, 84 insertions(+), 18 deletions(-) Approvals: Kudu Jenkins: Verified Hao Hao: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/13492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I437a8b42182dcfa09972f8059ccb6af280925085 Gerrit-Change-Number: 13492 Gerrit-PatchSet: 4 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [hms] Support repairing entries with invalid IDs
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13492 ) Change subject: [hms] Support repairing entries with invalid IDs .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I437a8b42182dcfa09972f8059ccb6af280925085 Gerrit-Change-Number: 13492 Gerrit-PatchSet: 3 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 03 Jun 2019 17:52:40 + Gerrit-HasComments: No
[kudu-CR] docs: update docs for heap sampling
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/13459 ) Change subject: docs: update docs for heap sampling .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13459 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5bdf8b2175f1bc492e98dc2a815bde4ab8fbe626 Gerrit-Change-Number: 13459 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 03 Jun 2019 17:25:01 + Gerrit-HasComments: No
[kudu-CR] docs: update docs for heap sampling
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13459 ) Change subject: docs: update docs for heap sampling .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13459 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5bdf8b2175f1bc492e98dc2a815bde4ab8fbe626 Gerrit-Change-Number: 13459 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 03 Jun 2019 17:24:38 + Gerrit-HasComments: No
[kudu-CR] docs: update docs for heap sampling
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/13459 ) Change subject: docs: update docs for heap sampling .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/13459/1/docs/troubleshooting.adoc File docs/troubleshooting.adoc: http://gerrit.cloudera.org:8080/#/c/13459/1/docs/troubleshooting.adoc@a640 PS1, Line 640: > Should we add a doc on how to disable heap sampling? I don't think so, unless we hear of some case where it caused problems for someone. Otherwise people might start thinking it's a thing that "needs tuning for production" and I don't want people to make false optimizations -- To view, visit http://gerrit.cloudera.org:8080/13459 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5bdf8b2175f1bc492e98dc2a815bde4ab8fbe626 Gerrit-Change-Number: 13459 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 03 Jun 2019 17:23:52 + Gerrit-HasComments: Yes
[kudu-CR] [java] Fix ordering bug in AsyncKuduSession
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/13496 ) Change subject: [java] Fix ordering bug in AsyncKuduSession .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13496 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I498636aeb0de2673c15b90260e6acf56de1ead06 Gerrit-Change-Number: 13496 Gerrit-PatchSet: 2 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 03 Jun 2019 16:52:03 + Gerrit-HasComments: No
[kudu-CR] [java] Micro-improvements to DataGenerator
Will Berkeley has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13498 ) Change subject: [java] Micro-improvements to DataGenerator .. [java] Micro-improvements to DataGenerator Change-Id: I78eca7c6a4d8e0f52fe646a4157774b9123c8a4a Reviewed-on: http://gerrit.cloudera.org:8080/13498 Tested-by: Kudu Jenkins Reviewed-by: Grant Henke --- M java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java 1 file changed, 30 insertions(+), 28 deletions(-) Approvals: Kudu Jenkins: Verified Grant Henke: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/13498 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I78eca7c6a4d8e0f52fe646a4157774b9123c8a4a Gerrit-Change-Number: 13498 Gerrit-PatchSet: 3 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley
[kudu-CR] [backup] Deflake TestKuduBackup.testRandomBackupAndRestore
Will Berkeley has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13499 ) Change subject: [backup] Deflake TestKuduBackup.testRandomBackupAndRestore .. [backup] Deflake TestKuduBackup.testRandomBackupAndRestore TestKuduBackup.testRandomBackupAndRestore was flaky because it the DataGenerator sometimes generated duplicate rows, which caused count mismatches between the number of rows upserted and the number of rows scanned from the table (particularly when the random schema had a single INT8 primary key). This fix works around the problem by eliminating the validation of backups against the generated rows, reducing the set of validations to just the validation of the final state of the table against the restored table. Change-Id: Ic8500f884b05d09a8679863c380dd89f1675519f Reviewed-on: http://gerrit.cloudera.org:8080/13499 Tested-by: Kudu Jenkins Reviewed-by: Grant Henke --- M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala 1 file changed, 9 insertions(+), 5 deletions(-) Approvals: Kudu Jenkins: Verified Grant Henke: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/13499 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ic8500f884b05d09a8679863c380dd89f1675519f Gerrit-Change-Number: 13499 Gerrit-PatchSet: 4 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley
[kudu-CR] [java] Fix ordering bug in AsyncKuduSession
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13496 ) Change subject: [java] Fix ordering bug in AsyncKuduSession .. Patch Set 1: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/13496/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java: http://gerrit.cloudera.org:8080/#/c/13496/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java@306 PS1, Line 306:* Returns a buffer to the inactive queue after flushing. > The buffer is already flushed at this point. This function resets state pos Sounds good. We can commit this as is and do that in follow up work. -- To view, visit http://gerrit.cloudera.org:8080/13496 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I498636aeb0de2673c15b90260e6acf56de1ead06 Gerrit-Change-Number: 13496 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 03 Jun 2019 16:50:29 + Gerrit-HasComments: Yes
[kudu-CR] [java] Deflake TestKuduSession.testInsertAutoFlushBackgroundNonCoveredRange
Will Berkeley has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13497 ) Change subject: [java] Deflake TestKuduSession.testInsertAutoFlushBackgroundNonCoveredRange .. [java] Deflake TestKuduSession.testInsertAutoFlushBackgroundNonCoveredRange The test uses an AsyncKuduSession in AUTO_FLUSH_BACKGROUND mode to send 20 inserts. 10 of them should fail because they are in a non-covered range. However, the test did not join on the deferred when it called AsyncKuduSession#flush. Therefore, there was no guarantee the operations had been completed. So, every now and then, the ops were still in flight when the errors were checked, causing the test to fail because it saw no error where it expected to see errors. Before this fix, the test failed because of this problem about 1/1000 times. I ran 1000 with the fix and saw no failures, but the rate was so low maybe I was lucky. Regardless, I'm confident the test is more correct with this fix. Change-Id: I40360e9c7979c2edf01b4a70b64f71af4d1a1e11 Reviewed-on: http://gerrit.cloudera.org:8080/13497 Tested-by: Will Berkeley Reviewed-by: Grant Henke --- M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduSession.java 1 file changed, 2 insertions(+), 2 deletions(-) Approvals: Will Berkeley: Verified Grant Henke: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/13497 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I40360e9c7979c2edf01b4a70b64f71af4d1a1e11 Gerrit-Change-Number: 13497 Gerrit-PatchSet: 2 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley
[kudu-CR] [java] Deflake TestKuduSession.testInsertAutoFlushBackgroundNonCoveredRange
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13497 ) Change subject: [java] Deflake TestKuduSession.testInsertAutoFlushBackgroundNonCoveredRange .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13497 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I40360e9c7979c2edf01b4a70b64f71af4d1a1e11 Gerrit-Change-Number: 13497 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 03 Jun 2019 16:48:18 + Gerrit-HasComments: No
[kudu-CR] [java] Fix ordering bug in AsyncKuduSession
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/13496 ) Change subject: [java] Fix ordering bug in AsyncKuduSession .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/13496/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java: http://gerrit.cloudera.org:8080/#/c/13496/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java@306 PS1, Line 306:* Returns a buffer to the inactive queue after flushing. > nit: Can you update this comment since the order changed? Maybe: The buffer is already flushed at this point. This function resets state post-flush. I'm thinking about rewriting some of the internals of AsyncKuduSession to make buffer management easier to understand. -- To view, visit http://gerrit.cloudera.org:8080/13496 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I498636aeb0de2673c15b90260e6acf56de1ead06 Gerrit-Change-Number: 13496 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 03 Jun 2019 16:47:42 + Gerrit-HasComments: Yes
[kudu-CR] [java] Micro-improvements to DataGenerator
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13498 ) Change subject: [java] Micro-improvements to DataGenerator .. Patch Set 2: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/13498/2/java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java File java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java: http://gerrit.cloudera.org:8080/#/c/13498/2/java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java@85 PS2, Line 85: continue; > The following two general principles of readable code: I am fine with the change, was just curious the motivation given it was non-functional and minimal impact. -- To view, visit http://gerrit.cloudera.org:8080/13498 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I78eca7c6a4d8e0f52fe646a4157774b9123c8a4a Gerrit-Change-Number: 13498 Gerrit-PatchSet: 2 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 03 Jun 2019 16:45:37 + Gerrit-HasComments: Yes
[kudu-CR] [java] Micro-improvements to DataGenerator
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/13498 ) Change subject: [java] Micro-improvements to DataGenerator .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/13498/2/java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java File java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java: http://gerrit.cloudera.org:8080/#/c/13498/2/java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java@85 PS2, Line 85: continue; > Really curious about the motivation for this. The following two general principles of readable code: 1. Minimize the amount of indentation, because each level of indentation is usually another level of control flow to keep in mind. 2. Early return (or break or continue) is good because it allows the reader to eliminate cases from consideration in the rest of the function or block. Also, I didn't actually mean to submit this patch. I just made this change to help me understand this code better, and then forgot to undo it when submitting the follow-up. -- To view, visit http://gerrit.cloudera.org:8080/13498 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I78eca7c6a4d8e0f52fe646a4157774b9123c8a4a Gerrit-Change-Number: 13498 Gerrit-PatchSet: 2 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 03 Jun 2019 16:44:17 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2791: TTL cache in DNS resolver (part 2)
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/13469 ) Change subject: KUDU-2791: TTL cache in DNS resolver (part 2) .. Patch Set 6: > > IIRC, nscd caches some other things, like user/group info etc. > > I thought that was a downside in some cases. > > > Is nscd cumbersome to use? > > No, but the leaner the install requirements the better. Making the install leaner is a noble goal, agree. Then I think it makes sense to make sure the non-nscd case is no worse if running Kudu with this patch and remove the recommendation about using nscd. -- To view, visit http://gerrit.cloudera.org:8080/13469 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib22e87d28573fdb93ba18cd6d99d8bde7524a4fc Gerrit-Change-Number: 13469 Gerrit-PatchSet: 6 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Mon, 03 Jun 2019 16:34:51 + Gerrit-HasComments: No
[kudu-CR] [java] Fix ordering bug in AsyncKuduSession
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13496 ) Change subject: [java] Fix ordering bug in AsyncKuduSession .. Patch Set 1: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/13496/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java: http://gerrit.cloudera.org:8080/#/c/13496/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java@306 PS1, Line 306:* Returns a buffer to the inactive queue after flushing. nit: Can you update this comment since the order changed? Maybe: Returns a buffer to the inactive queue and flushes the buffer. -- To view, visit http://gerrit.cloudera.org:8080/13496 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I498636aeb0de2673c15b90260e6acf56de1ead06 Gerrit-Change-Number: 13496 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 03 Jun 2019 16:14:14 + Gerrit-HasComments: Yes
[kudu-CR] [java] Micro-improvements to DataGenerator
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13498 ) Change subject: [java] Micro-improvements to DataGenerator .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/13498/2/java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java File java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java: http://gerrit.cloudera.org:8080/#/c/13498/2/java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java@85 PS2, Line 85: continue; Really curious about the motivation for this. -- To view, visit http://gerrit.cloudera.org:8080/13498 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I78eca7c6a4d8e0f52fe646a4157774b9123c8a4a Gerrit-Change-Number: 13498 Gerrit-PatchSet: 2 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Comment-Date: Mon, 03 Jun 2019 16:09:07 + Gerrit-HasComments: Yes
[kudu-CR] [backup] Deflake TestKuduBackup.testRandomBackupAndRestore
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13499 ) Change subject: [backup] Deflake TestKuduBackup.testRandomBackupAndRestore .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13499 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic8500f884b05d09a8679863c380dd89f1675519f Gerrit-Change-Number: 13499 Gerrit-PatchSet: 3 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Comment-Date: Mon, 03 Jun 2019 16:10:11 + Gerrit-HasComments: No
[kudu-CR] KUDU-2791: TTL cache in DNS resolver (part 2)
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13469 ) Change subject: KUDU-2791: TTL cache in DNS resolver (part 2) .. Patch Set 6: > IIRC, nscd caches some other things, like user/group info etc. I thought that was a downside in some cases. > Is nscd cumbersome to use? No, but the leaner the install requirements the better. -- To view, visit http://gerrit.cloudera.org:8080/13469 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib22e87d28573fdb93ba18cd6d99d8bde7524a4fc Gerrit-Change-Number: 13469 Gerrit-PatchSet: 6 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Mon, 03 Jun 2019 16:02:34 + Gerrit-HasComments: No
[kudu-CR] [backup] Deflake TestKuduBackup.testRandomBackupAndRestore
Hello Mike Percy, Kudu Jenkins, Adar Dembo, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13499 to look at the new patch set (#3). Change subject: [backup] Deflake TestKuduBackup.testRandomBackupAndRestore .. [backup] Deflake TestKuduBackup.testRandomBackupAndRestore TestKuduBackup.testRandomBackupAndRestore was flaky because it the DataGenerator sometimes generated duplicate rows, which caused count mismatches between the number of rows upserted and the number of rows scanned from the table (particularly when the random schema had a single INT8 primary key). This fix works around the problem by eliminating the validation of backups against the generated rows, reducing the set of validations to just the validation of the final state of the table against the restored table. Change-Id: Ic8500f884b05d09a8679863c380dd89f1675519f --- M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala 1 file changed, 9 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/99/13499/3 -- To view, visit http://gerrit.cloudera.org:8080/13499 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic8500f884b05d09a8679863c380dd89f1675519f Gerrit-Change-Number: 13499 Gerrit-PatchSet: 3 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy
[kudu-CR] [java] Micro-improvements to DataGenerator
Hello Mike Percy, Kudu Jenkins, Adar Dembo, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13498 to look at the new patch set (#2). Change subject: [java] Micro-improvements to DataGenerator .. [java] Micro-improvements to DataGenerator Change-Id: I78eca7c6a4d8e0f52fe646a4157774b9123c8a4a --- M java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java 1 file changed, 30 insertions(+), 28 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/13498/2 -- To view, visit http://gerrit.cloudera.org:8080/13498 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I78eca7c6a4d8e0f52fe646a4157774b9123c8a4a Gerrit-Change-Number: 13498 Gerrit-PatchSet: 2 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy
[kudu-CR] [hms] Support repairing entries with invalid IDs
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13492 ) Change subject: [hms] Support repairing entries with invalid IDs .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/13492/2/java/kudu-hive/src/main/java/org/apache/kudu/hive/metastore/KuduMetastorePlugin.java File java/kudu-hive/src/main/java/org/apache/kudu/hive/metastore/KuduMetastorePlugin.java: http://gerrit.cloudera.org:8080/#/c/13492/2/java/kudu-hive/src/main/java/org/apache/kudu/hive/metastore/KuduMetastorePlugin.java@320 PS2, Line 320: return Boolean.parseBoolean(properties.get(KUDU_MASTER_EVENT)); > Not your fault, but can you add 'containsKey' check before this? Thanks! Done -- To view, visit http://gerrit.cloudera.org:8080/13492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I437a8b42182dcfa09972f8059ccb6af280925085 Gerrit-Change-Number: 13492 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 03 Jun 2019 14:38:09 + Gerrit-HasComments: Yes
[kudu-CR] [hms] Support repairing entries with invalid IDs
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13492 to look at the new patch set (#3). Change subject: [hms] Support repairing entries with invalid IDs .. [hms] Support repairing entries with invalid IDs This patch updates the hms tool to support fixing HMS tables with missing or invalid kudu.table_id properties. To support this, the patch changes the HMS plugin to allow tables with unmatched IDs to be altered when `kudu.check_id` is false in the environment context. Change-Id: I437a8b42182dcfa09972f8059ccb6af280925085 --- M java/kudu-hive/src/main/java/org/apache/kudu/hive/metastore/KuduMetastorePlugin.java M java/kudu-hive/src/test/java/org/apache/kudu/hive/metastore/TestKuduMetastorePlugin.java M src/kudu/hms/hms_catalog.cc M src/kudu/hms/hms_catalog.h M src/kudu/hms/hms_client.cc M src/kudu/hms/hms_client.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_hms.cc 8 files changed, 84 insertions(+), 18 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/13492/3 -- To view, visit http://gerrit.cloudera.org:8080/13492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I437a8b42182dcfa09972f8059ccb6af280925085 Gerrit-Change-Number: 13492 Gerrit-PatchSet: 3 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [tablet] Support accurate count of rows
Hello Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13456 to look at the new patch set (#7). Change subject: [tablet] Support accurate count of rows .. [tablet] Support accurate count of rows A tablet is consisted of one MRS and a group of DRS. And the group of DRS makes up a RowSetTree. Thus, the number of live rows in a tablet comes from MRS and RowSetTree. At the same time, a new capability is added to the tablet superblock to ensure that only newly created tablets enable the capability. 1.At the beginning, new rows will be written (insert/delete/ reinsert) into MRS and counter1 holds the number of rows; 2.Next, MRS is being flushed to DRS (flush1): When MRS is being flushed, it will be attached to the RowSetTree, so we can count rows there (still counter1); When MRS is flushed to DRS, new RowSetMetadatas will be created and every one has persistent counter2; 3.Then, there will be updates to DRS and they will be accumulated in DMS. counter3 holds the number of rows; 4.If DMS is flushed (flush2): When DMS is being flushed, it will be attached to the Redo list, and its counter3 will be swapped to DeltaTracker; When DMS is flushed to Redo, the value of counter3 will be added to the counter2 of RowSetMetadata; 5.MinorCompact: No influence since any insertions or deletions come from memory; 6.MajorCompact: No influence, just like step5; 7.Compact: Just like step2. __counter2(persistent) / | __RowSetMetadata __counter3 / / flush1 | | [MRS] ---> [ DRS ... ] -- Undo + Redo + DMS | ^ | \__ |__| counter1 flush2 Change-Id: I2e6378e289bb85024c29e96c2b153fc417ed6412 --- M src/kudu/master/sys_catalog.cc M src/kudu/tablet/compaction-test.cc M src/kudu/tablet/compaction.cc M src/kudu/tablet/delta_tracker.cc M src/kudu/tablet/delta_tracker.h M src/kudu/tablet/deltamemstore-test.cc M src/kudu/tablet/deltamemstore.cc M src/kudu/tablet/deltamemstore.h M src/kudu/tablet/diskrowset-test.cc M src/kudu/tablet/diskrowset.cc M src/kudu/tablet/diskrowset.h M src/kudu/tablet/memrowset-test.cc M src/kudu/tablet/memrowset.cc M src/kudu/tablet/memrowset.h M src/kudu/tablet/metadata.proto M src/kudu/tablet/mock-rowsets.h M src/kudu/tablet/rowset.cc M src/kudu/tablet/rowset.h M src/kudu/tablet/rowset_metadata.cc M src/kudu/tablet/rowset_metadata.h M src/kudu/tablet/tablet-test-util.h M src/kudu/tablet/tablet-test.cc M src/kudu/tablet/tablet.cc M src/kudu/tablet/tablet.h 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/tablet_copy_client-test.cc M src/kudu/tserver/tablet_copy_client.cc M src/kudu/tserver/ts_tablet_manager.cc 30 files changed, 378 insertions(+), 31 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/13456/7 -- To view, visit http://gerrit.cloudera.org:8080/13456 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2e6378e289bb85024c29e96c2b153fc417ed6412 Gerrit-Change-Number: 13456 Gerrit-PatchSet: 7 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu
[kudu-CR] [tablet] Support accurate count of rows
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/13456 ) Change subject: [tablet] Support accurate count of rows .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/13456/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13456/6//COMMIT_MSG@33 PS6, Line 33: 8.For the historical tablets: : When a negative number is returned, it means it is a historical : tablet. And a historical tablet can return a positive number of : live rows after compaction and etc. > need to be compacted?) so naturally the user will turn to other > means, such as rereplicating all the tablets from one tserver at a > time. BTW, it doesn't seem to have that feature yet, does it? -- To view, visit http://gerrit.cloudera.org:8080/13456 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2e6378e289bb85024c29e96c2b153fc417ed6412 Gerrit-Change-Number: 13456 Gerrit-PatchSet: 6 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu Gerrit-Comment-Date: Mon, 03 Jun 2019 12:08:22 + Gerrit-HasComments: Yes
[kudu-CR] [tablet] Support accurate count of rows
helifu has posted comments on this change. (
http://gerrit.cloudera.org:8080/13456 )
Change subject: [tablet] Support accurate count of rows
..
Patch Set 6:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/13456/6//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13456/6//COMMIT_MSG@33
PS6, Line 33: 8.For the historical tablets:
: When a negative number is returned, it means it is a historical
: tablet. And a historical tablet can return a positive number of
: live rows after compaction and etc.
> New fields are cheap, especially booleans (~2 bytes). And it might be nice
Done
http://gerrit.cloudera.org:8080/#/c/13456/6/src/kudu/tablet/delta_tracker.h
File src/kudu/tablet/delta_tracker.h:
http://gerrit.cloudera.org:8080/#/c/13456/6/src/kudu/tablet/delta_tracker.h@269
PS6, Line 269: // Count the number of deleted rows in this Tracker.
> The comment suggests that it'll count up the rows in the entire tracker, (i
Done
http://gerrit.cloudera.org:8080/#/c/13456/4/src/kudu/tablet/memrowset.h
File src/kudu/tablet/memrowset.h:
http://gerrit.cloudera.org:8080/#/c/13456/4/src/kudu/tablet/memrowset.h@258
PS4, Line 258: virtual int64_t CountLiveRows() const override {
> Having done this, could you add DCHECKs to the various counting/subtraction
Done
http://gerrit.cloudera.org:8080/#/c/13456/4/src/kudu/tablet/rowset_metadata.h
File src/kudu/tablet/rowset_metadata.h:
http://gerrit.cloudera.org:8080/#/c/13456/4/src/kudu/tablet/rowset_metadata.h@228
PS4, Line 228: // Note: positive number means live rows and negative number
means deleted rows.
> Sorry, I didn't mean to suggest that you should rename methods like these "
Done
--
To view, visit http://gerrit.cloudera.org:8080/13456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e6378e289bb85024c29e96c2b153fc417ed6412
Gerrit-Change-Number: 13456
Gerrit-PatchSet: 6
Gerrit-Owner: helifu
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: helifu
Gerrit-Comment-Date: Mon, 03 Jun 2019 11:59:21 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2348: Pick a random replica in RemoteTablet.java
Hello Will Berkeley, Kudu Jenkins, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12158 to look at the new patch set (#10). Change subject: KUDU-2348: Pick a random replica in RemoteTablet.java .. KUDU-2348: Pick a random replica in RemoteTablet.java In RemoteTablet.java, 'getClosestServerInfo' always returns whichever server ends up last in the map iteration order, if all servers in the cluster is fully remote. This behavior would cause heavy load on a particular server if there are few servers in the cluster. This patch fix the issue by generating a random non-negative integer(randomInt) in RemoteTablet intializer and choose server based on randomInt. The randomInt variable is static so the choice is same across multiple RemoteTablet instances. Change-Id: I3d70e45d4c9532bb32223c10936b4ff8fd99 --- M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java 1 file changed, 25 insertions(+), 18 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/12158/10 -- To view, visit http://gerrit.cloudera.org:8080/12158 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3d70e45d4c9532bb32223c10936b4ff8fd99 Gerrit-Change-Number: 12158 Gerrit-PatchSet: 10 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Reviewer: Yifan Zhang
[kudu-CR] [java] Deflake TestKuduSession.testInsertAutoFlushBackgroundNonCoveredRange
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/13497 ) Change subject: [java] Deflake TestKuduSession.testInsertAutoFlushBackgroundNonCoveredRange .. Patch Set 1: Verified+1 Looks like a (probably benign) race involving log-capturing code and the stack trace collector. Certainly unrelated to this patch. -- To view, visit http://gerrit.cloudera.org:8080/13497 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I40360e9c7979c2edf01b4a70b64f71af4d1a1e11 Gerrit-Change-Number: 13497 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 03 Jun 2019 09:41:31 + Gerrit-HasComments: No
[kudu-CR] [java] Deflake TestKuduSession.testInsertAutoFlushBackgroundNonCoveredRange
Will Berkeley has removed a vote on this change. Change subject: [java] Deflake TestKuduSession.testInsertAutoFlushBackgroundNonCoveredRange .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/13497 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I40360e9c7979c2edf01b4a70b64f71af4d1a1e11 Gerrit-Change-Number: 13497 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley
[kudu-CR] [java] Micro-improvements to DataGenerator
Will Berkeley has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13498 Change subject: [java] Micro-improvements to DataGenerator .. [java] Micro-improvements to DataGenerator Change-Id: I78eca7c6a4d8e0f52fe646a4157774b9123c8a4a --- M java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java 1 file changed, 30 insertions(+), 28 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/13498/1 -- To view, visit http://gerrit.cloudera.org:8080/13498 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I78eca7c6a4d8e0f52fe646a4157774b9123c8a4a Gerrit-Change-Number: 13498 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley
[kudu-CR] [backup] Deflake TestKuduBackup.testRandomBackupAndRestore
Will Berkeley has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13499 Change subject: [backup] Deflake TestKuduBackup.testRandomBackupAndRestore .. [backup] Deflake TestKuduBackup.testRandomBackupAndRestore TestKuduBackup.testRandomBackupAndRestore was flaky because it the DataGenerator sometimes generated duplicate rows, which caused count mismatches between the number of rows upserted and the number of rows scanned from the table (particularly when the random schema had a single INT8 primary key). This fix works around the problem by eliminating the validation of backups against the generated rows, reducing the set of validations to just the validation of the final state of the table against the restored table. Change-Id: Ic8500f884b05d09a8679863c380dd89f1675519f --- M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala 1 file changed, 8 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/99/13499/1 -- To view, visit http://gerrit.cloudera.org:8080/13499 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ic8500f884b05d09a8679863c380dd89f1675519f Gerrit-Change-Number: 13499 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley
[kudu-CR] [java] Deflake TestKuduSession.testInsertAutoFlushBackgroundNonCoveredRange
Will Berkeley has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13497 Change subject: [java] Deflake TestKuduSession.testInsertAutoFlushBackgroundNonCoveredRange .. [java] Deflake TestKuduSession.testInsertAutoFlushBackgroundNonCoveredRange The test uses an AsyncKuduSession in AUTO_FLUSH_BACKGROUND mode to send 20 inserts. 10 of them should fail because they are in a non-covered range. However, the test did not join on the deferred when it called AsyncKuduSession#flush. Therefore, there was no guarantee the operations had been completed. So, every now and then, the ops were still in flight when the errors were checked, causing the test to fail because it saw no error where it expected to see errors. Before this fix, the test failed because of this problem about 1/1000 times. I ran 1000 with the fix and saw no failures, but the rate was so low maybe I was lucky. Regardless, I'm confident the test is more correct with this fix. Change-Id: I40360e9c7979c2edf01b4a70b64f71af4d1a1e11 --- M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduSession.java 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/97/13497/1 -- To view, visit http://gerrit.cloudera.org:8080/13497 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I40360e9c7979c2edf01b4a70b64f71af4d1a1e11 Gerrit-Change-Number: 13497 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley
[kudu-CR] [java] Deflake TestKuduScanner.testDiffScan
Will Berkeley has submitted this change and it was merged. (
http://gerrit.cloudera.org:8080/13495 )
Change subject: [java] Deflake TestKuduScanner.testDiffScan
..
[java] Deflake TestKuduScanner.testDiffScan
I noticed a funny failure in TestKuduScanner.testDiffScan:
21:55:41.681 [DEBUG - New I/O worker #13] (AsyncKuduScanner.java:556) Can not
open scanner
org.apache.kudu.client.NonRecoverableException: snapshot scan start timestamp
is earlier than the ancient history mark. Consider increasing the value of the
configuration parameter --tablet_history_max_age_sec. Snapshot timestamp: P: 0
usec, L: 0 Ancient History Mark: P: 1558907741677535 usec, L: 0 Physical time
difference: -1558907741.678s
The client logs the request it sent, which indeed has a
'snap_start_timestamp' of 0. This timestamp is supposed to come from
incrementing the propagated timestamp received from the client after
some random inserts and mutations. Fortunately, this test has nice
logging: it logs the operations it did and the timestamp:
21:55:40.874 [INFO - Time-limited test] (TestKuduScanner.java:250) Before: {}
21:55:40.874 [INFO - Time-limited test] (TestKuduScanner.java:255) startHT: 0
'generateMutationOperations' could generate 0 mutations, causing this
test to fail. This requires the random number generator to generate 0 3
times in a row when sampling uniformly from {0, 1, 2, 3, 4}, so the odds
of failure due to this defect in the test were only 1/125. I fixed this
by adjusting the number of initial inserts generated so it is always at
least 1.
I ran the test 1250 times with the fix and saw zero failures. Without
the fix, I saw 9/1250 failures.
Change-Id: Iadb46e5dae71724aa8ff88d04c40ef4eaf1ddd2a
Reviewed-on: http://gerrit.cloudera.org:8080/13495
Tested-by: Will Berkeley
Reviewed-by: Alexey Serbin
---
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/RandomUtils.java
2 files changed, 32 insertions(+), 14 deletions(-)
Approvals:
Will Berkeley: Verified
Alexey Serbin: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/13495
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Iadb46e5dae71724aa8ff88d04c40ef4eaf1ddd2a
Gerrit-Change-Number: 13495
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Will Berkeley
[kudu-CR] [java] Deflake TestKuduScanner.testDiffScan
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/13495 ) Change subject: [java] Deflake TestKuduScanner.testDiffScan .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13495 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iadb46e5dae71724aa8ff88d04c40ef4eaf1ddd2a Gerrit-Change-Number: 13495 Gerrit-PatchSet: 2 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 03 Jun 2019 06:54:52 + Gerrit-HasComments: No
[kudu-CR] [java] Fix ordering bug in AsyncKuduSession
Will Berkeley has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13496 Change subject: [java] Fix ordering bug in AsyncKuduSession .. [java] Fix ordering bug in AsyncKuduSession AsyncKuduSession.testBatchErrorCauseSessionStuck occasionally failed because AsyncKuduSession#hasPendingOperations returned true even though the test joined on the deferred results of its two inserts. I tracked this down to a small mis-ordering of lines where the AsyncKuduSession fires a flush notification (from a Netty worker thread) before it actually swaps the just-flushed buffer back to the inactive list, so there was a small window of time when the client thread could observe hasPendingOperations() as true even though there were no pending operations. I ran the test 100 times in dist-test with this fix and saw no failures. I didn't run the test pre-fix through dist-test but the flaky dashboard indicated it was 2-3% flaky. Change-Id: I498636aeb0de2673c15b90260e6acf56de1ead06 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/96/13496/1 -- To view, visit http://gerrit.cloudera.org:8080/13496 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I498636aeb0de2673c15b90260e6acf56de1ead06 Gerrit-Change-Number: 13496 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley
[kudu-CR] [java] Deflake TestKuduScanner.testDiffScan
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/13495 ) Change subject: [java] Deflake TestKuduScanner.testDiffScan .. Patch Set 2: Verified+1 When will the flakes end :'( -- To view, visit http://gerrit.cloudera.org:8080/13495 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iadb46e5dae71724aa8ff88d04c40ef4eaf1ddd2a Gerrit-Change-Number: 13495 Gerrit-PatchSet: 2 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 03 Jun 2019 06:02:43 + Gerrit-HasComments: No
[kudu-CR] [java] Deflake TestKuduScanner.testDiffScan
Will Berkeley has removed a vote on this change. Change subject: [java] Deflake TestKuduScanner.testDiffScan .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/13495 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: Iadb46e5dae71724aa8ff88d04c40ef4eaf1ddd2a Gerrit-Change-Number: 13495 Gerrit-PatchSet: 2 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
