[kudu-CR] deltamemstore: support iteration with snap to exclude
Hello Mike Percy, Grant Henke, Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/11029 to review the following change. Change subject: deltamemstore: support iteration with snap_to_exclude .. deltamemstore: support iteration with snap_to_exclude This commit modifies the DeltaMemStore iterator to take snap_to_exclude into consideration (if it is set) when determining which updates and deletes are relevant to an iterator. I also copied the no-reinsert DCHECK into Update(), as I wasted some time adding reinserts to a test only to learn later (when I added iteration) that the DMS doesn't allow that. Change-Id: I04af3737960f3fcc7b1921a77ff91e1607b7bc47 --- M src/kudu/tablet/deltamemstore-test.cc M src/kudu/tablet/deltamemstore.cc 2 files changed, 146 insertions(+), 39 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/29/11029/1 -- To view, visit http://gerrit.cloudera.org:8080/11029 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I04af3737960f3fcc7b1921a77ff91e1607b7bc47 Gerrit-Change-Number: 11029 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] schema: add is deleted virtual column
Hello Mike Percy, Dan Burkert, Kudu Jenkins, Grant Henke, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10968 to look at the new patch set (#7). Change subject: schema: add is_deleted virtual column .. schema: add is_deleted virtual column This patch introduces a very basic concept of a "virtual column", defined as a static column with a name, type, and defaults. A Kudu subsystem that wishes to interact with a virtual column needs to first figure out if the projection includes it (via ColumnSchema::Equals()). When projected, the virtual column's data will be entirely default; it's the subsystem's responsibility to fill in something meaningful afterwards. This first cut is hardly robust. It remains to be seen how usable this bare-bones abstraction will be; so far it's sufficient for the MemRowSet. Change-Id: I56eb1d44ba8bfbd76d8bb794b8076b695782939e --- M src/kudu/common/schema.cc M src/kudu/common/schema.h M src/kudu/master/catalog_manager.cc M src/kudu/master/master-test.cc 4 files changed, 53 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/68/10968/7 -- To view, visit http://gerrit.cloudera.org:8080/10968 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I56eb1d44ba8bfbd76d8bb794b8076b695782939e Gerrit-Change-Number: 10968 Gerrit-PatchSet: 7 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] memrowset: support iteration with include deleted rows
Hello Mike Percy, Grant Henke, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10929 to look at the new patch set (#5). Change subject: memrowset: support iteration with include_deleted_rows .. memrowset: support iteration with include_deleted_rows This is another piece in the incremental backup puzzle. The idea is that when taking an incremental backup, the scan will include rows that have been deleted. The backup will figure out which rows were deleted by including a special "is deleted" virtual column in the projection. This commit introduces a new iterator option that can be used to include deleted rows, and uses it in the MemRowSet. Change-Id: Ie018043518b437ecc719cf9f87b2c5eea560c9a1 --- M src/kudu/tablet/memrowset-test.cc M src/kudu/tablet/memrowset.cc M src/kudu/tablet/memrowset.h M src/kudu/tablet/rowset.cc M src/kudu/tablet/rowset.h 5 files changed, 77 insertions(+), 31 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/29/10929/5 -- To view, visit http://gerrit.cloudera.org:8080/10929 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie018043518b437ecc719cf9f87b2c5eea560c9a1 Gerrit-Change-Number: 10929 Gerrit-PatchSet: 5 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] memrowset: support iteration with snap to exclude
Hello Tidy Bot, Mike Percy, Grant Henke, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10926 to look at the new patch set (#7). Change subject: memrowset: support iteration with snap_to_exclude .. memrowset: support iteration with snap_to_exclude Following on the commit that introduced the snap_to_exclude iterator option, this commit modifies the MemRowSet iterator to take snap_to_exclude into consideration (if it is set) when determining which rows and mutations are relevant to an iterator. Change-Id: I9be2ab5af4d5223889e4545ae1db7cc0275480ce --- M src/kudu/tablet/diskrowset-test.cc M src/kudu/tablet/memrowset-test.cc M src/kudu/tablet/memrowset.cc M src/kudu/tablet/memrowset.h M src/kudu/tablet/tablet-test-util.h 5 files changed, 144 insertions(+), 28 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/10926/7 -- To view, visit http://gerrit.cloudera.org:8080/10926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9be2ab5af4d5223889e4545ae1db7cc0275480ce Gerrit-Change-Number: 10926 Gerrit-PatchSet: 7 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] memrowset: support iteration with is deleted virtual column
Hello Mike Percy, Dan Burkert, Grant Henke, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10990 to look at the new patch set (#5). Change subject: memrowset: support iteration with is_deleted virtual column .. memrowset: support iteration with is_deleted virtual column This patch rounds out the MemRowSet changes for incremental backups. Taken together, it is now possible to iterate on a specific time range and to learn which rows were deleted during that time range. Change-Id: Ic6b053f5a3696eb9d7c26b8e3d96752f4f87bcd8 --- M src/kudu/tablet/memrowset-test.cc M src/kudu/tablet/memrowset.cc M src/kudu/tablet/memrowset.h 3 files changed, 94 insertions(+), 14 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/10990/5 -- To view, visit http://gerrit.cloudera.org:8080/10990 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic6b053f5a3696eb9d7c26b8e3d96752f4f87bcd8 Gerrit-Change-Number: 10990 Gerrit-PatchSet: 5 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] exactly once writes-itest for faulty disks
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8388 ) Change subject: exactly_once_writes-itest for faulty disks .. Patch Set 8: Verified+1 Unrelated flake -- To view, visit http://gerrit.cloudera.org:8080/8388 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I966b41b71e39827f8f78fcca59a2208f13f71e53 Gerrit-Change-Number: 8388 Gerrit-PatchSet: 8 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Tue, 24 Jul 2018 04:52:06 + Gerrit-HasComments: No
[kudu-CR] schema: add is deleted virtual column
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/10968 )
Change subject: schema: add is_deleted virtual column
..
Patch Set 6:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/10968/6/src/kudu/common/schema.cc
File src/kudu/common/schema.cc:
http://gerrit.cloudera.org:8080/#/c/10968/6/src/kudu/common/schema.cc@140
PS6, Line 140: static const bool kFalse = false;
> Can you comment why the read default should be false? It's a little differe
Sure, will add a comment here.
http://gerrit.cloudera.org:8080/#/c/10968/6/src/kudu/common/schema.cc@145
PS6, Line 145: ,
> Nit: Comment with /* read_default=*/ to match others.
Done
http://gerrit.cloudera.org:8080/#/c/10968/6/src/kudu/common/schema.cc@154
PS6, Line 154: // TODO(adar): C++11 provides a single-arg constructor
> Nit: Do we assign individual names to given TODO's? I was under the impress
We follow clang-tidy's recommendation, which is derived from the Google style
guide:
https://clang.llvm.org/extra/clang-tidy/checks/google-readability-todo.html
It allows for both bug numbers (preferred) as well as usernames.
http://gerrit.cloudera.org:8080/#/c/10968/6/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/10968/6/src/kudu/master/catalog_manager.cc@1302
PS6, Line 1302: if (ContainsKey(Schema::kVirtualColumnNames, col_name)) {
> Would it be more future proof to prevent the $$ pattern in general?
Perhaps, but I'd like to collect more feedback on this approach before I make
that change.
--
To view, visit http://gerrit.cloudera.org:8080/10968
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I56eb1d44ba8bfbd76d8bb794b8076b695782939e
Gerrit-Change-Number: 10968
Gerrit-PatchSet: 6
Gerrit-Owner: Adar Dembo
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Tue, 24 Jul 2018 04:50:55 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2509 fix use-after-free in case of WAL replay error
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10997 ) Change subject: KUDU-2509 fix use-after-free in case of WAL replay error .. KUDU-2509 fix use-after-free in case of WAL replay error Fixed use-after-free mistake in case of a failure to apply a pending commit message from the WAL while bootstrapping a tablet. Also, a repro scenario to expose the use-after-free condition is added. Prior to the fix, the repro scenario would crash with SIGSEGV on Linux or with SIGBUS on OS X (at least for DEBUG builds). Change-Id: I11373b1cc34d9e2e0181bee2d3841b49022218ed Reviewed-on: http://gerrit.cloudera.org:8080/10997 Tested-by: Alexey Serbin Reviewed-by: Adar Dembo --- M src/kudu/tablet/tablet_bootstrap-test.cc M src/kudu/tablet/tablet_bootstrap.cc 2 files changed, 74 insertions(+), 3 deletions(-) Approvals: Alexey Serbin: Verified Adar Dembo: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/10997 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I11373b1cc34d9e2e0181bee2d3841b49022218ed Gerrit-Change-Number: 10997 Gerrit-PatchSet: 6 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot
[kudu-CR] [log-test] clean-up on WAL entry test operations
Alexey Serbin has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/11028 ) Change subject: [log-test] clean-up on WAL entry test operations .. Removed reviewer Kudu Jenkins with the following votes: * Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/11028 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: I01ef7798c63c18cb03df5969fc73bbb0ec65111b Gerrit-Change-Number: 11028 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin
[kudu-CR] [log-test] clean-up on WAL entry test operations
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11028 ) Change subject: [log-test] clean-up on WAL entry test operations .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11028 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I01ef7798c63c18cb03df5969fc73bbb0ec65111b Gerrit-Change-Number: 11028 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 24 Jul 2018 04:36:59 + Gerrit-HasComments: No
[kudu-CR] KUDU-2509 fix use-after-free in case of WAL replay error
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10997 ) Change subject: KUDU-2509 fix use-after-free in case of WAL replay error .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10997 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I11373b1cc34d9e2e0181bee2d3841b49022218ed Gerrit-Change-Number: 10997 Gerrit-PatchSet: 5 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Tue, 24 Jul 2018 04:35:28 + Gerrit-HasComments: No
[kudu-CR] Allow non-HMS compatible tables to be renamed after turning on HMS integration
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11016 ) Change subject: Allow non-HMS compatible tables to be renamed after turning on HMS integration .. Patch Set 2: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/11016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If554efbda06e33b5fdb40565e700bad8a306c143 Gerrit-Change-Number: 11016 Gerrit-PatchSet: 2 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 24 Jul 2018 04:34:19 + Gerrit-HasComments: No
[kudu-CR] Add alter external catalogs flag to table rename tool
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11017 ) Change subject: Add alter_external_catalogs flag to table rename tool .. Patch Set 2: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/11017 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9f5d97f234283b7740df727e085523c7f0b09735 Gerrit-Change-Number: 11017 Gerrit-PatchSet: 2 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 24 Jul 2018 04:33:39 + Gerrit-HasComments: No
[kudu-CR] exactly once writes-itest for faulty disks
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8388 ) Change subject: exactly_once_writes-itest for faulty disks .. Patch Set 8: It still runs and isn't too flaky (ran a thousand runs in release and TSAN)! Just changed some minor things -- To view, visit http://gerrit.cloudera.org:8080/8388 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I966b41b71e39827f8f78fcca59a2208f13f71e53 Gerrit-Change-Number: 8388 Gerrit-PatchSet: 8 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Tue, 24 Jul 2018 03:28:57 + Gerrit-HasComments: No
[kudu-CR] exactly once writes-itest for faulty disks
Hello Tidy Bot, David Ribeiro Alves, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8388 to look at the new patch set (#8). Change subject: exactly_once_writes-itest for faulty disks .. exactly_once_writes-itest for faulty disks This patch adds an ExactlyOnceWrites test that exercises intermittently crashy disks. This serves to not only test disk failures during writes, but also test disk failures during tablet copies, all while maintaining exactly-once semantics. exactly_once_writes-itest is also updated to use std::thread instead of kudu::Thread, and to favor returning Status instead of void. Change-Id: I966b41b71e39827f8f78fcca59a2208f13f71e53 --- M src/kudu/integration-tests/exactly_once_writes-itest.cc M src/kudu/util/test_macros.h 2 files changed, 241 insertions(+), 121 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/88/8388/8 -- To view, visit http://gerrit.cloudera.org:8080/8388 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I966b41b71e39827f8f78fcca59a2208f13f71e53 Gerrit-Change-Number: 8388 Gerrit-PatchSet: 8 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] [log-test] clean-up on WAL entry test operations
Alexey Serbin has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/11028 ) Change subject: [log-test] clean-up on WAL entry test operations .. [log-test] clean-up on WAL entry test operations Change-Id: I01ef7798c63c18cb03df5969fc73bbb0ec65111b --- M src/kudu/consensus/log-test-base.h M src/kudu/consensus/log-test.cc M src/kudu/tablet/tablet_bootstrap-test.cc 3 files changed, 95 insertions(+), 93 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/11028/2 -- To view, visit http://gerrit.cloudera.org:8080/11028 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I01ef7798c63c18cb03df5969fc73bbb0ec65111b Gerrit-Change-Number: 11028 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin
[kudu-CR] [log-test] clean-up on WAL entry test operations
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11028 ) Change subject: [log-test] clean-up on WAL entry test operations .. Patch Set 1: Verified+1 Unrelated flake: org.apache.kudu.backup.TestKuduBackup.Random Backup and Restore -- To view, visit http://gerrit.cloudera.org:8080/11028 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I01ef7798c63c18cb03df5969fc73bbb0ec65111b Gerrit-Change-Number: 11028 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Comment-Date: Tue, 24 Jul 2018 02:56:08 + Gerrit-HasComments: No
[kudu-CR] [log-test] clean-up on WAL entry test operations
Alexey Serbin has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/11028 ) Change subject: [log-test] clean-up on WAL entry test operations .. Removed reviewer Kudu Jenkins with the following votes: * Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/11028 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: I01ef7798c63c18cb03df5969fc73bbb0ec65111b Gerrit-Change-Number: 11028 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
[kudu-CR] [log-test] clean-up on WAL entry test operations
Alexey Serbin has removed Tidy Bot from this change. ( http://gerrit.cloudera.org:8080/11028 ) Change subject: [log-test] clean-up on WAL entry test operations .. Removed reviewer Tidy Bot. -- To view, visit http://gerrit.cloudera.org:8080/11028 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: I01ef7798c63c18cb03df5969fc73bbb0ec65111b Gerrit-Change-Number: 11028 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-2509 fix use-after-free in case of WAL replay error
Alexey Serbin has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/10997 ) Change subject: KUDU-2509 fix use-after-free in case of WAL replay error .. Removed reviewer Kudu Jenkins with the following votes: * Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/10997 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: I11373b1cc34d9e2e0181bee2d3841b49022218ed Gerrit-Change-Number: 10997 Gerrit-PatchSet: 5 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot
[kudu-CR] KUDU-2509 fix use-after-free in case of WAL replay error
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/10997 ) Change subject: KUDU-2509 fix use-after-free in case of WAL replay error .. Patch Set 5: Verified+1 Unrelated flakes in TSAN build: StopTablets/StopTabletITest.TestStoppedTabletsScansGetRedirected/0 org.apache.kudu.backup.TestKuduBackup.classMethod -- To view, visit http://gerrit.cloudera.org:8080/10997 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I11373b1cc34d9e2e0181bee2d3841b49022218ed Gerrit-Change-Number: 10997 Gerrit-PatchSet: 5 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Tue, 24 Jul 2018 02:54:50 + Gerrit-HasComments: No
[kudu-CR] [log-test] clean-up on WAL entry test operations
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11028 Change subject: [log-test] clean-up on WAL entry test operations .. [log-test] clean-up on WAL entry test operations Change-Id: I01ef7798c63c18cb03df5969fc73bbb0ec65111b --- M src/kudu/consensus/log-test-base.h M src/kudu/consensus/log-test.cc M src/kudu/tablet/tablet_bootstrap-test.cc 3 files changed, 85 insertions(+), 82 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/11028/1 -- To view, visit http://gerrit.cloudera.org:8080/11028 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I01ef7798c63c18cb03df5969fc73bbb0ec65111b Gerrit-Change-Number: 11028 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
[kudu-CR] KUDU-2509 fix use-after-free in case of WAL replay error
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/10997 )
Change subject: KUDU-2509 fix use-after-free in case of WAL replay error
..
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10997/4/src/kudu/tablet/tablet_bootstrap-test.cc
File src/kudu/tablet/tablet_bootstrap-test.cc:
http://gerrit.cloudera.org:8080/#/c/10997/4/src/kudu/tablet/tablet_bootstrap-test.cc@707
PS4, Line 707: const auto schema = Schema({ ColumnSchema("key", INT32),
> I think it's not re-factoring LogTestBase just for this tiny thing. Howeve
s/it's not re-factoring/it's not worth re-factoring/
--
To view, visit http://gerrit.cloudera.org:8080/10997
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I11373b1cc34d9e2e0181bee2d3841b49022218ed
Gerrit-Change-Number: 10997
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Tue, 24 Jul 2018 00:52:24 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2509 fix use-after-free in case of WAL replay error
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/10997 )
Change subject: KUDU-2509 fix use-after-free in case of WAL replay error
..
Patch Set 4:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/10997/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/10997/4//COMMIT_MSG@9
PS4, Line 9: an
> Nit: a
Done
http://gerrit.cloudera.org:8080/#/c/10997/4/src/kudu/tablet/tablet_bootstrap-test.cc
File src/kudu/tablet/tablet_bootstrap-test.cc:
http://gerrit.cloudera.org:8080/#/c/10997/4/src/kudu/tablet/tablet_bootstrap-test.cc@699
PS4, Line 699: true
> This is the default value, no? Also, don't we need a NO_FATALS() wrapper ar
It's a good call. Indeed, that's necessary if we want to spot an error: I used
just bland copy-paste technique in this place. I'll add an additional patch to
correct that in other test scenarios in this test.
http://gerrit.cloudera.org:8080/#/c/10997/4/src/kudu/tablet/tablet_bootstrap-test.cc@707
PS4, Line 707: const auto schema = Schema({ ColumnSchema("key", INT32),
> Can you refactor LogTestBase a bit so you don't need to redescribe schema_
I think it's not re-factoring LogTestBase just for this tiny thing. However, I
updated the code of the test so now it's easy to follow the idea behind.
http://gerrit.cloudera.org:8080/#/c/10997/4/src/kudu/tablet/tablet_bootstrap-test.cc@728
PS4, Line 728: TxResultPB* result = mutate_commit->mutable_result();
: OperationResultPB* mutate = result->add_ops();
: MemStoreTargetPB* target = mutate->add_mutated_stores();
: target->set_mrs_id(1);
> Maybe combine into one line?
Done
http://gerrit.cloudera.org:8080/#/c/10997/4/src/kudu/tablet/tablet_bootstrap-test.cc@738
PS4, Line 738: result = insert_commit->mutable_result();
: OperationResultPB* insert = result->add_ops();
: target = insert->add_mutated_stores();
: target->set_mrs_id(1);
> Combine this too?
Done
--
To view, visit http://gerrit.cloudera.org:8080/10997
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I11373b1cc34d9e2e0181bee2d3841b49022218ed
Gerrit-Change-Number: 10997
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Tue, 24 Jul 2018 00:51:29 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2509 fix use-after-free in case of WAL replay error
Hello Tidy Bot, Mike Percy, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10997 to look at the new patch set (#5). Change subject: KUDU-2509 fix use-after-free in case of WAL replay error .. KUDU-2509 fix use-after-free in case of WAL replay error Fixed use-after-free mistake in case of a failure to apply a pending commit message from the WAL while bootstrapping a tablet. Also, a repro scenario to expose the use-after-free condition is added. Prior to the fix, the repro scenario would crash with SIGSEGV on Linux or with SIGBUS on OS X (at least for DEBUG builds). Change-Id: I11373b1cc34d9e2e0181bee2d3841b49022218ed --- M src/kudu/tablet/tablet_bootstrap-test.cc M src/kudu/tablet/tablet_bootstrap.cc 2 files changed, 74 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/97/10997/5 -- To view, visit http://gerrit.cloudera.org:8080/10997 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I11373b1cc34d9e2e0181bee2d3841b49022218ed Gerrit-Change-Number: 10997 Gerrit-PatchSet: 5 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot
[kudu-CR] hms-tool: refactor check tool and combine upgrade and fix
Dan Burkert has posted comments on this change. (
http://gerrit.cloudera.org:8080/11018 )
Change subject: hms-tool: refactor check tool and combine upgrade and fix
..
Patch Set 3:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/11018/2/src/kudu/gutil/strings/escaping.h
File src/kudu/gutil/strings/escaping.h:
http://gerrit.cloudera.org:8080/#/c/11018/2/src/kudu/gutil/strings/escaping.h@145
PS2, Line 145: return CUnescape(source, dest, nullptr);
> warning: use nullptr [modernize-use-nullptr]
Done
http://gerrit.cloudera.org:8080/#/c/11018/2/src/kudu/tools/tool_action_hms.cc
File src/kudu/tools/tool_action_hms.cc:
http://gerrit.cloudera.org:8080/#/c/11018/2/src/kudu/tools/tool_action_hms.cc@78
PS2, Line 78: using std::pair;
> warning: using decl 'map' is unused [misc-unused-using-decls]
Done
http://gerrit.cloudera.org:8080/#/c/11018/2/src/kudu/tools/tool_action_hms.cc@118
PS2, Line 118: shared_ptr kudu_client;
> warning: missing username/bug in TODO [google-readability-todo]
Done
http://gerrit.cloudera.org:8080/#/c/11018/2/src/kudu/tools/tool_action_hms.cc@153
PS2, Line 153: "Kudu table",
> warning: parameter 'out' is unused [misc-unused-parameters]
Done
http://gerrit.cloudera.org:8080/#/c/11018/2/src/kudu/tools/tool_action_hms.cc@170
PS2, Line 170: "HMS database",
> warning: parameter 'out' is unused [misc-unused-parameters]
Done
http://gerrit.cloudera.org:8080/#/c/11018/2/src/kudu/tools/tool_action_hms.cc@194
PS2, Line 194: DataTable table({
> warning: non-const reference parameter 'table_pairs', make it const or use
Done
http://gerrit.cloudera.org:8080/#/c/11018/2/src/kudu/tools/tool_action_hms.cc@484
PS2, Line 484: // This most likely means the database doesn't exist,
but it could be ambiguous.
> warning: do not use 'else' after 'continue' [readability-else-after-return]
Done
http://gerrit.cloudera.org:8080/#/c/11018/2/src/kudu/tools/tool_action_hms.cc@586
PS2, Line 586: return Status::RuntimeError("Failed to fix some catalog
metadata inconsistencies");
> warning: do not use 'else' after 'return' [readability-else-after-return]
Done
--
To view, visit http://gerrit.cloudera.org:8080/11018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f
Gerrit-Change-Number: 11018
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Tue, 24 Jul 2018 00:24:51 +
Gerrit-HasComments: Yes
[kudu-CR] Allow non-HMS compatible tables to be renamed after turning on HMS integration
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11016 ) Change subject: Allow non-HMS compatible tables to be renamed after turning on HMS integration .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/11016/1/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/11016/1/src/kudu/master/catalog_manager.cc@2300 PS1, Line 2300: a t > Nit: drop this Done -- To view, visit http://gerrit.cloudera.org:8080/11016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If554efbda06e33b5fdb40565e700bad8a306c143 Gerrit-Change-Number: 11016 Gerrit-PatchSet: 2 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 24 Jul 2018 00:24:43 + Gerrit-HasComments: Yes
[kudu-CR] Allow non-HMS compatible tables to be renamed after turning on HMS integration
Hello Kudu Jenkins, Adar Dembo, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11016 to look at the new patch set (#2). Change subject: Allow non-HMS compatible tables to be renamed after turning on HMS integration .. Allow non-HMS compatible tables to be renamed after turning on HMS integration This is a bug fix followup to cee17c03bc30037bf2a, which introduced automatic downcasing of table names during new table creation and table renaming. Change-Id: If554efbda06e33b5fdb40565e700bad8a306c143 --- M src/kudu/client/client.h M src/kudu/integration-tests/master_hms-itest.cc M src/kudu/master/catalog_manager.cc 3 files changed, 50 insertions(+), 14 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/11016/2 -- To view, visit http://gerrit.cloudera.org:8080/11016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If554efbda06e33b5fdb40565e700bad8a306c143 Gerrit-Change-Number: 11016 Gerrit-PatchSet: 2 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Add alter external catalogs flag to table rename tool
Hello Kudu Jenkins, Adar Dembo, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11017 to look at the new patch set (#2). Change subject: Add alter_external_catalogs flag to table rename tool .. Add alter_external_catalogs flag to table rename tool It's going to be necessary to be able to alter the Kudu catalog independently of the HMS in order to repair and upgrade tables when the HMS integration is turned on. A follow-up commit will include a test that exercises this flag when the HMS integration is enabled. Change-Id: I9f5d97f234283b7740df727e085523c7f0b09735 --- M src/kudu/client/client.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_common.cc M src/kudu/tools/tool_action_common.h M src/kudu/tools/tool_action_hms.cc M src/kudu/tools/tool_action_table.cc 6 files changed, 26 insertions(+), 15 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/11017/2 -- To view, visit http://gerrit.cloudera.org:8080/11017 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9f5d97f234283b7740df727e085523c7f0b09735 Gerrit-Change-Number: 11017 Gerrit-PatchSet: 2 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Add alter external catalogs flag to table rename tool
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11017 ) Change subject: Add alter_external_catalogs flag to table rename tool .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/11017/1/src/kudu/client/client.h File src/kudu/client/client.h: http://gerrit.cloudera.org:8080/#/c/11017/1/src/kudu/client/client.h@68 PS1, Line 68: } // namespace tools > +1 Done -- To view, visit http://gerrit.cloudera.org:8080/11017 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9f5d97f234283b7740df727e085523c7f0b09735 Gerrit-Change-Number: 11017 Gerrit-PatchSet: 2 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 24 Jul 2018 00:23:49 + Gerrit-HasComments: Yes
[kudu-CR] hms-tool: refactor check tool and combine upgrade and fix
Hello Tidy Bot, Kudu Jenkins, Adar Dembo, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11018 to look at the new patch set (#3). Change subject: hms-tool: refactor check tool and combine upgrade and fix .. hms-tool: refactor check tool and combine upgrade and fix This commit combines the 'hms upgrade' and 'hms fix' tools. The tools previously had overlapping responsibilities for checking some types of inconsistencies. The 'fix' tool now has flags which can control whether it attempts certain types of fixes: --drop_orphan_hms_tables=false --create_missing_hms_tables=true --fix_inconsistent_tables=true --upgrade_hms_tables=true `drop_orphan_hms_tables` defaults to false since it deletes Hive tables, and the tool can not rule-out that they are being used. Additionally, a --dryrun flag is provided in order to print steps that would be taken without actually making modifications. The checking logic has been expanded to account for more potential inconsistencies, and where possible the fix tool now can automatically repair these inconsistencies. Finally, the input prompt and default database functionality has been removed in order to simplify the tool. Instead, the check tool will issue hints including instructions for how to rename tables without a database or with a Hive-incompatible name using the 'kudu table rename_table' tool. We can always add this functionality back in the future if we determine it helps out users. Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f --- M src/kudu/gutil/strings/escaping.h M src/kudu/hms/hms_catalog.cc M src/kudu/hms/hms_catalog.h M src/kudu/tools/CMakeLists.txt M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_hms.cc 6 files changed, 893 insertions(+), 856 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/18/11018/3 -- To view, visit http://gerrit.cloudera.org:8080/11018 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f Gerrit-Change-Number: 11018 Gerrit-PatchSet: 3 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] Add alter external catalogs flag to table rename tool
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11017 ) Change subject: Add alter_external_catalogs flag to table rename tool .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/11017/1/src/kudu/client/client.h File src/kudu/client/client.h: http://gerrit.cloudera.org:8080/#/c/11017/1/src/kudu/client/client.h@68 PS1, Line 68: Status AlterKuduTableOnly(client::KuduClient* kudu_client, > It would be a lot cleaner to have a method in tool_action_common that sets +1 -- To view, visit http://gerrit.cloudera.org:8080/11017 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9f5d97f234283b7740df727e085523c7f0b09735 Gerrit-Change-Number: 11017 Gerrit-PatchSet: 1 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 23 Jul 2018 23:01:03 + Gerrit-HasComments: Yes
[kudu-CR] Allow non-HMS compatible tables to be renamed after turning on HMS integration
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11016 ) Change subject: Allow non-HMS compatible tables to be renamed after turning on HMS integration .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/11016/1/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/11016/1/src/kudu/master/catalog_manager.cc@2300 PS1, Line 2300: the Nit: drop this -- To view, visit http://gerrit.cloudera.org:8080/11016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If554efbda06e33b5fdb40565e700bad8a306c143 Gerrit-Change-Number: 11016 Gerrit-PatchSet: 1 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 23 Jul 2018 22:58:04 + Gerrit-HasComments: Yes
[kudu-CR] hms-tool: replace a few copies with const refs
Dan Burkert has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11015 ) Change subject: hms-tool: replace a few copies with const refs .. hms-tool: replace a few copies with const refs This patch contains no functional changes. Change-Id: Id2ace8469b82c995f6f528aff4322bf4fed22bd2 Reviewed-on: http://gerrit.cloudera.org:8080/11015 Reviewed-by: Hao Hao Tested-by: Kudu Jenkins --- M src/kudu/tools/tool_action_hms.cc 1 file changed, 20 insertions(+), 16 deletions(-) Approvals: Hao Hao: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/11015 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Id2ace8469b82c995f6f528aff4322bf4fed22bd2 Gerrit-Change-Number: 11015 Gerrit-PatchSet: 2 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] exactly once writes-itest for faulty disks
David Ribeiro Alves has posted comments on this change. ( http://gerrit.cloudera.org:8080/8388 ) Change subject: exactly_once_writes-itest for faulty disks .. Patch Set 7: this still worth merging? -- To view, visit http://gerrit.cloudera.org:8080/8388 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I966b41b71e39827f8f78fcca59a2208f13f71e53 Gerrit-Change-Number: 8388 Gerrit-PatchSet: 7 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Mon, 23 Jul 2018 22:46:18 + Gerrit-HasComments: No
[kudu-CR] KUDU-2509 fix use-after-free in case of WAL replay error
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/10997 )
Change subject: KUDU-2509 fix use-after-free in case of WAL replay error
..
Patch Set 4:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/10997/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/10997/4//COMMIT_MSG@9
PS4, Line 9: an
Nit: a
http://gerrit.cloudera.org:8080/#/c/10997/4/src/kudu/tablet/tablet_bootstrap-test.cc
File src/kudu/tablet/tablet_bootstrap-test.cc:
http://gerrit.cloudera.org:8080/#/c/10997/4/src/kudu/tablet/tablet_bootstrap-test.cc@699
PS4, Line 699: true
This is the default value, no? Also, don't we need a NO_FATALS() wrapper around
these calls?
http://gerrit.cloudera.org:8080/#/c/10997/4/src/kudu/tablet/tablet_bootstrap-test.cc@707
PS4, Line 707: const auto schema = Schema({ ColumnSchema("key", INT32),
Can you refactor LogTestBase a bit so you don't need to redescribe schema_ in
its entirety? Maybe a method that returns the column schemas that go into
schema_, then you could add just the one column to it here? Also it'd document
the intent.
http://gerrit.cloudera.org:8080/#/c/10997/4/src/kudu/tablet/tablet_bootstrap-test.cc@728
PS4, Line 728: TxResultPB* result = mutate_commit->mutable_result();
: OperationResultPB* mutate = result->add_ops();
: MemStoreTargetPB* target = mutate->add_mutated_stores();
: target->set_mrs_id(1);
Maybe combine into one line?
mutate_commit()->mutable_result()->add_ops()->add_mutated_stores()->set_mrs_id(1);
http://gerrit.cloudera.org:8080/#/c/10997/4/src/kudu/tablet/tablet_bootstrap-test.cc@738
PS4, Line 738: result = insert_commit->mutable_result();
: OperationResultPB* insert = result->add_ops();
: target = insert->add_mutated_stores();
: target->set_mrs_id(1);
Combine this too?
--
To view, visit http://gerrit.cloudera.org:8080/10997
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I11373b1cc34d9e2e0181bee2d3841b49022218ed
Gerrit-Change-Number: 10997
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Mon, 23 Jul 2018 22:31:03 +
Gerrit-HasComments: Yes
[kudu-CR] hms-tool: replace a few copies with const refs
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11015 ) Change subject: hms-tool: replace a few copies with const refs .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11015 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id2ace8469b82c995f6f528aff4322bf4fed22bd2 Gerrit-Change-Number: 11015 Gerrit-PatchSet: 1 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 23 Jul 2018 22:28:47 + Gerrit-HasComments: No
[kudu-CR] KUDU-2509 fix use-after-free in case of WAL replay error
Hello Tidy Bot, Mike Percy, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10997 to look at the new patch set (#4). Change subject: KUDU-2509 fix use-after-free in case of WAL replay error .. KUDU-2509 fix use-after-free in case of WAL replay error Fixed use-after-free mistake in case of an failure to apply a pending commit message from the WAL while bootstrapping a tablet. Also, a repro scenario to expose the use-after-free condition is added. Prior to the fix, the repro scenario would crash with SIGSEGV on Linux or with SIGBUS on OS X (at least for DEBUG builds). Change-Id: I11373b1cc34d9e2e0181bee2d3841b49022218ed --- M src/kudu/tablet/tablet_bootstrap-test.cc M src/kudu/tablet/tablet_bootstrap.cc 2 files changed, 81 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/97/10997/4 -- To view, visit http://gerrit.cloudera.org:8080/10997 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I11373b1cc34d9e2e0181bee2d3841b49022218ed Gerrit-Change-Number: 10997 Gerrit-PatchSet: 4 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot
[kudu-CR] Add alter external catalogs flag to table rename tool
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11017 ) Change subject: Add alter_external_catalogs flag to table rename tool .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/11017/1/src/kudu/client/client.h File src/kudu/client/client.h: http://gerrit.cloudera.org:8080/#/c/11017/1/src/kudu/client/client.h@68 PS1, Line 68: Status AlterKuduTableOnly(client::KuduClient* kudu_client, It would be a lot cleaner to have a method in tool_action_common that sets the option on a KuduTableAlterer, then only one method needs to be a friend here. -- To view, visit http://gerrit.cloudera.org:8080/11017 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9f5d97f234283b7740df727e085523c7f0b09735 Gerrit-Change-Number: 11017 Gerrit-PatchSet: 1 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 23 Jul 2018 22:20:50 + Gerrit-HasComments: Yes
[kudu-CR] hms-tool: refactor check tool and combine upgrade and fix
Hello Tidy Bot, Kudu Jenkins, Adar Dembo, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11018 to look at the new patch set (#2). Change subject: hms-tool: refactor check tool and combine upgrade and fix .. hms-tool: refactor check tool and combine upgrade and fix This commit combines the 'hms upgrade' and 'hms fix' tools. The tools previously had overlapping responsibilities for checking some types of inconsistencies. The 'fix' tool now has flags which can control whether it attempts certain types of fixes: --drop_orphan_hms_tables=false --create_missing_hms_tables=true --fix_inconsistent_tables=true --upgrade_hms_tables=true `drop_orphan_hms_tables` defaults to false since it deletes Hive tables, and the tool can not rule-out that they are being used. Additionally, a --dryrun flag is provided in order to print steps that would be taken without actually making modifications. The checking logic has been expanded to account for more potential inconsistencies, and where possible the fix tool now can automatically repair these inconsistencies. Finally, the input prompt and default database functionality has been removed in order to simplify the tool. Instead, the check tool will issue hints including instructions for how to rename tables without a database or with a Hive-incompatible name using the 'kudu table rename_table' tool. We can always add this functionality back in the future if we determine it helps out users. Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f --- M src/kudu/client/client.h M src/kudu/gutil/strings/escaping.h M src/kudu/hms/hms_catalog.cc M src/kudu/hms/hms_catalog.h M src/kudu/tools/CMakeLists.txt M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_hms.cc 7 files changed, 891 insertions(+), 852 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/18/11018/2 -- To view, visit http://gerrit.cloudera.org:8080/11018 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f Gerrit-Change-Number: 11018 Gerrit-PatchSet: 2 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR](gh-pages) [site] Add new committers (abukor, sailesh)
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11019 ) Change subject: [site] Add new committers (abukor, sailesh) .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11019 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: comment Gerrit-Change-Id: Icc5e4bfbd22bb6744b8ddd912e1e4607b0794f41 Gerrit-Change-Number: 11019 Gerrit-PatchSet: 2 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Comment-Date: Mon, 23 Jul 2018 21:05:18 + Gerrit-HasComments: No
[kudu-CR](gh-pages) [site] Add new committers (abukor, sailesh)
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/11019 ) Change subject: [site] Add new committers (abukor, sailesh) .. Patch Set 2: Verified+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/11019/1/committers.md File committers.md: http://gerrit.cloudera.org:8080/#/c/11019/1/committers.md@15 PS1, Line 15: | abukor | Attila Bukor | PMC | > Could you add Sailesh to this list as well while you're at it? Done -- To view, visit http://gerrit.cloudera.org:8080/11019 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: comment Gerrit-Change-Id: Icc5e4bfbd22bb6744b8ddd912e1e4607b0794f41 Gerrit-Change-Number: 11019 Gerrit-PatchSet: 2 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Comment-Date: Mon, 23 Jul 2018 21:04:59 + Gerrit-HasComments: Yes
[kudu-CR](gh-pages) [site] Add new committers (abukor, sailesh)
Hello Dan Burkert, Jean-Daniel Cryans, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11019 to look at the new patch set (#2). Change subject: [site] Add new committers (abukor, sailesh) .. [site] Add new committers (abukor, sailesh) Change-Id: Icc5e4bfbd22bb6744b8ddd912e1e4607b0794f41 --- M committers.md 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/19/11019/2 -- To view, visit http://gerrit.cloudera.org:8080/11019 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: newpatchset Gerrit-Change-Id: Icc5e4bfbd22bb6744b8ddd912e1e4607b0794f41 Gerrit-Change-Number: 11019 Gerrit-PatchSet: 2 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans
[kudu-CR](gh-pages) [site] Add new committer (abukor)
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11019 ) Change subject: [site] Add new committer (abukor) .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/11019/1/committers.md File committers.md: http://gerrit.cloudera.org:8080/#/c/11019/1/committers.md@15 PS1, Line 15: | abukor | Attila Bukor | PMC | Could you add Sailesh to this list as well while you're at it? -- To view, visit http://gerrit.cloudera.org:8080/11019 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: comment Gerrit-Change-Id: Icc5e4bfbd22bb6744b8ddd912e1e4607b0794f41 Gerrit-Change-Number: 11019 Gerrit-PatchSet: 1 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Comment-Date: Mon, 23 Jul 2018 21:02:50 + Gerrit-HasComments: Yes
[kudu-CR](gh-pages) [site] Add new committer (abukor)
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/11019 ) Change subject: [site] Add new committer (abukor) .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11019 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: comment Gerrit-Change-Id: Icc5e4bfbd22bb6744b8ddd912e1e4607b0794f41 Gerrit-Change-Number: 11019 Gerrit-PatchSet: 1 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Dan Burkert Gerrit-Comment-Date: Mon, 23 Jul 2018 20:30:40 + Gerrit-HasComments: No
[kudu-CR] Add error handling to Backup and Restore
Tony Foerster has posted comments on this change. ( http://gerrit.cloudera.org:8080/10941 ) Change subject: Add error handling to Backup and Restore .. Patch Set 9: Build failure is not related to this changeset (OptionalSSL/TestRpc.TestClientConnectionMetrics/0) -- To view, visit http://gerrit.cloudera.org:8080/10941 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I104fd604e12b70fcff9beace71ed4ef96b54d691 Gerrit-Change-Number: 10941 Gerrit-PatchSet: 9 Gerrit-Owner: Tony Foerster Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tony Foerster Gerrit-Comment-Date: Mon, 23 Jul 2018 20:17:35 + Gerrit-HasComments: No
[kudu-CR](gh-pages) [site] Add new committer (abukor)
Attila Bukor has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11019 Change subject: [site] Add new committer (abukor) .. [site] Add new committer (abukor) Change-Id: Icc5e4bfbd22bb6744b8ddd912e1e4607b0794f41 --- M committers.md 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/19/11019/1 -- To view, visit http://gerrit.cloudera.org:8080/11019 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: newchange Gerrit-Change-Id: Icc5e4bfbd22bb6744b8ddd912e1e4607b0794f41 Gerrit-Change-Number: 11019 Gerrit-PatchSet: 1 Gerrit-Owner: Attila Bukor
[kudu-CR] hms-tool: refactor check tool and combine upgrade and fix
Hello Adar Dembo, Hao Hao, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/11018 to review the following change. Change subject: hms-tool: refactor check tool and combine upgrade and fix .. hms-tool: refactor check tool and combine upgrade and fix This commit combines the 'hms upgrade' and 'hms fix' tools. The tools previously had overlapping responsibilities for checking some types of inconsistencies. The 'fix' tool now has flags which can control whether it attempts certain types of fixes: --drop_orphan_hms_tables=false --create_missing_hms_tables=true --fix_inconsistent_tables=true --upgrade_hms_tables=true `drop_orphan_hms_tables` defaults to false since it deletes Hive tables, and the tool can not rule-out that they are being used. Additionally, a --dryrun flag is provided in order to print steps that would be taken without actually making modifications. The checking logic has been expanded to account for more potential inconsistencies, and where possible the fix tool now can automatically repair these inconsistencies. Finally, the input prompt and default database functionality has been removed in order to simplify the tool. Instead, the check tool will issue hints including instructions for how to rename tables without a database or with a Hive-incompatible name using the 'kudu table rename_table' tool. We can always add this functionality back in the future if we determine it helps out users. Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f --- M src/kudu/client/client.h M src/kudu/gutil/strings/escaping.h M src/kudu/hms/hms_catalog.cc M src/kudu/hms/hms_catalog.h M src/kudu/tools/CMakeLists.txt M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_hms.cc 7 files changed, 896 insertions(+), 824 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/18/11018/1 -- To view, visit http://gerrit.cloudera.org:8080/11018 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f Gerrit-Change-Number: 11018 Gerrit-PatchSet: 1 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Hao Hao
[kudu-CR] Add alter external catalogs flag to table rename tool
Hello Adar Dembo, Hao Hao, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/11017 to review the following change. Change subject: Add alter_external_catalogs flag to table rename tool .. Add alter_external_catalogs flag to table rename tool It's going to be necessary to be able to alter the Kudu catalog independently of the HMS in order to repair and upgrade tables when the HMS integration is turned on. A follow-up commit will include a test that exercises this flag when the HMS integration is enabled. Change-Id: I9f5d97f234283b7740df727e085523c7f0b09735 --- M src/kudu/client/client.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_table.cc 3 files changed, 30 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/11017/1 -- To view, visit http://gerrit.cloudera.org:8080/11017 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I9f5d97f234283b7740df727e085523c7f0b09735 Gerrit-Change-Number: 11017 Gerrit-PatchSet: 1 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Hao Hao
[kudu-CR] Allow non-HMS compatible tables to be renamed after turning on HMS integration
Hello Adar Dembo, Hao Hao, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/11016 to review the following change. Change subject: Allow non-HMS compatible tables to be renamed after turning on HMS integration .. Allow non-HMS compatible tables to be renamed after turning on HMS integration This is a bug fix followup to cee17c03bc30037bf2a, which introduced automatic downcasing of table names during new table creation and table renaming. Change-Id: If554efbda06e33b5fdb40565e700bad8a306c143 --- M src/kudu/client/client.h M src/kudu/integration-tests/master_hms-itest.cc M src/kudu/master/catalog_manager.cc 3 files changed, 50 insertions(+), 14 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/11016/1 -- To view, visit http://gerrit.cloudera.org:8080/11016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: If554efbda06e33b5fdb40565e700bad8a306c143 Gerrit-Change-Number: 11016 Gerrit-PatchSet: 1 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Hao Hao
[kudu-CR] Add error handling to Backup and Restore
Hello Kudu Jenkins, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10941 to look at the new patch set (#9). Change subject: Add error handling to Backup and Restore .. Add error handling to Backup and Restore A backup/restore will be attempted on every table. If any one table fails the job will exit with a non-zero code. If a table is successfully backed up in the current job or the table metadata has a timestamp up to the current user supplied timestamp, the table will be marked successful. If a table fails to back up any partial data will be removed. If a table fails to restore and a restore Kudu table has already been created, it will not be removed. Spark SaveMode overwrite/append semantics are not handled as part of this commit. Change-Id: I104fd604e12b70fcff9beace71ed4ef96b54d691 --- M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackupOptions.scala M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala 5 files changed, 346 insertions(+), 145 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/10941/9 -- To view, visit http://gerrit.cloudera.org:8080/10941 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I104fd604e12b70fcff9beace71ed4ef96b54d691 Gerrit-Change-Number: 10941 Gerrit-PatchSet: 9 Gerrit-Owner: Tony Foerster Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tony Foerster
[kudu-CR] hms-tool: replace a few copies with const refs
Hello Adar Dembo, Hao Hao, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/11015 to review the following change. Change subject: hms-tool: replace a few copies with const refs .. hms-tool: replace a few copies with const refs This patch contains no functional changes. Change-Id: Id2ace8469b82c995f6f528aff4322bf4fed22bd2 --- M src/kudu/tools/tool_action_hms.cc 1 file changed, 20 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/11015/1 -- To view, visit http://gerrit.cloudera.org:8080/11015 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Id2ace8469b82c995f6f528aff4322bf4fed22bd2 Gerrit-Change-Number: 11015 Gerrit-PatchSet: 1 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Hao Hao
[kudu-CR] Add error handling to Backup and Restore
Tony Foerster has posted comments on this change. (
http://gerrit.cloudera.org:8080/10941 )
Change subject: Add error handling to Backup and Restore
..
Patch Set 8:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/10941/6/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala
File java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala:
http://gerrit.cloudera.org:8080/#/c/10941/6/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala@35
PS6, Line 35: private val cause: Throwable
= null)
> nit: Just None.orNull is a bit unusual. Just null should work.
Done
http://gerrit.cloudera.org:8080/#/c/10941/6/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala@46
PS6, Line 46: // TODO: Make parallel so each table isn't process serially?
> nit: s/vailid/valid
Done
http://gerrit.cloudera.org:8080/#/c/10941/6/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala@57
PS6, Line 57: // Backup for this timestamp has already been
completed.
> I think we should only consider the case where getToMs == options.timestamp
Done
http://gerrit.cloudera.org:8080/#/c/10941/6/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala@111
PS6, Line 111: val out = fs.create(hPath, /* overwrite= */ false)
> nit: extra spaces
Done
http://gerrit.cloudera.org:8080/#/c/10941/6/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala
File java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala:
http://gerrit.cloudera.org:8080/#/c/10941/6/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala@90
PS6, Line 90: val conf = session.sparkContext.hadoopConfiguration
> nit: extra spaces. I think a formatter is aligning the =. This should be fi
right it's my formatter...
http://gerrit.cloudera.org:8080/#/c/10941/6/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala@103
PS6, Line 103:context: KuduContext):
Try[KuduTable] = Try {
> nit: explicit return type.
?? This has an explicit return type
http://gerrit.cloudera.org:8080/#/c/10941/6/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala@115
PS6, Line 115: def main(args: Array[String]): Unit = {
> Where is this used? We definitely don't want the restore process dropping t
It was used when removing partial "${tableName}-restore" tables. I've removed
that part of the code for now but this was left behind. Fixed.
http://gerrit.cloudera.org:8080/#/c/10941/6/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala@136
PS6, Line 136:
> nit: s/note/not
Done
--
To view, visit http://gerrit.cloudera.org:8080/10941
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I104fd604e12b70fcff9beace71ed4ef96b54d691
Gerrit-Change-Number: 10941
Gerrit-PatchSet: 8
Gerrit-Owner: Tony Foerster
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tony Foerster
Gerrit-Comment-Date: Mon, 23 Jul 2018 19:36:21 +
Gerrit-HasComments: Yes
[kudu-CR] Add error handling to Backup and Restore
Hello Kudu Jenkins, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10941 to look at the new patch set (#8). Change subject: Add error handling to Backup and Restore .. Add error handling to Backup and Restore A backup/restore will be attempted on every table. If any one table fails the job will exit with a non-zero code. If a table is successfully backed up in the current job or the table metadata has a timestamp up to the current user supplied timestamp, the table will be marked successful. If a table fails to back up any partial data will be removed. If a table fails to restore and a restore Kudu table has already been created, it will not be removed. Spark SaveMode overwrite/append semantics are not handled as part of this commit. Change-Id: I104fd604e12b70fcff9beace71ed4ef96b54d691 --- A java/kudu-backup/out/test/resources/log4j.properties M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackupOptions.scala M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala 6 files changed, 369 insertions(+), 145 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/10941/8 -- To view, visit http://gerrit.cloudera.org:8080/10941 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I104fd604e12b70fcff9beace71ed4ef96b54d691 Gerrit-Change-Number: 10941 Gerrit-PatchSet: 8 Gerrit-Owner: Tony Foerster Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tony Foerster
[kudu-CR] Add error handling to Backup and Restore
Hello Kudu Jenkins, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10941 to look at the new patch set (#7). Change subject: Add error handling to Backup and Restore .. Add error handling to Backup and Restore A backup/restore will be attempted on every table. If any one table fails the job will exit with a non-zero code. If a table is successfully backed up in the current job or the table metadata has a timestamp up to the current user supplied timestamp, the table will be marked successful. If a table fails to back up any partial data will be removed. If a table fails to restore and a restore Kudu table has already been created, it will not be removed. Spark SaveMode overwrite/append semantics are not handled as part of this commit. Change-Id: I104fd604e12b70fcff9beace71ed4ef96b54d691 --- M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackupOptions.scala M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala 5 files changed, 354 insertions(+), 153 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/10941/7 -- To view, visit http://gerrit.cloudera.org:8080/10941 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I104fd604e12b70fcff9beace71ed4ef96b54d691 Gerrit-Change-Number: 10941 Gerrit-PatchSet: 7 Gerrit-Owner: Tony Foerster Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tony Foerster
[kudu-CR] memrowset: support iteration with snap to exclude
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/10926 ) Change subject: memrowset: support iteration with snap_to_exclude .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/10926/6/src/kudu/tablet/memrowset-test.cc File src/kudu/tablet/memrowset-test.cc: http://gerrit.cloudera.org:8080/#/c/10926/6/src/kudu/tablet/memrowset-test.cc@620 PS6, Line 620: // Captures zero rows; a snapshot range [x, x) does not include anything. > That's illegal, though there's nothing enforcing that today. I'd add enforc That makes sense. Given this is internal I don't think that edge case enforcement is required. -- To view, visit http://gerrit.cloudera.org:8080/10926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9be2ab5af4d5223889e4545ae1db7cc0275480ce Gerrit-Change-Number: 10926 Gerrit-PatchSet: 6 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 23 Jul 2018 18:28:43 + Gerrit-HasComments: Yes
