[kudu-CR] WIP [raft consensus-itest] proper fix for Test KUDU 1735

2018-01-09 Thread Alexey Serbin (Code Review)
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 Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 


[kudu-CR] [raft consensus-itest] proper fix for Test KUDU 1735

2018-01-09 Thread Alexey Serbin (Code Review)
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 Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 


[kudu-CR] KUDU-2202: support for removing data directories (take two)

2018-01-09 Thread Andrew Wong (Code Review)
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 Dembo 
Gerrit-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

2018-01-09 Thread Alexey Serbin (Code Review)
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

2018-01-09 Thread Mahdi Askari (Code Review)
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 Askari 
Gerrit-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

2018-01-09 Thread Mahdi Askari (Code Review)
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 Askari 
Gerrit-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

2018-01-09 Thread Todd Lipcon (Code Review)
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 Askari 
Gerrit-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

2018-01-09 Thread Todd Lipcon (Code Review)
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

2018-01-09 Thread Todd Lipcon (Code Review)
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 Wong 
Gerrit-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

2018-01-09 Thread Todd Lipcon (Code Review)
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 Burkert 
Gerrit-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

2018-01-09 Thread Todd Lipcon (Code Review)
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

2018-01-09 Thread Todd Lipcon (Code Review)
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 Berkeley 
Gerrit-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

2018-01-09 Thread Alexey Serbin (Code Review)
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 Berkeley 
Gerrit-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)

2018-01-09 Thread Adar Dembo (Code Review)
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 Dembo 
Gerrit-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)

2018-01-09 Thread Adar Dembo (Code Review)
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 Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] Fixup DeltaStats::ToString

2018-01-09 Thread Dan Burkert (Code Review)
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 Burkert 
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] [raft consensus-itest] fix for Test KUDU 1735

2018-01-09 Thread Alexey Serbin (Code Review)
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 Serbin 
Gerrit-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

2018-01-09 Thread Alexey Serbin (Code Review)
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 Dembo 
Tested-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

2018-01-09 Thread Adar Dembo (Code Review)
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 Serbin 
Gerrit-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

2018-01-09 Thread Alexey Serbin (Code Review)
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

2018-01-09 Thread Dan Burkert (Code Review)
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 Burkert 
Gerrit-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

2018-01-09 Thread Will Berkeley (Code Review)
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 Berkeley 
Gerrit-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

2018-01-09 Thread Alexey Serbin (Code Review)
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 Dembo 
Tested-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

2018-01-09 Thread Alexey Serbin (Code Review)
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 Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 


[kudu-CR] [tests] update tests for replication scheme consistency

2018-01-09 Thread Alexey Serbin (Code Review)
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 Serbin 
Gerrit-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

2018-01-09 Thread Mike Percy (Code Review)
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 Percy 
Gerrit-Reviewer: Edward Fancher 


[kudu-CR] KUDU-2233 Add a test case for compactions in the past

2018-01-09 Thread Todd Lipcon (Code Review)
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 Alves 
Gerrit-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

2018-01-09 Thread Dan Burkert (Code Review)
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 Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] [webui] Make tombstone tablet info less confusing

2018-01-09 Thread Will Berkeley (Code Review)
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

2018-01-09 Thread Todd Lipcon (Code Review)
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 Alves 
Reviewed-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

2018-01-09 Thread Todd Lipcon (Code Review)
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

2018-01-09 Thread Alexey Serbin (Code Review)
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 Serbin 
Gerrit-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

2018-01-09 Thread Alexey Serbin (Code Review)
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 Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR] [tests] update tests for replication scheme consistency

2018-01-09 Thread Adar Dembo (Code Review)
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 Serbin 
Gerrit-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

2018-01-09 Thread Adar Dembo (Code Review)
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 Serbin 
Gerrit-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

2018-01-09 Thread Dan Burkert (Code Review)
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 Burkert 
Gerrit-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

2018-01-09 Thread Dan Burkert (Code Review)
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

2018-01-09 Thread Dan Burkert (Code Review)
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

2018-01-09 Thread Todd Lipcon (Code Review)
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 Alves 
Gerrit-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

2018-01-09 Thread Todd Lipcon (Code Review)
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 Alves 
Gerrit-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

2018-01-09 Thread Adar Dembo (Code Review)
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 Dembo 
Gerrit-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)

2018-01-09 Thread Adar Dembo (Code Review)
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 Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR](branch-1.6.x) Add 'kudu fs list' tool

2018-01-09 Thread Todd Lipcon (Code Review)
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 Burkert 
Gerrit-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

2018-01-09 Thread Todd Lipcon (Code Review)
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 Burkert 
Gerrit-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

2018-01-09 Thread Adar Dembo (Code Review)
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 Dembo 
Gerrit-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

2018-01-09 Thread Adar Dembo (Code Review)
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 Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR](gh-pages) docs refresh

2018-01-09 Thread Andrew Wong (Code Review)
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

2018-01-09 Thread David Ribeiro Alves (Code Review)
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 Alves 
Gerrit-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

2018-01-09 Thread Edward Fancher (Code Review)
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 Fancher 
Gerrit-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)

2018-01-09 Thread Grant Henke (Code Review)
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)

2018-01-09 Thread Grant Henke (Code Review)
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 Henke 
Gerrit-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

2018-01-09 Thread David Ribeiro Alves (Code Review)
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 Alves 
Gerrit-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

2018-01-09 Thread David Ribeiro Alves (Code Review)
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 Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon