[kudu-CR] WIP [raft consensus-itest] proper fix for Test KUDU 1735
Hello Mike Percy, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8989 to look at the new patch set (#2). Change subject: WIP [raft_consensus-itest] proper fix for Test_KUDU_1735 .. WIP [raft_consensus-itest] proper fix for Test_KUDU_1735 Adapted logic of the Test_KUDU_1735 scenario to work for both 3-2-3 and 3-4-3 replication schemes. This is a follow-up for 8bcc80eec075321faef26b2a0ccac12ac9d7. WIP: I noticed the RaftConsensusITest.Test_KUDU_1735 started failing after 1277f69a1feb3715750552991bc19444282f652e. Prior to that fix, the test worked without any changes for both 3-2-3 and 3-4-3 modes. Change-Id: Ibd7695f2b78da9755eb109940b40bda2bc6ec02e --- M src/kudu/integration-tests/raft_consensus-itest.cc 1 file changed, 62 insertions(+), 20 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/89/8989/2 -- To view, visit http://gerrit.cloudera.org:8080/8989 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibd7695f2b78da9755eb109940b40bda2bc6ec02e Gerrit-Change-Number: 8989 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey SerbinGerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] [raft consensus-itest] proper fix for Test KUDU 1735
Alexey Serbin has removed Adar Dembo from this change. ( http://gerrit.cloudera.org:8080/8989 ) Change subject: [raft_consensus-itest] proper fix for Test_KUDU_1735 .. Removed reviewer Adar Dembo. -- To view, visit http://gerrit.cloudera.org:8080/8989 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: Ibd7695f2b78da9755eb109940b40bda2bc6ec02e Gerrit-Change-Number: 8989 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey SerbinGerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] KUDU-2202: support for removing data directories (take two)
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8978 ) Change subject: KUDU-2202: support for removing data directories (take two) .. Patch Set 1: (8 comments) http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/fs/data_dirs.h File src/kudu/fs/data_dirs.h: http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/fs/data_dirs.h@220 PS1, Line 220: the DataDirManager is : // opened as per the new contents of of the provided directories but nit: extra "of" Also it wasn't immediately clear what "new contents of the provided directories" was referring to. Maybe consider rewording "...the DataDirManager is opened to reflect the provided directories but..." or something? (in fs_manager.cc too) http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/fs/data_dirs.h@223 PS1, Line 223: bool update_on_disk; nit: should still note the default http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/fs/data_dirs.cc File src/kudu/fs/data_dirs.cc: http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/fs/data_dirs.cc@657 PS1, Line 657: Note: If 'has_missing_instance' is true, opts_.update_on_disk is : // guaranteed to be true. nit: Maybe note that this is due to the ternary parameterization of CheckIntegrity? Or note why this is important, e.g. "...so we can go ahead an update the instance files". http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/fs/data_dirs.cc@805 PS1, Line 805: // This uuid's directory is missing outright, which can happen when : // opts_.read_only and opts_.update_on_disk are both true. So this can only happen when we open our "staging" FsManager and we're missing a directory? I need to ponder this a bit more, not sure I've fully grokked this snippet. http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/fs/fs_manager.h File src/kudu/fs/fs_manager.h: http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/fs/fs_manager.h@102 PS1, Line 102: bool update_on_disk; nit: note the default http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/tools/kudu-tool-test.cc@2232 PS1, Line 2232: ASSERT_OK(mts->WaitStarted()); Could we check that we still have all the tablets we expected? http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/tools/kudu-tool-test.cc@2240 PS1, Line 2240: // Reconfigure the tserver to drop the data directory and restart it. nit: mind adding to the comment something along the lines of, "waiting for the tablets to bootstrap should fail"? WaitStarted() and Start() are so similar, I was a surprised for a second that WaitStarted() would fail until I realized that we're waiting for the bootstraps and not for the FsManager to start up. http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/tools/tool_action_fs.cc File src/kudu/tools/tool_action_fs.cc: http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/tools/tool_action_fs.cc@854 PS1, Line 854: "Updates the set of data directories in an existing Kudu filesystem") : .ExtraDescription("If a data directory is in use by a tablet and is " : "removed, the operation will fail unless --force is also used") : .AddOptionalParameter("fs_wal_dir") : .AddOptionalParameter("fs_data_dirs") : .AddOptionalParameter("force", boost::none, string("If true, permits " : "the removal of a data directory that is configured for use by " : "existing tablets. Those tablets will fail the next time the server " : "is started")) micro-nit: I know in docs we favor being explicit, that local replicas are called replicas and tablets are a logical concept. Does the same apply for tooling? -- To view, visit http://gerrit.cloudera.org:8080/8978 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b Gerrit-Change-Number: 8978 Gerrit-PatchSet: 1 Gerrit-Owner: Adar DemboGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 10 Jan 2018 03:22:04 + Gerrit-HasComments: Yes
[kudu-CR] [raft consensus-itest] proper fix for Test KUDU 1735
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8989 Change subject: [raft_consensus-itest] proper fix for Test_KUDU_1735 .. [raft_consensus-itest] proper fix for Test_KUDU_1735 Adapted logic of the Test_KUDU_1735 scenario to work for both 3-2-3 and 3-4-3 replication schemes. This is a follow-up for 8bcc80eec075321faef26b2a0ccac12ac9d7. Change-Id: Ibd7695f2b78da9755eb109940b40bda2bc6ec02e --- M src/kudu/integration-tests/raft_consensus-itest.cc 1 file changed, 71 insertions(+), 20 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/89/8989/1 -- To view, visit http://gerrit.cloudera.org:8080/8989 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ibd7695f2b78da9755eb109940b40bda2bc6ec02e Gerrit-Change-Number: 8989 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
[kudu-CR] Adding a subsection to NTP troubleshooting section to cover frequent cases of NTP crashes when the Kudu cluster is deployed on Microsoft Azure
Hello Alex Rodoni, Kudu Jenkins, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8960 to look at the new patch set (#3). Change subject: Adding a subsection to NTP troubleshooting section to cover frequent cases of NTP crashes when the Kudu cluster is deployed on Microsoft Azure .. Adding a subsection to NTP troubleshooting section to cover frequent cases of NTP crashes when the Kudu cluster is deployed on Microsoft Azure Change-Id: I5d638aa26137d3a795fce1d0f3f744d4969bdf22 --- M docs/troubleshooting.adoc 1 file changed, 30 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/60/8960/3 -- To view, visit http://gerrit.cloudera.org:8080/8960 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5d638aa26137d3a795fce1d0f3f744d4969bdf22 Gerrit-Change-Number: 8960 Gerrit-PatchSet: 3 Gerrit-Owner: Mahdi AskariGerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Adding a subsection to NTP troubleshooting section to cover frequent cases of NTP crashes when the Kudu cluster is deployed on Microsoft Azure
Hello Alex Rodoni, Kudu Jenkins, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8960 to look at the new patch set (#2). Change subject: Adding a subsection to NTP troubleshooting section to cover frequent cases of NTP crashes when the Kudu cluster is deployed on Microsoft Azure .. Adding a subsection to NTP troubleshooting section to cover frequent cases of NTP crashes when the Kudu cluster is deployed on Microsoft Azure Change-Id: I5d638aa26137d3a795fce1d0f3f744d4969bdf22 --- M docs/troubleshooting.adoc 1 file changed, 30 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/60/8960/2 -- To view, visit http://gerrit.cloudera.org:8080/8960 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5d638aa26137d3a795fce1d0f3f744d4969bdf22 Gerrit-Change-Number: 8960 Gerrit-PatchSet: 2 Gerrit-Owner: Mahdi AskariGerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Adding a subsection to NTP troubleshooting section to cover frequent cases of NTP crashes when the Kudu cluster is deployed on Microsoft Azure
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/8960 ) Change subject: Adding a subsection to NTP troubleshooting section to cover frequent cases of NTP crashes when the Kudu cluster is deployed on Microsoft Azure .. Patch Set 1: (7 comments) http://gerrit.cloudera.org:8080/#/c/8960/1/docs/troubleshooting.adoc File docs/troubleshooting.adoc: http://gerrit.cloudera.org:8080/#/c/8960/1/docs/troubleshooting.adoc@125 PS1, Line 125: NTP Clock on Microsoft Azure Deployments nit: trailing whitespace http://gerrit.cloudera.org:8080/#/c/8960/1/docs/troubleshooting.adoc@127 PS1, Line 127: NTP Clock reports unsynchronized in some cases on Kudu clusters deployed on Microsoft Azure. Following frequent root causes can help troubleshoot the issue. nit: can you wrap this to 80-100 coluns as done elsewhere in the file? http://gerrit.cloudera.org:8080/#/c/8960/1/docs/troubleshooting.adoc@129 PS1, Line 129: https this raw link sitting in the documentation doesn't look good http://gerrit.cloudera.org:8080/#/c/8960/1/docs/troubleshooting.adoc@129 PS1, Line 129: Trun typo http://gerrit.cloudera.org:8080/#/c/8960/1/docs/troubleshooting.adoc@129 PS1, Line 129: CentrifyDC is Centrify Azure-specific? Should this be a separate troubleshooting point? http://gerrit.cloudera.org:8080/#/c/8960/1/docs/troubleshooting.adoc@129 PS1, Line 129: centrifydc.conf can you format file names appropriately using `...`? http://gerrit.cloudera.org:8080/#/c/8960/1/docs/troubleshooting.adoc@135 PS1, Line 135: script how can this script be automated to run at startup on every startup? Seems to require a manual step in there -- To view, visit http://gerrit.cloudera.org:8080/8960 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5d638aa26137d3a795fce1d0f3f744d4969bdf22 Gerrit-Change-Number: 8960 Gerrit-PatchSet: 1 Gerrit-Owner: Mahdi AskariGerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 10 Jan 2018 02:05:46 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2115: avoid compacting already-compacted rowsets
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8859 ) Change subject: KUDU-2115: avoid compacting already-compacted rowsets .. KUDU-2115: avoid compacting already-compacted rowsets Tablets will perform compaction selection on a copy of the currently available rowsets in order to avoid holding the component_lock_ for the duration of rowset selection. It is then verified that nothing else compacted the selected rowsets by iterating over the selected rowsets and checking that they still exist to be compacted. However, the initial selection of rowsets is performed on a snapshotted copy of the available rowsets, and in between the snapshot of the rowsets and the verification, the component_lock_ is dropped, allowing the following race: T1: runs PickRowSetsToCompact, picks {A, B}, begins compaction T2: runs PickRowSetsToCompact: T2: snapshots the rowset tree (which still contains {A, B}) T2: drops component_lock_ after taking the snapshot T1: finishes compaction and removes {A, B} from the rowset tree, unlocking each rowset's compact_flush_lock_ T2: iterates over the rowset tree and sees {A, B} as IsAvailableForCompaction(), since they have been unlocked T2: selects {A, B} as a part of its compaction T2: grabs the component_lock_ to verify that the selected rowsets are still available T2: sees that some of the selected rowsets would not be returned because they are missing from the currently available rowsets (DFATALs and aborts the compaction) I verified the diagnosis by placing a random sleep just after making the copy in PickRowSetsToCompact() and seeing TabletServerDiskFailureTest.TestRandomOpSequence, which schedules many compactions, fail consistently with the failed verification. Upon making the fix, this passes. This can lead to scheduling a fairly useless compaction. This patch adds an API to RowSets to mark itself as compacted, and ensure that when selecting rowsets to compact, check that rowset candidates have not already been compacted. To verify the solution, I've added a small test that schedules several concurrent compactions. Upon adding the aforementioned randomized delay and removing a couple sanity D/CHECKs (including the above verification), I saw the test fail, leading to an unexpected number of rows left in the tablet. Change-Id: I4dab330d61facb18717f6faf179f9b94a9e55236 Reviewed-on: http://gerrit.cloudera.org:8080/8859 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon--- M src/kudu/tablet/compaction-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/mock-rowsets.h M src/kudu/tablet/rowset.h M src/kudu/tablet/tablet.cc 9 files changed, 170 insertions(+), 77 deletions(-) Approvals: Kudu Jenkins: Verified Todd Lipcon: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/8859 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I4dab330d61facb18717f6faf179f9b94a9e55236 Gerrit-Change-Number: 8859 Gerrit-PatchSet: 8 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2115: avoid compacting already-compacted rowsets
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/8859 ) Change subject: KUDU-2115: avoid compacting already-compacted rowsets .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/8859 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4dab330d61facb18717f6faf179f9b94a9e55236 Gerrit-Change-Number: 8859 Gerrit-PatchSet: 7 Gerrit-Owner: Andrew WongGerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 10 Jan 2018 01:58:37 + Gerrit-HasComments: No
[kudu-CR] Fixup DeltaStats::ToString
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/8988 ) Change subject: Fixup DeltaStats::ToString .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/8988 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I75f578fa5a858609107e51779fb4e272f6c57a13 Gerrit-Change-Number: 8988 Gerrit-PatchSet: 1 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 10 Jan 2018 01:56:24 + Gerrit-HasComments: No
[kudu-CR] Fixup DeltaStats::ToString
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8988 ) Change subject: Fixup DeltaStats::ToString .. Fixup DeltaStats::ToString Simplifies the implementation and fixes the mismatched delimiters. Change-Id: I75f578fa5a858609107e51779fb4e272f6c57a13 Reviewed-on: http://gerrit.cloudera.org:8080/8988 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon--- M src/kudu/tablet/delta_stats.cc 1 file changed, 8 insertions(+), 12 deletions(-) Approvals: Kudu Jenkins: Verified Todd Lipcon: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/8988 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I75f578fa5a858609107e51779fb4e272f6c57a13 Gerrit-Change-Number: 8988 Gerrit-PatchSet: 2 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [webui] Make tombstone tablet info less confusing
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/8981 ) Change subject: [webui] Make tombstone tablet info less confusing .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/8981/2/src/kudu/tserver/tserver_path_handlers.cc File src/kudu/tserver/tserver_path_handlers.cc: http://gerrit.cloudera.org:8080/#/c/8981/2/src/kudu/tserver/tserver_path_handlers.cc@211 PS2, Line 211: return replica->HumanReadableState().find("TABLET_DATA_TOMBSTONED") != string::npos; hrm, we can't get at this in a less stringy way? http://gerrit.cloudera.org:8080/#/c/8981/2/src/kudu/tserver/tserver_path_handlers.cc@327 PS2, Line 327: Do not delete them think it would be nice to say something like "The tombstone markers are necessary for correct operation of Kudu. These tablets have had all of their data removed from disk and do not consume significant resources, and must not be deleted." -- To view, visit http://gerrit.cloudera.org:8080/8981 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5c879cc7ff634e5b434fc33374d3010cf1f262cb Gerrit-Change-Number: 8981 Gerrit-PatchSet: 2 Gerrit-Owner: Will BerkeleyGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Wed, 10 Jan 2018 01:55:42 + Gerrit-HasComments: Yes
[kudu-CR] [webui] Make tombstone tablet info less confusing
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/8981 ) Change subject: [webui] Make tombstone tablet info less confusing .. Patch Set 1: > > Build Failed > > Alexey is fixing the last few tests now that 3-4-3 is on. I'll wait > until that's done then kick off a new set of tests. Fixed as of 8bcc80eec075321faef26b2a0ccac12ac9d7 -- To view, visit http://gerrit.cloudera.org:8080/8981 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5c879cc7ff634e5b434fc33374d3010cf1f262cb Gerrit-Change-Number: 8981 Gerrit-PatchSet: 1 Gerrit-Owner: Will BerkeleyGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Wed, 10 Jan 2018 00:32:11 + Gerrit-HasComments: No
[kudu-CR] KUDU-2202: support for removing data directories (take two)
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/8978 ) Change subject: KUDU-2202: support for removing data directories (take two) .. Patch Set 2: Verified+1 The test failures were a batch of clock synchronization errors on one distributed slave. -- To view, visit http://gerrit.cloudera.org:8080/8978 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b Gerrit-Change-Number: 8978 Gerrit-PatchSet: 2 Gerrit-Owner: Adar DemboGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 10 Jan 2018 00:12:00 + Gerrit-HasComments: No
[kudu-CR] KUDU-2202: support for removing data directories (take two)
Adar Dembo has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/8978 ) Change subject: KUDU-2202: support for removing data directories (take two) .. Removed reviewer Kudu Jenkins with the following votes: * Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/8978 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b Gerrit-Change-Number: 8978 Gerrit-PatchSet: 2 Gerrit-Owner: Adar DemboGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Fixup DeltaStats::ToString
Hello Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/8988 to review the following change. Change subject: Fixup DeltaStats::ToString .. Fixup DeltaStats::ToString Simplifies the implementation and fixes the mismatched delimiters. Change-Id: I75f578fa5a858609107e51779fb4e272f6c57a13 --- M src/kudu/tablet/delta_stats.cc 1 file changed, 8 insertions(+), 12 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/88/8988/1 -- To view, visit http://gerrit.cloudera.org:8080/8988 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I75f578fa5a858609107e51779fb4e272f6c57a13 Gerrit-Change-Number: 8988 Gerrit-PatchSet: 1 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Todd Lipcon
[kudu-CR] [raft consensus-itest] fix for Test KUDU 1735
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/8987 ) Change subject: [raft_consensus-itest] fix for Test_KUDU_1735 .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/8987 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8d96b66d528f90f7611ef32726401e3208b0e47d Gerrit-Change-Number: 8987 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey SerbinGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Comment-Date: Tue, 09 Jan 2018 23:19:02 + Gerrit-HasComments: No
[kudu-CR] [raft consensus-itest] fix for Test KUDU 1735
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8987 ) Change subject: [raft_consensus-itest] fix for Test_KUDU_1735 .. [raft_consensus-itest] fix for Test_KUDU_1735 Quick fir for Test_KUDU_1735 after enabling 3-4-3 replication mode by default. For some reason, this test slipped under radar and I discovered that it fails only now (the other relevant tests were already updated). I tried to make this test work for 3-4-3, but it was not successful yet. So, to fix the build, let's push this easy fix first. This is a follow-up for 1ac10d5364db507e5052f5dc13c19d32f62dc4f6. Change-Id: I8d96b66d528f90f7611ef32726401e3208b0e47d Reviewed-on: http://gerrit.cloudera.org:8080/8987 Reviewed-by: Adar DemboTested-by: Alexey Serbin --- M src/kudu/integration-tests/raft_consensus-itest.cc 1 file changed, 6 insertions(+), 0 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Alexey Serbin: Verified -- To view, visit http://gerrit.cloudera.org:8080/8987 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I8d96b66d528f90f7611ef32726401e3208b0e47d Gerrit-Change-Number: 8987 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] [raft consensus-itest] fix for Test KUDU 1735
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/8987 ) Change subject: [raft_consensus-itest] fix for Test_KUDU_1735 .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/8987 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8d96b66d528f90f7611ef32726401e3208b0e47d Gerrit-Change-Number: 8987 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey SerbinGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Comment-Date: Tue, 09 Jan 2018 23:17:41 + Gerrit-HasComments: No
[kudu-CR] [raft consensus-itest] fix for Test KUDU 1735
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8987 Change subject: [raft_consensus-itest] fix for Test_KUDU_1735 .. [raft_consensus-itest] fix for Test_KUDU_1735 Quick fir for Test_KUDU_1735 after enabling 3-4-3 replication mode by default. For some reason, this test slipped under radar and I discovered that it fails only now (the other relevant tests were already updated). I tried to make this test work for 3-4-3, but it was not successful yet. So, to fix the build, let's push this easy fix first. This is a follow-up for 1ac10d5364db507e5052f5dc13c19d32f62dc4f6. Change-Id: I8d96b66d528f90f7611ef32726401e3208b0e47d --- M src/kudu/integration-tests/raft_consensus-itest.cc 1 file changed, 6 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/87/8987/1 -- To view, visit http://gerrit.cloudera.org:8080/8987 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I8d96b66d528f90f7611ef32726401e3208b0e47d Gerrit-Change-Number: 8987 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
[kudu-CR] KUDU-2253 Deltafile on-disk size is 3x larger than expected
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/8982 ) Change subject: KUDU-2253 Deltafile on-disk size is 3x larger than expected .. Patch Set 1: The failures are known issues from another patch series, however it does appear there's a bug in this patch. Setting the flag to true by default makes compaction-test CHECK fail. I'm looking into it. -- To view, visit http://gerrit.cloudera.org:8080/8982 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4cea3371fcf57f89fe10a3b9262bc152023cb04c Gerrit-Change-Number: 8982 Gerrit-PatchSet: 1 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 09 Jan 2018 22:46:38 + Gerrit-HasComments: No
[kudu-CR] [webui] Make tombstone tablet info less confusing
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/8981 ) Change subject: [webui] Make tombstone tablet info less confusing .. Patch Set 1: > Build Failed Alexey is fixing the last few tests now that 3-4-3 is on. I'll wait until that's done then kick off a new set of tests. -- To view, visit http://gerrit.cloudera.org:8080/8981 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5c879cc7ff634e5b434fc33374d3010cf1f262cb Gerrit-Change-Number: 8981 Gerrit-PatchSet: 1 Gerrit-Owner: Will BerkeleyGerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 09 Jan 2018 22:29:11 + Gerrit-HasComments: No
[kudu-CR] [tests] update tests for replication scheme consistency
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8979 ) Change subject: [tests] update tests for replication scheme consistency .. [tests] update tests for replication scheme consistency Fixed and updated some scenarios once the re-replication scheme consistency is enforced: * MasterTest.TestRegisterAndHeartbeat scenario * MasterReplicationTest.TestHeartbeatAcceptedByAnyMaster * MasterCertAuthorityTest.RefuseToSignInvalidCSR * MasterCertAuthorityTest.MasterLeaderSignsCSR This is a follow-up for 1769eed53ee2c21a88a766cb72bf8c9ae622099d. Change-Id: I8f5bbb581abeb3a781b522dff90711b2ffc988df Reviewed-on: http://gerrit.cloudera.org:8080/8979 Reviewed-by: Adar DemboTested-by: Alexey Serbin --- M src/kudu/integration-tests/master_cert_authority-itest.cc M src/kudu/integration-tests/master_replication-itest.cc M src/kudu/master/master-test.cc 3 files changed, 101 insertions(+), 23 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Alexey Serbin: Verified -- To view, visit http://gerrit.cloudera.org:8080/8979 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I8f5bbb581abeb3a781b522dff90711b2ffc988df Gerrit-Change-Number: 8979 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin
[kudu-CR] [tests] update tests for replication scheme consistency
Alexey Serbin has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/8979 ) Change subject: [tests] update tests for replication scheme consistency .. Removed reviewer Kudu Jenkins with the following votes: * Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/8979 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: I8f5bbb581abeb3a781b522dff90711b2ffc988df Gerrit-Change-Number: 8979 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey SerbinGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin
[kudu-CR] [tests] update tests for replication scheme consistency
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/8979 ) Change subject: [tests] update tests for replication scheme consistency .. Patch Set 2: Verified+1 Will fix RaftConsensusITest.Test_KUDU_1735 in a separate changelist. -- To view, visit http://gerrit.cloudera.org:8080/8979 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f5bbb581abeb3a781b522dff90711b2ffc988df Gerrit-Change-Number: 8979 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey SerbinGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Comment-Date: Tue, 09 Jan 2018 21:17:22 + Gerrit-HasComments: No
[kudu-CR] WIP: move fake xml file generation to run-test.sh
Hello Edward Fancher, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/8984 to review the following change. Change subject: WIP: move fake xml file generation to run-test.sh .. WIP: move fake xml file generation to run-test.sh TODO: I still need to test this Change-Id: Iaa89c806039a96ac0a6b7262ded74f70f49f87ac --- M build-support/jenkins/build-and-test.sh M build-support/run-test.sh 2 files changed, 6 insertions(+), 18 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/8984/1 -- To view, visit http://gerrit.cloudera.org:8080/8984 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Iaa89c806039a96ac0a6b7262ded74f70f49f87ac Gerrit-Change-Number: 8984 Gerrit-PatchSet: 1 Gerrit-Owner: Mike PercyGerrit-Reviewer: Edward Fancher
[kudu-CR] KUDU-2233 Add a test case for compactions in the past
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/8885 ) Change subject: KUDU-2233 Add a test case for compactions in the past .. Patch Set 7: (4 comments) http://gerrit.cloudera.org:8080/#/c/8885/7/src/kudu/integration-tests/fuzz-itest.cc File src/kudu/integration-tests/fuzz-itest.cc: http://gerrit.cloudera.org:8080/#/c/8885/7/src/kudu/integration-tests/fuzz-itest.cc@287 PS7, Line 287: it's nit: its http://gerrit.cloudera.org:8080/#/c/8885/7/src/kudu/integration-tests/fuzz-itest.cc@299 PS7, Line 299: DCHECK(last_op); I think CHECK is probably fine here since it's not a perf issue and would make a crash easier to follow http://gerrit.cloudera.org:8080/#/c/8885/7/src/kudu/integration-tests/fuzz-itest.cc@302 PS7, Line 302: while (attempts < max_attempts) { can you use ASSERT_EVENTUALLY for this loop? http://gerrit.cloudera.org:8080/#/c/8885/7/src/kudu/integration-tests/fuzz-itest.cc@1083 PS7, Line 1083: TEST_F(FuzzTest, DISABLED_TestNoCompactionsInThePast) { can you add the JIRA number to a comment here? -- To view, visit http://gerrit.cloudera.org:8080/8885 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibf5685897ef7580dd743faac4d68690a25663e4c Gerrit-Change-Number: 8885 Gerrit-PatchSet: 7 Gerrit-Owner: David Ribeiro AlvesGerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 09 Jan 2018 20:36:06 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2253 Deltafile on-disk size is 3x larger than expected
Hello Will Berkeley, Grant Henke, Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/8982 to review the following change. Change subject: KUDU-2253 Deltafile on-disk size is 3x larger than expected .. KUDU-2253 Deltafile on-disk size is 3x larger than expected While looking into the performance of the integration test written for KUDU-2251 (https://gerrit.cloudera.org/#/c/8951/ revision 6), Todd and I found that the on-disk deltafiles written are about 3x larger than expected. The culprit is an optimization in the CFile value index which is turned off for delta files. The optimization truncates large keys after the first unique byte between sequential values. The deltafile values, in the case of this integration test, include the small DeltaKey, and the 8KiB updated value. As a result the BTree interior nodes are being completely filled by only ~4 values (32KiB cblock size by default). This makes the BTree far less effective, and means that the full updated data is written many times. We expect fixing this will improve performance for update-heavy workloads with large values (for example, YCSB). Enabling the optimization changes the on-disk format of delta files, so we have to proceed in steps. This commit enables deltafile reader compatibility with the optimization, but doesn't yet default to using it while writing delta files. A new experimental flag, deltafile_optimize_index_keys controls whether to write deltafiles with the optimization. We should change the default to true after a waiting a minimum of one release, in order to allow downgrading Kudu one minor release. Testing: I've added basic forwards/backwards compatibility tests. I plan to add a more intensive test of the optimization as part of the integration test in KUDU-2251. Change-Id: I4cea3371fcf57f89fe10a3b9262bc152023cb04c --- M src/kudu/cfile/cfile_util.h M src/kudu/cfile/cfile_writer.cc M src/kudu/tablet/deltafile-test.cc M src/kudu/tablet/deltafile.cc M src/kudu/tablet/deltafile.h 5 files changed, 63 insertions(+), 60 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/82/8982/1 -- To view, visit http://gerrit.cloudera.org:8080/8982 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I4cea3371fcf57f89fe10a3b9262bc152023cb04c Gerrit-Change-Number: 8982 Gerrit-PatchSet: 1 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Grant Henke Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] [webui] Make tombstone tablet info less confusing
Will Berkeley has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8981 Change subject: [webui] Make tombstone tablet info less confusing .. [webui] Make tombstone tablet info less confusing Previously, when a tombstone tablet was reloaded at server startup, the last status message was "Tablet initializing...". This was confusing as it set the expectation that something more was going to happen to the tombstoned tablet. The message is now simply "Tombstoned". Also, now that tombstoned tablets can vote, they retain cmeta despite not participating in non-election Raft operations. Their list of peers is not updated and not usually relevant. It might be confusing to see it on the web ui. This patch suppresses it. Change-Id: I5c879cc7ff634e5b434fc33374d3010cf1f262cb --- M src/kudu/tserver/tablet_server-test.cc M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/tserver_path_handlers.cc 3 files changed, 48 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/81/8981/1 -- To view, visit http://gerrit.cloudera.org:8080/8981 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I5c879cc7ff634e5b434fc33374d3010cf1f262cb Gerrit-Change-Number: 8981 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley
[kudu-CR] [mvcc] Fix watermark advancement in the absence of committed transactions
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8964 ) Change subject: [mvcc] Fix watermark advancement in the absence of committed transactions .. [mvcc] Fix watermark advancement in the absence of committed transactions MvccSnapshot's 'none_committed_at_or_after_' (ncaoa) watermark indicates the point after which no transactions are committed. This is used to cull redo deltas that cannot contain any committed transactions under a snapshot, thus reducing work when applying deltas. By definition, this watermark is never supposed to be lower than the 'all_committed_before_' watermark. If this invariant is broken delta selection might be wrong by skipping whole delta files. Normally the ncaoa watermark is advanced when transactions are marked as committed, in which case this is done correctly. However there is problem when the 'all_committed_before_' watermark is advanced and there are no committed or in flight transactions. In this case the ncaoa watermark might be left behind, manifesting as incorrect delta application/skipping or even crashes. This patch includes a fix and a very directed test that makes sure it is correct. A follow up patch includes a new test in fuzz-itest that would trigger a crash without the fix. Change-Id: Icf94ea289204adf2eed797c3623cdea1b11b09db Reviewed-on: http://gerrit.cloudera.org:8080/8964 Tested-by: David Ribeiro AlvesReviewed-by: Todd Lipcon --- M src/kudu/tablet/mvcc-test.cc M src/kudu/tablet/mvcc.cc M src/kudu/tablet/mvcc.h 3 files changed, 50 insertions(+), 1 deletion(-) Approvals: David Ribeiro Alves: Verified Todd Lipcon: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/8964 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Icf94ea289204adf2eed797c3623cdea1b11b09db Gerrit-Change-Number: 8964 Gerrit-PatchSet: 3 Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Add some DVLOG statements to help in debugging compaction issues
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8886 ) Change subject: Add some DVLOG statements to help in debugging compaction issues .. Add some DVLOG statements to help in debugging compaction issues This adds some DVLOG statemenst that were crucial in debugging KUDU-2233, particularly aroung merge compactions, clock advancement and log gc. Change-Id: I958045a75feceee19c09961de106bbacd343c739 Reviewed-on: http://gerrit.cloudera.org:8080/8886 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon--- M src/kudu/tablet/compaction.cc M src/kudu/tablet/mvcc.cc M src/kudu/tablet/tablet_replica.cc 3 files changed, 21 insertions(+), 2 deletions(-) Approvals: Kudu Jenkins: Verified Todd Lipcon: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/8886 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I958045a75feceee19c09961de106bbacd343c739 Gerrit-Change-Number: 8886 Gerrit-PatchSet: 6 Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [tests] update tests for replication scheme consistency
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/8979 ) Change subject: [tests] update tests for replication scheme consistency .. Patch Set 1: > What about the raft_consensus-itest failure? Is that unrelated? I > saw it locally, and it also cropped up in our nightly tests. > > Here's the output from my local run: > https://gist.github.com/adembo/3b28823884b457f0c4019e9e17a0ab6a. So far I could not attribute the failure in raft_consensus-itest (Test_KUDU_1735) to this stuff. But yes -- I see it fails and I'll fix it. Probably, I will add a separate changelist for that test. -- To view, visit http://gerrit.cloudera.org:8080/8979 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f5bbb581abeb3a781b522dff90711b2ffc988df Gerrit-Change-Number: 8979 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey SerbinGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 09 Jan 2018 20:30:07 + Gerrit-HasComments: No
[kudu-CR] [tests] update tests for replication scheme consistency
Hello Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8979 to look at the new patch set (#2). Change subject: [tests] update tests for replication scheme consistency .. [tests] update tests for replication scheme consistency Fixed and updated some scenarios once the re-replication scheme consistency is enforced: * MasterTest.TestRegisterAndHeartbeat scenario * MasterReplicationTest.TestHeartbeatAcceptedByAnyMaster * MasterCertAuthorityTest.RefuseToSignInvalidCSR * MasterCertAuthorityTest.MasterLeaderSignsCSR This is a follow-up for 1769eed53ee2c21a88a766cb72bf8c9ae622099d. Change-Id: I8f5bbb581abeb3a781b522dff90711b2ffc988df --- M src/kudu/integration-tests/master_cert_authority-itest.cc M src/kudu/integration-tests/master_replication-itest.cc M src/kudu/master/master-test.cc 3 files changed, 101 insertions(+), 23 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/8979/2 -- To view, visit http://gerrit.cloudera.org:8080/8979 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8f5bbb581abeb3a781b522dff90711b2ffc988df Gerrit-Change-Number: 8979 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey SerbinGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [tests] update tests for replication scheme consistency
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/8979 ) Change subject: [tests] update tests for replication scheme consistency .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/8979 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f5bbb581abeb3a781b522dff90711b2ffc988df Gerrit-Change-Number: 8979 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey SerbinGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 09 Jan 2018 20:28:58 + Gerrit-HasComments: No
[kudu-CR] [tests] update tests for replication scheme consistency
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/8979 ) Change subject: [tests] update tests for replication scheme consistency .. Patch Set 1: Code-Review+2 What about the raft_consensus-itest failure? Is that unrelated? I saw it locally, and it also cropped up in our nightly tests. Here's the output from my local run: https://gist.github.com/adembo/3b28823884b457f0c4019e9e17a0ab6a. -- To view, visit http://gerrit.cloudera.org:8080/8979 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f5bbb581abeb3a781b522dff90711b2ffc988df Gerrit-Change-Number: 8979 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey SerbinGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 09 Jan 2018 20:22:58 + Gerrit-HasComments: No
[kudu-CR](branch-1.5.x) Add 'kudu fs list' tool
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/8970 ) Change subject: Add 'kudu fs list' tool .. Patch Set 1: Yep, only issue here was the change of Arena's ctor from 2 args to 1. I just filled in the default that's now in master. -- To view, visit http://gerrit.cloudera.org:8080/8970 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.5.x Gerrit-MessageType: comment Gerrit-Change-Id: I7f5a63e636d95e3ee55bb4955cece7f5d0b7532d Gerrit-Change-Number: 8970 Gerrit-PatchSet: 1 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 09 Jan 2018 20:16:50 + Gerrit-HasComments: No
[kudu-CR](branch-1.6.x) Add 'kudu fs list' tool
Dan Burkert has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8967 ) Change subject: Add 'kudu fs list' tool .. Add 'kudu fs list' tool This tool aims to replace exploratory usages of 'kudu fs dump' and 'kudu local_replica dump' with an improved, unified tool. 'kudu fs list' is more flexible, easier to use, and can show more information. Output is formatted using the DataTable abstraction, which gives it good default pretty-printing, with options to output in CSV and JSON for scripts. Results can easily be filtered to a specific table, tablet, column, rowset, or block using flags. The tool can output many different fields: table, table-id, tablet-id, partition, rowset-id, block-id, block-kind, column, column-id, cfile-data-type, cfile-encoding, cfile-compression, cfile-num-values, cfile-size cfile-incompatible-features, cfile-compatible-features, cfile-min-key, cfile-max-key, and cfile-delta-stats. More fields should be straightforward to add. The tool transparently joins information from tablet superblocks with CFile footers, only materializing the metadata necessary to satisfy the requested fields and filters. Examples: To get our bearings, let's look at what tablets are stored on a local tablet server: ```bash $ kudu fs list --fs-wal-dir=/data/kudu/tserver \ --columns="table, table-id, tablet-id, partition" table | table-id |tablet-id |partition ---+--+--+- loadgen_auto_06c8038c02da40048397e4f6ad1662c3 | 84ff589b979e4f90aa630e7179fcb644 | 2a631714f2d243ff92bf525630baa1ec | HASH (key) PARTITION 7, RANGE (key) PARTITION UNBOUNDED loadgen_auto_fa03aaf7bdf54bb4896c534f38d177a1 | 800af247c1424ecd8e96a37b5ee4d311 | 36827286a00049bc8b242243c6728157 | HASH (key) PARTITION 0, RANGE (key) PARTITION UNBOUNDED loadgen_auto_06c8038c02da40048397e4f6ad1662c3 | 84ff589b979e4f90aa630e7179fcb644 | 3880b30ccebd4ede867febd9c7d5580f | HASH (key) PARTITION 0, RANGE (key) PARTITION UNBOUNDED loadgen_auto_06c8038c02da40048397e4f6ad1662c3 | 84ff589b979e4f90aa630e7179fcb644 | 39436e9e17d84884b1cb689e88b8415f | HASH (key) PARTITION 5, RANGE (key) PARTITION UNBOUNDED loadgen_auto_fa03aaf7bdf54bb4896c534f38d177a1 | 800af247c1424ecd8e96a37b5ee4d311 | 44252efb9aaa4c2c963cf6dd5e875c04 | HASH (key) PARTITION 3, RANGE (key) PARTITION UNBOUNDED loadgen_auto_06c8038c02da40048397e4f6ad1662c3 | 84ff589b979e4f90aa630e7179fcb644 | 57c92ed8391b4d2bbfdeb339f9fb59fd | HASH (key) PARTITION 2, RANGE (key) PARTITION UNBOUNDED loadgen_auto_fa03aaf7bdf54bb4896c534f38d177a1 | 800af247c1424ecd8e96a37b5ee4d311 | 68a64aba5917499ebb7773f16bcd6f6d | HASH (key) PARTITION 7, RANGE (key) PARTITION UNBOUNDED loadgen_auto_06c8038c02da40048397e4f6ad1662c3 | 84ff589b979e4f90aa630e7179fcb644 | 6b5f0729a9bf454791239f77b0912f4e | HASH (key) PARTITION 1, RANGE (key) PARTITION UNBOUNDED loadgen_auto_06c8038c02da40048397e4f6ad1662c3 | 84ff589b979e4f90aa630e7179fcb644 | 8a2d120bd6984144ae963bfe8435206e | HASH (key) PARTITION 4, RANGE (key) PARTITION UNBOUNDED loadgen_auto_fa03aaf7bdf54bb4896c534f38d177a1 | 800af247c1424ecd8e96a37b5ee4d311 | 8b3ba4f415f945849a6a690a142cf1e4 | HASH (key) PARTITION 5, RANGE (key) PARTITION UNBOUNDED loadgen_auto_fa03aaf7bdf54bb4896c534f38d177a1 | 800af247c1424ecd8e96a37b5ee4d311 | 9656be3aa07248a69e3ad6edaa0048cb | HASH (key) PARTITION 1, RANGE (key) PARTITION UNBOUNDED loadgen_auto_fa03aaf7bdf54bb4896c534f38d177a1 | 800af247c1424ecd8e96a37b5ee4d311 | 9e8e444d079842a9b4a83ee9f8bed633 | HASH (key) PARTITION 6, RANGE (key) PARTITION UNBOUNDED loadgen_auto_06c8038c02da40048397e4f6ad1662c3 | 84ff589b979e4f90aa630e7179fcb644 | a794a8e5d3f24e70a96b0beb5a355823 | HASH (key) PARTITION 3, RANGE (key) PARTITION UNBOUNDED loadgen_auto_fa03aaf7bdf54bb4896c534f38d177a1 | 800af247c1424ecd8e96a37b5ee4d311 | bfb8f24b91cd4ecf924aacbb37125041 | HASH (key) PARTITION 2, RANGE (key) PARTITION UNBOUNDED foo | e184a99893b44b17a7b2131123c6de0e | c3ce418c72ab4fea8548387f236dd1fa | loadgen_auto_fa03aaf7bdf54bb4896c534f38d177a1 | 800af247c1424ecd8e96a37b5ee4d311 | e00a284081ca468a994a3609a511e886 | HASH (key) PARTITION 4, RANGE (key) PARTITION UNBOUNDED loadgen_auto_06c8038c02da40048397e4f6ad1662c3 | 84ff589b979e4f90aa630e7179fcb644 | efa22fc899a44bb2a16f620464a15c60 | HASH (key) PARTITION 6, RANGE (key) PARTITION UNBOUNDED ``` The 'foo' table looks interesting; let's drill down into its tablet, and see what rowsets and blocks it has, and some of their associated metadata: ```bash $ kudu fs list --fs-wal-dir=/data/kudu/tserver \ --columns="rowset-id, column, column-id, block-kind,
[kudu-CR](branch-1.5.x) Add 'kudu fs list' tool
Dan Burkert has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8970 ) Change subject: Add 'kudu fs list' tool .. Add 'kudu fs list' tool This tool aims to replace exploratory usages of 'kudu fs dump' and 'kudu local_replica dump' with an improved, unified tool. 'kudu fs list' is more flexible, easier to use, and can show more information. Output is formatted using the DataTable abstraction, which gives it good default pretty-printing, with options to output in CSV and JSON for scripts. Results can easily be filtered to a specific table, tablet, column, rowset, or block using flags. The tool can output many different fields: table, table-id, tablet-id, partition, rowset-id, block-id, block-kind, column, column-id, cfile-data-type, cfile-encoding, cfile-compression, cfile-num-values, cfile-size cfile-incompatible-features, cfile-compatible-features, cfile-min-key, cfile-max-key, and cfile-delta-stats. More fields should be straightforward to add. The tool transparently joins information from tablet superblocks with CFile footers, only materializing the metadata necessary to satisfy the requested fields and filters. Examples: To get our bearings, let's look at what tablets are stored on a local tablet server: ```bash $ kudu fs list --fs-wal-dir=/data/kudu/tserver \ --columns="table, table-id, tablet-id, partition" table | table-id |tablet-id |partition ---+--+--+- loadgen_auto_06c8038c02da40048397e4f6ad1662c3 | 84ff589b979e4f90aa630e7179fcb644 | 2a631714f2d243ff92bf525630baa1ec | HASH (key) PARTITION 7, RANGE (key) PARTITION UNBOUNDED loadgen_auto_fa03aaf7bdf54bb4896c534f38d177a1 | 800af247c1424ecd8e96a37b5ee4d311 | 36827286a00049bc8b242243c6728157 | HASH (key) PARTITION 0, RANGE (key) PARTITION UNBOUNDED loadgen_auto_06c8038c02da40048397e4f6ad1662c3 | 84ff589b979e4f90aa630e7179fcb644 | 3880b30ccebd4ede867febd9c7d5580f | HASH (key) PARTITION 0, RANGE (key) PARTITION UNBOUNDED loadgen_auto_06c8038c02da40048397e4f6ad1662c3 | 84ff589b979e4f90aa630e7179fcb644 | 39436e9e17d84884b1cb689e88b8415f | HASH (key) PARTITION 5, RANGE (key) PARTITION UNBOUNDED loadgen_auto_fa03aaf7bdf54bb4896c534f38d177a1 | 800af247c1424ecd8e96a37b5ee4d311 | 44252efb9aaa4c2c963cf6dd5e875c04 | HASH (key) PARTITION 3, RANGE (key) PARTITION UNBOUNDED loadgen_auto_06c8038c02da40048397e4f6ad1662c3 | 84ff589b979e4f90aa630e7179fcb644 | 57c92ed8391b4d2bbfdeb339f9fb59fd | HASH (key) PARTITION 2, RANGE (key) PARTITION UNBOUNDED loadgen_auto_fa03aaf7bdf54bb4896c534f38d177a1 | 800af247c1424ecd8e96a37b5ee4d311 | 68a64aba5917499ebb7773f16bcd6f6d | HASH (key) PARTITION 7, RANGE (key) PARTITION UNBOUNDED loadgen_auto_06c8038c02da40048397e4f6ad1662c3 | 84ff589b979e4f90aa630e7179fcb644 | 6b5f0729a9bf454791239f77b0912f4e | HASH (key) PARTITION 1, RANGE (key) PARTITION UNBOUNDED loadgen_auto_06c8038c02da40048397e4f6ad1662c3 | 84ff589b979e4f90aa630e7179fcb644 | 8a2d120bd6984144ae963bfe8435206e | HASH (key) PARTITION 4, RANGE (key) PARTITION UNBOUNDED loadgen_auto_fa03aaf7bdf54bb4896c534f38d177a1 | 800af247c1424ecd8e96a37b5ee4d311 | 8b3ba4f415f945849a6a690a142cf1e4 | HASH (key) PARTITION 5, RANGE (key) PARTITION UNBOUNDED loadgen_auto_fa03aaf7bdf54bb4896c534f38d177a1 | 800af247c1424ecd8e96a37b5ee4d311 | 9656be3aa07248a69e3ad6edaa0048cb | HASH (key) PARTITION 1, RANGE (key) PARTITION UNBOUNDED loadgen_auto_fa03aaf7bdf54bb4896c534f38d177a1 | 800af247c1424ecd8e96a37b5ee4d311 | 9e8e444d079842a9b4a83ee9f8bed633 | HASH (key) PARTITION 6, RANGE (key) PARTITION UNBOUNDED loadgen_auto_06c8038c02da40048397e4f6ad1662c3 | 84ff589b979e4f90aa630e7179fcb644 | a794a8e5d3f24e70a96b0beb5a355823 | HASH (key) PARTITION 3, RANGE (key) PARTITION UNBOUNDED loadgen_auto_fa03aaf7bdf54bb4896c534f38d177a1 | 800af247c1424ecd8e96a37b5ee4d311 | bfb8f24b91cd4ecf924aacbb37125041 | HASH (key) PARTITION 2, RANGE (key) PARTITION UNBOUNDED foo | e184a99893b44b17a7b2131123c6de0e | c3ce418c72ab4fea8548387f236dd1fa | loadgen_auto_fa03aaf7bdf54bb4896c534f38d177a1 | 800af247c1424ecd8e96a37b5ee4d311 | e00a284081ca468a994a3609a511e886 | HASH (key) PARTITION 4, RANGE (key) PARTITION UNBOUNDED loadgen_auto_06c8038c02da40048397e4f6ad1662c3 | 84ff589b979e4f90aa630e7179fcb644 | efa22fc899a44bb2a16f620464a15c60 | HASH (key) PARTITION 6, RANGE (key) PARTITION UNBOUNDED ``` The 'foo' table looks interesting; let's drill down into its tablet, and see what rowsets and blocks it has, and some of their associated metadata: ```bash $ kudu fs list --fs-wal-dir=/data/kudu/tserver \ --columns="rowset-id, column, column-id, block-kind,
[kudu-CR] Add some DVLOG statements to help in debugging compaction issues
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/8886 ) Change subject: Add some DVLOG statements to help in debugging compaction issues .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/8886 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I958045a75feceee19c09961de106bbacd343c739 Gerrit-Change-Number: 8886 Gerrit-PatchSet: 5 Gerrit-Owner: David Ribeiro AlvesGerrit-Reviewer: Andrew Wong Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 09 Jan 2018 20:15:14 + Gerrit-HasComments: No
[kudu-CR] [mvcc] Fix watermark advancement in the absence of committed transactions
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/8964 ) Change subject: [mvcc] Fix watermark advancement in the absence of committed transactions .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/8964 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icf94ea289204adf2eed797c3623cdea1b11b09db Gerrit-Change-Number: 8964 Gerrit-PatchSet: 2 Gerrit-Owner: David Ribeiro AlvesGerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 09 Jan 2018 20:14:01 + Gerrit-HasComments: No
[kudu-CR] fs: defer failure from metadata load to bootstrap when data dir is missing
Hello Kudu Jenkins, Andrew Wong, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8376 to look at the new patch set (#3). Change subject: fs: defer failure from metadata load to bootstrap when data dir is missing .. fs: defer failure from metadata load to bootstrap when data dir is missing An upcoming patch adds CLI tool actions to add and remove data directories. When a data dir is removed, all tablets with data on it will fail. Today that failure manifests as a FindOrDie in DataDirGroup::FromPB; we need to make that a little bit more graceful. This patch modifies the DataDirGroup FromPB/CopyToPB methods to return a failure when a data dir is missing. It further changes TabletMetadata to treat such failures as non-fatal, and adds checks to TabletBootstrap so that the failures manifest there instead. No tests in this patch because: 1. Andrew is already working on tablet-level tests for failed disks, and 2. The CLI tool patch adds coverage at the itest-level. Change-Id: I1e8d5697c2bb08287cce11fbdab6fb8d6e37d1ad --- M src/kudu/fs/block_manager-stress-test.cc M src/kudu/fs/block_manager-test.cc M src/kudu/fs/data_dirs-test.cc M src/kudu/fs/data_dirs.cc M src/kudu/fs/data_dirs.h M src/kudu/fs/log_block_manager-test.cc M src/kudu/tablet/tablet_bootstrap.cc M src/kudu/tablet/tablet_metadata.cc M src/kudu/tserver/tablet_copy_client.cc M src/kudu/tserver/ts_tablet_manager.cc 10 files changed, 136 insertions(+), 69 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/8376/3 -- To view, visit http://gerrit.cloudera.org:8080/8376 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1e8d5697c2bb08287cce11fbdab6fb8d6e37d1ad Gerrit-Change-Number: 8376 Gerrit-PatchSet: 3 Gerrit-Owner: Adar DemboGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2202: support for removing data directories (take two)
Hello Andrew Wong, Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/8978 to review the following change. Change subject: KUDU-2202: support for removing data directories (take two) .. KUDU-2202: support for removing data directories (take two) This commit extends "kudu fs update_dirs" to allow the removal of data directories. A data dir may be removed provided there are no tablets configured to use it. The --force flag may be used to override this safety check; tablets that depend on the removed data dir will fail to bootstrap the next time the server is started. I'll be the first to admit that this functionality isn't terribly useful at the moment because Kudu deployments are configured to stripe data across all data directories, so removing a data dir will fail all tablets, which is equivalent to reformatting the node. Nevertheless, it will become more useful as users customize the new --fs_target_data_dirs_per_tablet gflag. This is the second attempt at implementing this functionality. The first used a different interface and was vulnerable to a data loss issue documented in KUDU-2202. Andrew fixed that in commit 47b81c4, which reopens the door for safely removing data directories. I considered a stronger check that only prohibits data dir removal if there were data blocks on the to-be-removed data dir, but decided against it: - It's rare to find a tablet configured to use a data dir but without any actual blocks on it. In fact, that's only likely for empty tables, or tables with empty range partitions. - We'd still need to rewrite any unaffected tablet superblocks and remove the data dir from their data dir groups. There are several interesting modifications here: 1. PathInstanceMetadataFile::CheckIntegrity can now be run without checking for missing instance files. This is used when removing a data dir (its instance file will be missing). 2. The DataDirManager and FsManager may now be constructed with both read_only=true and update_on_disk=true. The CLI tool leverages this functionality to create a "speculative" FsManager using the new data dir configuration that is used to check whether existing tablets depend on the data dir to be removed. 3. The DataDirManager now checks for integrity after making on-disk changes. While not strictly necessary, this seems like a good sanity check to avoid introducing bugs. 4. The "kudu fs update_dirs" action was updated for data dir removal. It now accepts the --force option, without which removal will only be allowed if no tablets are configured to use the removed data dir. Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b --- M src/kudu/fs/block_manager_util-test.cc M src/kudu/fs/block_manager_util.cc M src/kudu/fs/block_manager_util.h M src/kudu/fs/data_dirs.cc M src/kudu/fs/data_dirs.h M src/kudu/fs/fs_manager-test.cc M src/kudu/fs/fs_manager.cc M src/kudu/fs/fs_manager.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_fs.cc 10 files changed, 317 insertions(+), 81 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/78/8978/1 -- To view, visit http://gerrit.cloudera.org:8080/8978 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b Gerrit-Change-Number: 8978 Gerrit-PatchSet: 1 Gerrit-Owner: Adar DemboGerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Todd Lipcon
[kudu-CR](branch-1.6.x) Add 'kudu fs list' tool
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/8967 ) Change subject: Add 'kudu fs list' tool .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/8967 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.6.x Gerrit-MessageType: comment Gerrit-Change-Id: I7f5a63e636d95e3ee55bb4955cece7f5d0b7532d Gerrit-Change-Number: 8967 Gerrit-PatchSet: 1 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 09 Jan 2018 20:10:44 + Gerrit-HasComments: No
[kudu-CR](branch-1.5.x) Add 'kudu fs list' tool
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/8970 ) Change subject: Add 'kudu fs list' tool .. Patch Set 1: Code-Review+2 Looks like a faithful backport -- To view, visit http://gerrit.cloudera.org:8080/8970 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.5.x Gerrit-MessageType: comment Gerrit-Change-Id: I7f5a63e636d95e3ee55bb4955cece7f5d0b7532d Gerrit-Change-Number: 8970 Gerrit-PatchSet: 1 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 09 Jan 2018 20:09:20 + Gerrit-HasComments: No
[kudu-CR] fs: defer failure from metadata load to bootstrap when data dir is missing
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/8376 ) Change subject: fs: defer failure from metadata load to bootstrap when data dir is missing .. Patch Set 2: (6 comments) http://gerrit.cloudera.org:8080/#/c/8376/2/src/kudu/fs/data_dirs.h File src/kudu/fs/data_dirs.h: http://gerrit.cloudera.org:8080/#/c/8376/2/src/kudu/fs/data_dirs.h@81 PS2, Line 81: Status LoadFromPB(const UuidIndexByUuidMap& uuid_idx_by_uuid, > worth short docs explaining the cases of bad-status here. Done http://gerrit.cloudera.org:8080/#/c/8376/2/src/kudu/fs/data_dirs.h@84 PS2, Line 84: Status CopyToPB(const UuidByUuidIndexMap& uuid_by_uuid_idx, > and here Done http://gerrit.cloudera.org:8080/#/c/8376/2/src/kudu/fs/data_dirs.h@276 PS2, Line 276: data dir : // is missing. > only looked at the headers so far (not the impl) but this left me a little Given my response to your other comment (in the impl), is this still relevant? http://gerrit.cloudera.org:8080/#/c/8376/2/src/kudu/fs/data_dirs.cc File src/kudu/fs/data_dirs.cc: http://gerrit.cloudera.org:8080/#/c/8376/2/src/kudu/fs/data_dirs.cc@318 PS2, Line 318: if (!FindCopy(uuid_by_uuid_idx, uuid_idx, )) { : return Status::NotFound(Substitute( : "could not find data dir with uuid index $0", uuid_idx)); > would this not be a programmer error? how would you end up with a uuid_idx It is, but I thought the symmetry with LoadFromPB was worthwhile because it makes the class' behavior more predictable. http://gerrit.cloudera.org:8080/#/c/8376/2/src/kudu/fs/data_dirs.cc@831 PS2, Line 831: RETURN_NOT_OK(group->CopyToPB(uuid_by_idx_, pb)); > I guess my confusion in the header was wrong, but per my comments above, I Indeed, but since I want CopyToPB to be symmetric with LoadFromPB, a RETURN_NOT_OK here seemed more appropriate. http://gerrit.cloudera.org:8080/#/c/8376/2/src/kudu/tablet/tablet_metadata.cc File src/kudu/tablet/tablet_metadata.cc: http://gerrit.cloudera.org:8080/#/c/8376/2/src/kudu/tablet/tablet_metadata.cc@440 PS2, Line 440: DeleteOrphanedBlocks(orphaned_blocks); > pondering this a bit more, I wonder whether we need to actually start stori This was a legitimate issue that Andrew has since fixed in commit 47b81c452194e75da7fd966f07766de4bdcdeab0. -- To view, visit http://gerrit.cloudera.org:8080/8376 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1e8d5697c2bb08287cce11fbdab6fb8d6e37d1ad Gerrit-Change-Number: 8376 Gerrit-PatchSet: 2 Gerrit-Owner: Adar DemboGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 09 Jan 2018 20:02:40 + Gerrit-HasComments: Yes
[kudu-CR] fs: defer failure from metadata load to bootstrap when data dir is missing
Adar Dembo has restored this change. ( http://gerrit.cloudera.org:8080/8376 ) Change subject: fs: defer failure from metadata load to bootstrap when data dir is missing .. Restored -- To view, visit http://gerrit.cloudera.org:8080/8376 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: restore Gerrit-Change-Id: I1e8d5697c2bb08287cce11fbdab6fb8d6e37d1ad Gerrit-Change-Number: 8376 Gerrit-PatchSet: 2 Gerrit-Owner: Adar DemboGerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR](gh-pages) docs refresh
Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8974 Change subject: docs refresh .. docs refresh Change-Id: I07d899da6ea7107f1e01c82d88b1a32583b57a54 --- M docs/administration.html M docs/background_tasks.html M docs/command_line_tools.html M docs/command_line_tools_reference.html M docs/configuration.html M docs/configuration_reference.html M docs/configuration_reference_unsupported.html M docs/contributing.html M docs/developing.html M docs/export_control.html M docs/index.html M docs/installation.html M docs/known_issues.html M docs/kudu-master_configuration_reference.html M docs/kudu-master_configuration_reference_unsupported.html M docs/kudu-tserver_configuration_reference.html M docs/kudu-tserver_configuration_reference_unsupported.html M docs/kudu_impala_integration.html M docs/prior_release_notes.html M docs/quickstart.html M docs/release_notes.html M docs/schema_design.html M docs/security.html M docs/transaction_semantics.html M docs/troubleshooting.html 25 files changed, 2,685 insertions(+), 2,278 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/74/8974/1 -- To view, visit http://gerrit.cloudera.org:8080/8974 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: newchange Gerrit-Change-Id: I07d899da6ea7107f1e01c82d88b1a32583b57a54 Gerrit-Change-Number: 8974 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong
[kudu-CR] KUDU-2233 Add a test case for compactions in the past
Hello Tidy Bot, Kudu Jenkins, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8885 to look at the new patch set (#7). Change subject: KUDU-2233 Add a test case for compactions in the past .. KUDU-2233 Add a test case for compactions in the past In some cases we're performing compactions before the clock has been updated at all, making the compaction snapshot an empty one. This makes it so the compaction runs on a snapshot that discards all redo deltas and applies all undos. When a compaction with such a snapshot finds two duplicate rows (a ghost and live row) it can't distinguish the two because the redo DELETE of the ghost row has been discarded, causing the crash reported in KUDU-2233. This is a small subset of a broader set of problems that happen when compactions are performed under such snapshots, the most obvious one being that updates are discarded silently. This state is hard to reach as it requires a restart of a tablet server with a considerable amount of data (i.e. not empty) with a single wal segment (previous segments having been gc'd in the previous incarnation), with a single op in it. In particular this single op needs to be a NO_OP or a CHANGE_CONFIG op, as these are the only kinds of operations whose timestamps aren't used to update the clock. In order to get to this and similar states in fuzz-itest this patch adds three new types of operations: - Roll log - Rolls the log if possible. - GC log - Garbage collects log segments, if possible. - Run election - Runs an (emulated) leader election increasing the term and adding a NO_OP to the log. These operations are presently not part of the operations considered when generating random test sequences as those might trigger the bug. This patch adds a narrow, exact repro of the issue that is described in KUDU-2233 in the form of an additional test that is presently disabled. The repro is narrow because it attempts to mimic the crash from the original ticket exactly, instead of trying to catch the broader set of problems. Change-Id: Ibf5685897ef7580dd743faac4d68690a25663e4c --- M src/kudu/consensus/log.cc M src/kudu/integration-tests/fuzz-itest.cc 2 files changed, 93 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/85/8885/7 -- To view, visit http://gerrit.cloudera.org:8080/8885 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibf5685897ef7580dd743faac4d68690a25663e4c Gerrit-Change-Number: 8885 Gerrit-PatchSet: 7 Gerrit-Owner: David Ribeiro AlvesGerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files
Hello Mike Percy, Jean-Daniel Cryans, Kudu Jenkins, Adar Dembo, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8757 to look at the new patch set (#14). Change subject: KUDU-2216. Post process gtest generated xml to include the output from the *.txt files .. KUDU-2216. Post process gtest generated xml to include the output from the *.txt files The gtest junit output doesn't include the output from the gtest run. This adds a simple post process parser to pull that information from the gtest result and put it in the junit output. This helps when triaging issues in jenkins and other tools that rely on junit output. I included all of the output, since it's difficult to know in advance which parts will be useful. We may want to keep a close eye on Jenkins to see if this results in too much output and we might decide to limit this to just failures at the cost of some increase in parsing complexity. Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70 --- A build-support/jenkins/add_gtest_stdout_to_junit_xml.py M build-support/jenkins/build-and-test.sh 2 files changed, 129 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/8757/14 -- To view, visit http://gerrit.cloudera.org:8080/8757 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70 Gerrit-Change-Number: 8757 Gerrit-PatchSet: 14 Gerrit-Owner: Edward FancherGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Edward Fancher Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-721: Support for Decimal type, Part 1 (Server, C++ Client)
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/8830 ) Change subject: KUDU-721: Support for Decimal type, Part 1 (Server, C++ Client) .. Patch Set 1: (25 comments) I need to tweak the decimal class a bit, but I updated based on the reviews. http://gerrit.cloudera.org:8080/#/c/8830/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8830/6//COMMIT_MSG@24 PS6, Line 24: internal types are represented and stored as > Could we if/def them out? I wasn't sure exactly how to do that in a way that wouldn't break the client. I really want to add this back but should do so after the initial patch. http://gerrit.cloudera.org:8080/#/c/8830/6/src/kudu/client/schema.h File src/kudu/client/schema.h: http://gerrit.cloudera.org:8080/#/c/8830/6/src/kudu/client/schema.h@64 PS6, Line 64: class KuduColumnTypeAttributes { > Nit: indentation Done http://gerrit.cloudera.org:8080/#/c/8830/6/src/kudu/client/schema.h@70 PS6, Line 70: > The 'explicit' keyword is only needed to avoid implicit conversions with si Done http://gerrit.cloudera.org:8080/#/c/8830/6/src/kudu/client/schema.h@75 PS6, Line 75: /// @return Precision for the column type. > These accessors are returning an int32_t 'copy', so what value does the 'co Done http://gerrit.cloudera.org:8080/#/c/8830/6/src/kudu/client/schema.h@84 PS6, Line 84: > Do you anticipate KuduColumnTypeAttributes evolving for other use cases in The only other attribute I can see being added as of now is "size". Though precision could be "reused" for that if we want. That would cover CHAR(size), VARCHAR(size), TIMESTAMP(precision) types if we want to add them in the future. http://gerrit.cloudera.org:8080/#/c/8830/6/src/kudu/client/schema.h@320 PS6, Line 320: /// > Add a note about the valid value ranges here and below. Done http://gerrit.cloudera.org:8080/#/c/8830/1/src/kudu/client/schema.h File src/kudu/client/schema.h: http://gerrit.cloudera.org:8080/#/c/8830/1/src/kudu/client/schema.h@71 PS1, Line 71: explicit > I don't think this is needed unless we want to protect against list-initial Done http://gerrit.cloudera.org:8080/#/c/8830/1/src/kudu/client/schema.h@76 PS1, Line 76: const > drop Done http://gerrit.cloudera.org:8080/#/c/8830/1/src/kudu/client/schema.h@81 PS1, Line 81: const > drop Done http://gerrit.cloudera.org:8080/#/c/8830/1/src/kudu/client/schema.h@321 PS1, Line 321: /// Clients must specify a precision for decimal columns. > Add the documented description for the precision parameter. Done http://gerrit.cloudera.org:8080/#/c/8830/1/src/kudu/client/schema.h@328 PS1, Line 328: /// Clients must specify a scale for decimal columns. > Add the documented description for the scale parameter. Done http://gerrit.cloudera.org:8080/#/c/8830/6/src/kudu/client/schema.cc File src/kudu/client/schema.cc: http://gerrit.cloudera.org:8080/#/c/8830/6/src/kudu/client/schema.cc@261 PS6, Line 261: if (data_->type == KuduColumnSchema::DECIMAL && !data_->has_scale) { > I think this should be checking the scale/precision are within range, other Done http://gerrit.cloudera.org:8080/#/c/8830/6/src/kudu/client/value-internal.h File src/kudu/client/value-internal.h: http://gerrit.cloudera.org:8080/#/c/8830/6/src/kudu/client/value-internal.h@48 PS6, Line 48: Slice slice_val_; > just curious - do you know why this isn't in the union? I'm mildly concerne slice_val_ can't be added to the union because it has a non-trivial default constructor. I tried re-arranging the fields but regardless the sizeof data is 48. http://gerrit.cloudera.org:8080/#/c/8830/5/src/kudu/common/common.proto File src/kudu/common/common.proto: http://gerrit.cloudera.org:8080/#/c/8830/5/src/kudu/common/common.proto@54 PS5, Line 54: DECIMAL32 = 17; > nit: could you add a comment to explain why 15 and 16 are skipped? Done http://gerrit.cloudera.org:8080/#/c/8830/6/src/kudu/common/decimal_value.h File src/kudu/common/decimal_value.h: http://gerrit.cloudera.org:8080/#/c/8830/6/src/kudu/common/decimal_value.h@31 PS6, Line 31: xported head > bikesheddy, but I'd mildly prefer plain 'Decimal' for this class name. If Done http://gerrit.cloudera.org:8080/#/c/8830/6/src/kudu/common/decimal_value.h@32 PS6, Line 32: #include "kudu/client/stubs.h" > indentation Done http://gerrit.cloudera.org:8080/#/c/8830/6/src/kudu/common/decimal_value.h@38 PS6, Line 38: > 'can be' here and below Done http://gerrit.cloudera.org:8080/#/c/8830/6/src/kudu/common/decimal_value.h@57 PS6, Line 57: > I'd err on leaving this out of the public API unless there's a really compe Done http://gerrit.cloudera.org:8080/#/c/8830/6/src/kudu/common/decimal_value.h@66 PS6, Line 66: // Returns a string representation of the DecimalValue with a given precision > I somewhat expected there to be basic math functions (at least add/subtract yeah, I think this should be a follow up. Decimal math functions
[kudu-CR] KUDU-721: Support for Decimal type, Part 1 (Server, C++ Client)
Hello Tidy Bot, Alexey Serbin, Dan Burkert, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8830 to look at the new patch set (#7). Change subject: KUDU-721: Support for Decimal type, Part 1 (Server, C++ Client) .. KUDU-721: Support for Decimal type, Part 1 (Server, C++ Client) Introduces the Decimal type to the server and C++ client. Follow on work will enhance the utility in the client and add support to the Java client and other integrations and add documentation. The decimal type has column type attributes to support the “parameterized type”. The precision and scale column type attributes are not stored, but instead are leveraged to map to a correctly sized internal type (DECIMAL32, DECIMAL64, DECIMAL128). These internal types are represented and stored as equivalently sized integers. This also removes the int128 suffixes because they break the client by using C++11 features. They may be added back in a different way later. Change-Id: I3b06142f43c66973f36376bd2c88ca6f8d9f7632 --- M src/kudu/client/CMakeLists.txt M src/kudu/client/scan_batch.cc M src/kudu/client/scan_batch.h M src/kudu/client/schema-internal.h M src/kudu/client/schema.cc M src/kudu/client/schema.h M src/kudu/client/value-internal.h M src/kudu/client/value.cc M src/kudu/client/value.h M src/kudu/common/CMakeLists.txt M src/kudu/common/common.proto A src/kudu/common/decimal.cc A src/kudu/common/decimal.h M src/kudu/common/partial_row.cc M src/kudu/common/partial_row.h M src/kudu/common/schema-test.cc M src/kudu/common/schema.cc M src/kudu/common/schema.h M src/kudu/common/types.cc M src/kudu/common/types.h M src/kudu/common/wire_protocol.cc M src/kudu/consensus/log-test.cc M src/kudu/integration-tests/all_types-itest.cc M src/kudu/util/int128-test.cc M src/kudu/util/int128.h 25 files changed, 666 insertions(+), 140 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/8830/7 -- To view, visit http://gerrit.cloudera.org:8080/8830 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3b06142f43c66973f36376bd2c88ca6f8d9f7632 Gerrit-Change-Number: 8830 Gerrit-PatchSet: 7 Gerrit-Owner: Grant HenkeGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] [mvcc] Fix watermark advancement in the absence of committed transactions
David Ribeiro Alves has posted comments on this change. ( http://gerrit.cloudera.org:8080/8964 ) Change subject: [mvcc] Fix watermark advancement in the absence of committed transactions .. Patch Set 2: Verified+1 Unrelated failure: "RaftConsensusITest.TestReplicaBehaviorViaRPC" -- To view, visit http://gerrit.cloudera.org:8080/8964 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icf94ea289204adf2eed797c3623cdea1b11b09db Gerrit-Change-Number: 8964 Gerrit-PatchSet: 2 Gerrit-Owner: David Ribeiro AlvesGerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 09 Jan 2018 08:32:59 + Gerrit-HasComments: No
[kudu-CR] [mvcc] Fix watermark advancement in the absence of committed transactions
David Ribeiro Alves has removed a vote on this change. Change subject: [mvcc] Fix watermark advancement in the absence of committed transactions .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/8964 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: Icf94ea289204adf2eed797c3623cdea1b11b09db Gerrit-Change-Number: 8964 Gerrit-PatchSet: 2 Gerrit-Owner: David Ribeiro AlvesGerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon