[kudu-CR] WIP: KUDU-1567. Decouple hard-minimum WAL segment retention from target

[Todd Lipcon (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/4470

to look at the new patch set (#2).

Change subject: WIP: KUDU-1567. Decouple hard-minimum WAL segment retention 
from target
..

WIP: KUDU-1567. Decouple hard-minimum WAL segment retention from target

This changes the behavior around the "minimum log segments to retain".
Previously, the maintenance manager considered it high priority to flush
any in-memory store which was retaining more than this number of log
segments. With the default log_min_segments_to_retain=2, this caused the
maintenance manager to trigger very small flushes (128MB) regardless of
the size of flush_threshold_mb. The end result here was high write
amplification.

Testing with -log_min_segments_to_retain=50 indicated that write
performance could be improved about 2x and write amplification reduced
by about 1.7x by removing this aggressive flush behavior.

However, setting the 'min segments to retain' also had the unfortunate
side effect of _always_ retaining 50 segments, regardless of whether
those were actually necessary for durability purposes. In a long-running
cluster, most tablets are not actively being loaded into at such a high
rate, and retaining 50 segments would mean unnecessary disk usage as
well as longer startup times in the absence of a solution to KUDU-38.

Thus, this patch takes the approach of decoupling the two ideas into two
separate configurations:

1) the original 'log_min_segments_to_retain', which can be left very
   low, and now is really only useful for things like post-mortem
   debugging. A future commit could change this to 1 or possibly even 0.

2) a new 'maintenance_manager_target_log_replay_size_mb' flag, which
   indicates the amount of retained log data at which point the MM
   should schedule flushes of in-memory stores.

With the new defaults, we should have the following behavior:
- an MRS can fill up until the logs reach 1GB. At that point, the MM
  will begin flushing.
- after a flush, the logs will be GCed down to 2 segments.

WIP for a few reasons:
- should have some more tests for the new behavior
- probably needs some code cleanup
- should be cluster-tested
- maybe the flag should be renamed to have the 'log' prefix despite
  being named in the maintenance_manager_* module.
- should we drop log_min_segments_to_retain to 0?
- should test interaction if log_max_segments_to_retain is smaller than
  the size configured by ..._replay_size_mb.

Change-Id: I31400e2200f9ce3eeb63f4bc948bc630e8c1115f
---
M src/kudu/consensus/log-test.cc
M src/kudu/consensus/log.cc
M src/kudu/consensus/log.h
M src/kudu/tablet/tablet-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_peer.cc
M src/kudu/tablet/tablet_peer.h
M src/kudu/tablet/tablet_peer_mm_ops.cc
M src/kudu/util/maintenance_manager-test.cc
M src/kudu/util/maintenance_manager.cc
11 files changed, 140 insertions(+), 109 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/4470/2
-- 
To view, visit http://gerrit.cloudera.org:8080/4470
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I31400e2200f9ce3eeb63f4bc948bc630e8c1115f
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot

[kudu-CR] cli tool: List all tablets/replica uuids with 'kudu table list'

[Dinesh Bhat (Code Review)]

Hello Dan Burkert, Adar Dembo, Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/4440

to look at the new patch set (#5).

Change subject: cli tool: List all tablets/replica_uuids with 'kudu table list'
..

cli tool: List all tablets/replica_uuids with 'kudu table list'

I noticed that given a tablet or replica_uuid we need to execute
multiple nested commands and also need to correlate tablets and
their replica uuids and their relation to tables. Added a verbose
flag to 'kudu table list' to make this simpler.

Change-Id: Ic8f8e0dfb8e7ba9f67d5926199a9b831351585a7
---
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tools/tool_action_table.cc
3 files changed, 88 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/4440/5
-- 
To view, visit http://gerrit.cloudera.org:8080/4440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic8f8e0dfb8e7ba9f67d5926199a9b831351585a7
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] cli tool: List all tablets/replica uuids with 'kudu table list'

[Dinesh Bhat (Code Review)]

Dinesh Bhat has posted comments on this change.

Change subject: cli tool: List all tablets/replica_uuids with 'kudu table list'
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4440/4/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:

PS4, Line 251:   ASSERT_STR_CONTAINS(stdout, kTableId);
 :   ASSERT_STR_CONTAINS(stdout, kAnotherT
> Is there a guarantee that the two tables will be printed in this order?
Hmmm, I looked at my test output and another cluster output where I had 4 
tables, and they seem to be ordered alphabetically. However, after looking at 
ListTables/CreateTable service handlers in catalog_manager they are using 
std::unordered_map which doesn't guarantee anything about ordering of the keys 
in the map. For safety purposes, I am removing this assumption from code. 
Thanks for catching that !!


-- 
To view, visit http://gerrit.cloudera.org:8080/4440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8f8e0dfb8e7ba9f67d5926199a9b831351585a7
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1614 - [python] Enable Set/Get of unixtime micros

[Jordan Birdsell (Code Review)]

Jordan Birdsell has posted comments on this change.

Change subject: KUDU-1614 - [python] Enable Set/Get of unixtime_micros
..


Patch Set 9:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/4417/8/python/kudu/client.pyx
File python/kudu/client.pyx:

Line 1772: elif t == KUDU_DOUBLE:
> where are these capabilities documented? maybe PartialRow should have a pyd
Agreed, so, the pydoc stuff is going to take some rework of the docstrings and 
some structure of the package after looking through it. Are you thinking we 
should address it class by class or do the api docs for the whole package at 
once. If we want to do it all at once, would it make sense to throw together 
something on the website in asciidoc for some common usage examples?


Line 1779: slc = new Slice(cpython.PyBytes_AsString(value))
> while we're here, maybe we should add a blanket else: raise Exception("Unab
Done


http://gerrit.cloudera.org:8080/#/c/4417/8/python/kudu/util.py
File python/kudu/util.py:

Line 26: timezone provided.
> nit: trailing space
Done


Line 68: if timestamp.tzinfo and timestamp.utcoffset():
> https://docs.python.org/2/library/datetime.html#datetime.datetime.now says 
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/4417
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jordan Birdsell 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jordan Birdsell 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1614 - [python] Enable Set/Get of unixtime micros

[Jordan Birdsell (Code Review)]

Hello David Ribeiro Alves, Will Berkeley, Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/4417

to look at the new patch set (#9).

Change subject: KUDU-1614 - [python] Enable Set/Get of unixtime_micros
..

KUDU-1614 - [python] Enable Set/Get of unixtime_micros

Currently, the python client in Kudu does not support setting or
getting columns with the unixtime_micros type. This patch enables
this capability and includes a unit test. This patch also fixes
a minor bug with write operations using column indexes (KUDU-1615).
This fix is reflected in the unit test associated with this patch.

Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
---
M python/kudu/client.pyx
M python/kudu/libkudu_client.pxd
M python/kudu/tests/common.py
M python/kudu/tests/test_client.py
M python/kudu/tests/test_scanner.py
M python/kudu/tests/test_scantoken.py
A python/kudu/tests/util.py
M python/kudu/util.py
M python/requirements.txt
M python/setup.py
10 files changed, 287 insertions(+), 67 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/4417/9
-- 
To view, visit http://gerrit.cloudera.org:8080/4417
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jordan Birdsell 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jordan Birdsell 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] consensus: remove some unimplemented methods

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: consensus: remove some unimplemented methods
..


consensus: remove some unimplemented methods

Change-Id: I82089fa5a4d1e62e63c2656cc6ffeefccf10376c
Reviewed-on: http://gerrit.cloudera.org:8080/4452
Reviewed-by: David Ribeiro Alves 
Tested-by: Kudu Jenkins
---
M src/kudu/consensus/raft_consensus_state.h
1 file changed, 0 insertions(+), 12 deletions(-)

Approvals:
  David Ribeiro Alves: Looks good to me, approved
  Kudu Jenkins: Verified



-- 
To view, visit http://gerrit.cloudera.org:8080/4452
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I82089fa5a4d1e62e63c2656cc6ffeefccf10376c
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR](gh-pages) Docs and download page for 1.0

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: Docs and download page for 1.0
..


Docs and download page for 1.0

Change-Id: I200e295652fea930f21d52836d75f3bc9c7a3db1
Reviewed-on: http://gerrit.cloudera.org:8080/4472
Reviewed-by: Jean-Daniel Cryans 
Tested-by: Todd Lipcon 
---
M apidocs/allclasses-frame.html
M apidocs/allclasses-noframe.html
M apidocs/constant-values.html
M apidocs/deprecated-list.html
M apidocs/help-doc.html
M apidocs/index-all.html
M apidocs/index.html
M apidocs/org/apache/kudu/ColumnSchema.html
M apidocs/org/apache/kudu/Schema.html
M apidocs/org/apache/kudu/Type.html
M apidocs/org/apache/kudu/annotations/InterfaceAudience.html
M apidocs/org/apache/kudu/annotations/InterfaceStability.html
M apidocs/org/apache/kudu/annotations/class-use/InterfaceAudience.html
M apidocs/org/apache/kudu/annotations/class-use/InterfaceStability.html
M apidocs/org/apache/kudu/annotations/package-frame.html
M apidocs/org/apache/kudu/annotations/package-summary.html
M apidocs/org/apache/kudu/annotations/package-tree.html
M apidocs/org/apache/kudu/annotations/package-use.html
M apidocs/org/apache/kudu/class-use/ColumnSchema.html
M apidocs/org/apache/kudu/class-use/Schema.html
M apidocs/org/apache/kudu/class-use/Type.html
M apidocs/org/apache/kudu/client/AbstractKuduScannerBuilder.html
M apidocs/org/apache/kudu/client/AlterTableOptions.html
M apidocs/org/apache/kudu/client/AlterTableResponse.html
M apidocs/org/apache/kudu/client/AsyncKuduClient.AsyncKuduClientBuilder.html
M apidocs/org/apache/kudu/client/AsyncKuduClient.html
M apidocs/org/apache/kudu/client/AsyncKuduScanner.AsyncKuduScannerBuilder.html
M apidocs/org/apache/kudu/client/AsyncKuduScanner.ReadMode.html
M apidocs/org/apache/kudu/client/AsyncKuduScanner.html
M apidocs/org/apache/kudu/client/AsyncKuduSession.html
M apidocs/org/apache/kudu/client/ColumnRangePredicate.html
M apidocs/org/apache/kudu/client/CreateTableOptions.html
M apidocs/org/apache/kudu/client/Delete.html
M apidocs/org/apache/kudu/client/DeleteTableResponse.html
M apidocs/org/apache/kudu/client/ExternalConsistencyMode.html
M apidocs/org/apache/kudu/client/HasFailedRpcException.html
M apidocs/org/apache/kudu/client/Insert.html
M apidocs/org/apache/kudu/client/IsAlterTableDoneResponse.html
M apidocs/org/apache/kudu/client/KuduClient.KuduClientBuilder.html
M apidocs/org/apache/kudu/client/KuduClient.html
M apidocs/org/apache/kudu/client/KuduException.html
M apidocs/org/apache/kudu/client/KuduPredicate.ComparisonOp.html
M apidocs/org/apache/kudu/client/KuduPredicate.html
M apidocs/org/apache/kudu/client/KuduScanToken.KuduScanTokenBuilder.html
M apidocs/org/apache/kudu/client/KuduScanToken.html
M apidocs/org/apache/kudu/client/KuduScanner.KuduScannerBuilder.html
M apidocs/org/apache/kudu/client/KuduScanner.html
M apidocs/org/apache/kudu/client/KuduSession.html
M apidocs/org/apache/kudu/client/KuduTable.html
M apidocs/org/apache/kudu/client/ListTablesResponse.html
M apidocs/org/apache/kudu/client/ListTabletServersResponse.html
M apidocs/org/apache/kudu/client/LocatedTablet.Replica.html
M apidocs/org/apache/kudu/client/LocatedTablet.html
M apidocs/org/apache/kudu/client/Operation.html
M apidocs/org/apache/kudu/client/OperationResponse.html
M apidocs/org/apache/kudu/client/PartialRow.html
M apidocs/org/apache/kudu/client/PleaseThrottleException.html
M apidocs/org/apache/kudu/client/RangePartitionBound.html
M apidocs/org/apache/kudu/client/RowError.html
M apidocs/org/apache/kudu/client/RowErrorsAndOverflowStatus.html
M apidocs/org/apache/kudu/client/RowResult.html
M apidocs/org/apache/kudu/client/RowResultIterator.html
M apidocs/org/apache/kudu/client/SessionConfiguration.FlushMode.html
M apidocs/org/apache/kudu/client/SessionConfiguration.html
M apidocs/org/apache/kudu/client/Statistics.Statistic.html
M apidocs/org/apache/kudu/client/Statistics.html
M apidocs/org/apache/kudu/client/Status.html
M apidocs/org/apache/kudu/client/Update.html
M apidocs/org/apache/kudu/client/Upsert.html
M apidocs/org/apache/kudu/client/class-use/AbstractKuduScannerBuilder.html
M apidocs/org/apache/kudu/client/class-use/AlterTableOptions.html
M apidocs/org/apache/kudu/client/class-use/AlterTableResponse.html
M 
apidocs/org/apache/kudu/client/class-use/AsyncKuduClient.AsyncKuduClientBuilder.html
M apidocs/org/apache/kudu/client/class-use/AsyncKuduClient.html
M 
apidocs/org/apache/kudu/client/class-use/AsyncKuduScanner.AsyncKuduScannerBuilder.html
M apidocs/org/apache/kudu/client/class-use/AsyncKuduScanner.ReadMode.html
M apidocs/org/apache/kudu/client/class-use/AsyncKuduScanner.html
M apidocs/org/apache/kudu/client/class-use/AsyncKuduSession.html
M apidocs/org/apache/kudu/client/class-use/ColumnRangePredicate.html
M apidocs/org/apache/kudu/client/class-use/CreateTableOptions.html
M apidocs/org/apache/kudu/client/class-use/Delete.html
M 

[kudu-CR](gh-pages) Remove 'beta' verbiage from site

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: Remove 'beta' verbiage from site
..


Remove 'beta' verbiage from site

Change-Id: I864b9fbbc1fc6c389ab4dc082e72a46cc99d9d68
Reviewed-on: http://gerrit.cloudera.org:8080/4473
Reviewed-by: Jean-Daniel Cryans 
Tested-by: Todd Lipcon 
---
M faq.md
M index.html
2 files changed, 13 insertions(+), 31 deletions(-)

Approvals:
  Jean-Daniel Cryans: Looks good to me, approved
  Todd Lipcon: Verified



-- 
To view, visit http://gerrit.cloudera.org:8080/4473
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I864b9fbbc1fc6c389ab4dc082e72a46cc99d9d68
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR](gh-pages) Remove 'beta' verbiage from site

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Remove 'beta' verbiage from site
..


Patch Set 1: Verified+1

-- 
To view, visit http://gerrit.cloudera.org:8080/4473
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I864b9fbbc1fc6c389ab4dc082e72a46cc99d9d68
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] [tests] MANUAL FLUSH --> AUTO FLUSH BACKGROUND

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: [tests] MANUAL_FLUSH --> AUTO_FLUSH_BACKGROUND
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4471/1/src/kudu/client/client-test.cc
File src/kudu/client/client-test.cc:

PS1, Line 3502: {
> sprurious change
ok, will return back.


http://gerrit.cloudera.org:8080/#/c/4471/1/src/kudu/integration-tests/alter_table-test.cc
File src/kudu/integration-tests/alter_table-test.cc:

Line 914
> hum, you removed the statement that incremented 'i' this test still passes?
Good catch -- yes, it's a mistake.  Will fix.


-- 
To view, visit http://gerrit.cloudera.org:8080/4471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ieafc198609cceb5d6945a910364056d81786629a
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes

[kudu-CR] consensus: fix some clang-tidy warnings

[Todd Lipcon (Code Review)]

Hello David Ribeiro Alves, Mike Percy, Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/4454

to look at the new patch set (#2).

Change subject: consensus: fix some clang-tidy warnings
..

consensus: fix some clang-tidy warnings

This fixes most of the clang-tidy warnings in the consensus module. I'm
preparing to do some refactoring in the module, so I figured it was
better to do the tidy ahead of time rather than fighting the clang-tidy
bot when I moved some code.

Change-Id: Ieaaafe0bbc6b809b379f25e2076453dea973a37f
---
M src/kudu/consensus/consensus-test-util.h
M src/kudu/consensus/consensus.cc
M src/kudu/consensus/consensus_meta-test.cc
M src/kudu/consensus/consensus_meta.h
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_peers.cc
M src/kudu/consensus/consensus_peers.h
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/consensus_queue.h
M src/kudu/consensus/leader_election-test.cc
M src/kudu/consensus/leader_election.cc
M src/kudu/consensus/leader_election.h
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log-test.cc
M src/kudu/consensus/log.cc
M src/kudu/consensus/log.h
M src/kudu/consensus/log_anchor_registry.cc
M src/kudu/consensus/log_anchor_registry.h
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/log_cache.cc
M src/kudu/consensus/log_cache.h
M src/kudu/consensus/log_index.cc
M src/kudu/consensus/log_metrics.h
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/log_reader.h
M src/kudu/consensus/log_util.h
M src/kudu/consensus/mt-log-test.cc
M src/kudu/consensus/opid_util.cc
M src/kudu/consensus/peer_manager.cc
M src/kudu/consensus/peer_manager.h
M src/kudu/consensus/quorum_util.cc
M src/kudu/consensus/raft_consensus-test.cc
M src/kudu/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus.h
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/consensus/raft_consensus_state-test.cc
M src/kudu/consensus/raft_consensus_state.h
M src/kudu/tablet/tablet_bootstrap-test.cc
39 files changed, 189 insertions(+), 219 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/4454/2
-- 
To view, visit http://gerrit.cloudera.org:8080/4454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ieaaafe0bbc6b809b379f25e2076453dea973a37f
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot

[kudu-CR] metrics: move SCOPED LATENCY METRIC to metrics.h

[Todd Lipcon (Code Review)]

Hello David Ribeiro Alves, Mike Percy,

I'd like you to do a code review.  Please visit

http://gerrit.cloudera.org:8080/4469

to review the following change.

Change subject: metrics: move SCOPED_LATENCY_METRIC to metrics.h
..

metrics: move SCOPED_LATENCY_METRIC to metrics.h

Addresses a simple TODO.

Change-Id: I1f1287ade44c60236d2859b5e528ce7e365645ef
---
M src/kudu/consensus/log_metrics.h
M src/kudu/util/metrics.h
2 files changed, 4 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/69/4469/1
-- 
To view, visit http://gerrit.cloudera.org:8080/4469
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I1f1287ade44c60236d2859b5e528ce7e365645ef
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Mike Percy 

[kudu-CR] [tests] MANUAL FLUSH --> AUTO FLUSH BACKGROUND

[Alexey Serbin (Code Review)]

Alexey Serbin has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/4471

Change subject: [tests] MANUAL_FLUSH --> AUTO_FLUSH_BACKGROUND
..

[tests] MANUAL_FLUSH --> AUTO_FLUSH_BACKGROUND

In tests, run KuduSession in AUTO_FLUSH_BACKGROUND instead of
MANUAL_FLUSH mode where appropriate.

Change-Id: Ieafc198609cceb5d6945a910364056d81786629a
---
M src/kudu/client/client-test.cc
M src/kudu/client/predicate-test.cc
M src/kudu/client/scan_token-test.cc
M src/kudu/integration-tests/all_types-itest.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/integration-tests/client_failover-itest.cc
M src/kudu/integration-tests/flex_partitioning-itest.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/integration-tests/tablet_history_gc-itest.cc
M src/kudu/integration-tests/test_workload.cc
M src/kudu/tools/ksck_remote-test.cc
11 files changed, 50 insertions(+), 75 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/4471/1
-- 
To view, visit http://gerrit.cloudera.org:8080/4471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ieafc198609cceb5d6945a910364056d81786629a
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 

[kudu-CR] consensus: fix some clang-tidy warnings

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: consensus: fix some clang-tidy warnings
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4454/1/src/kudu/consensus/log_metrics.h
File src/kudu/consensus/log_metrics.h:

Line 46: // TODO extract and generalize this for all histogram metrics
> warning: missing username/bug in TODO [google-readability-todo]
fixed in a separate patch


-- 
To view, visit http://gerrit.cloudera.org:8080/4454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ieaaafe0bbc6b809b379f25e2076453dea973a37f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] cli tool: List all tablets/replica uuids with 'kudu table list'

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: cli tool: List all tablets/replica_uuids with 'kudu table list'
..


Patch Set 5: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/4440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8f8e0dfb8e7ba9f67d5926199a9b831351585a7
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] Document Impala and Spark integration known issues & limitations

[Will Berkeley (Code Review)]

Will Berkeley has posted comments on this change.

Change subject: Document Impala and Spark integration known issues & limitations
..


Patch Set 1:

(1 comment)

Is there a note anywhere mentioning that the Spark integration is for Spark 
1.6, not 2.0? We should say somewhere in the docs since it's come up a couple 
of times in Slack, unless we set up 1.6 and 2.0 Spark builds before the next 
release.

http://gerrit.cloudera.org:8080/#/c/4443/1/docs/developing.adoc
File docs/developing.adoc:

PS1, Line 159: The column name can be altered in Kudu
unless the column is part of the primary key


-- 
To view, visit http://gerrit.cloudera.org:8080/4443
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I993a09a00f5ab0049fec95e967abc1740b44dc8d
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes

[kudu-CR] Implement an upgrade test

[Will Berkeley (Code Review)]

Will Berkeley has posted comments on this change.

Change subject: Implement an upgrade test
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4424/2/src/kudu/integration-tests/upgrade-test.cc
File src/kudu/integration-tests/upgrade-test.cc:

PS2, Line 108: FLAGS_test_version_a_bin
Typo: FLAGS_test_version_b_bin


-- 
To view, visit http://gerrit.cloudera.org:8080/4424
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2831b47e6c0b644a256e1914fa495f453318e0cd
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes

[kudu-CR] cli tool: List all tablets/replica uuids with 'kudu table list'

[Dinesh Bhat (Code Review)]

Hello Dan Burkert, Adar Dembo, Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/4440

to look at the new patch set (#3).

Change subject: cli tool: List all tablets/replica_uuids with 'kudu table list'
..

cli tool: List all tablets/replica_uuids with 'kudu table list'

I noticed that given a tablet or replica_uuid we need to execute
multiple nested commands and also need to correlate tablets and
their replica uuids and their relation to tables. Added a verbose
flag to 'kudu table list' to make this simpler.

Change-Id: Ic8f8e0dfb8e7ba9f67d5926199a9b831351585a7
---
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tools/tool_action_table.cc
3 files changed, 66 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/4440/3
-- 
To view, visit http://gerrit.cloudera.org:8080/4440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic8f8e0dfb8e7ba9f67d5926199a9b831351585a7
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [tools]: Keep the verbosity of CLI at WARNING and above

[Dinesh Bhat (Code Review)]

Hello David Ribeiro Alves, Adar Dembo, Alexey Serbin,

I'd like you to do a code review.  Please visit

http://gerrit.cloudera.org:8080/4447

to review the following change.

Change subject: [tools]: Keep the verbosity of CLI at WARNING and above
..

[tools]: Keep the verbosity of CLI at WARNING and above

This keeps the verbosity of the 'kudu' commands at WARNING
and above if the user had not specified 'minloglevel' or
'--v' on the command line options. Both stdout and stderr
will suppress INFO level output without having to explicitly
append '2>/dev/null' to cli commands.

Change-Id: I882a340d4c1d205e4e998c888f487b7185000e3c
---
M src/kudu/tools/tool_main.cc
1 file changed, 9 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/47/4447/1
-- 
To view, visit http://gerrit.cloudera.org:8080/4447
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I882a340d4c1d205e4e998c888f487b7185000e3c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 

[kudu-CR] Document Impala and Spark integration known issues & limitations

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: Document Impala and Spark integration known issues & limitations
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4443/2/docs/developing.adoc
File docs/developing.adoc:

Line 163: - `NULL`, `NOT NULL`, `<>`, and `OR` predicates are not pushed to 
Kudu, and
What about 'LIKE', 'IN' and 'BETWEEN' predicates?  Can they be pushed to Kudu 
now?  Or those are not supported by SparkSQL itself?


http://gerrit.cloudera.org:8080/#/c/4443/2/docs/kudu_impala_integration.adoc
File docs/kudu_impala_integration.adoc:

Line 1104: - Impala can not create Kudu tables with bounded range partitions, 
and can not
Is it possible to add hash partitions for already existing Kudu table in Impala?


-- 
To view, visit http://gerrit.cloudera.org:8080/4443
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I993a09a00f5ab0049fec95e967abc1740b44dc8d
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1623. Properly handle UPSERTS that only include PK column

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: KUDU-1623. Properly handle UPSERTS that only include PK column
..


Patch Set 3: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/4441
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic878f6e51ead5bf91335ddb47536e7c29de11ac4
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] [tools]: Keep the verbosity of CLI at WARNING and above

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: [tools]: Keep the verbosity of CLI at WARNING and above
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4447/1/src/kudu/tools/tool_main.cc
File src/kudu/tools/tool_main.cc:

PS1, Line 215:   // Set the verbosity of the commands to WARNING(2) and above.
 :   // If the user had explicitly specified verbosity, then user's
 :   // verbosity level is honored. Since '--v' depends on 
minloglevel
 :   // specifying either of them on CLI will override this setting.
 :   if 
(google::GetCommandLineFlagInfoOrDie("minloglevel").is_default &&
 :   google::GetCommandLineFlagInfoOrDie("v").is_default) {
 : google::SetCommandLineOption("minloglevel",
 :  
SimpleItoa(google::GLOG_WARNING).c_str());
 :   }
Couple of comments:
1. Can you move this into ParseCommandLineFlags, since it's  doing some parsing 
of its own?
2. I still think that the default logging behavior should be to log absolutely 
nothing. Maybe FATAL or higher so that _crashes_ are logged, but that's about 
it. This is because during regular operations (i.e. outside of crashes), every 
action returns a detailed Status which gets logged already. That Status should 
be enough to diagnose simple errors. If not, the user can rerun the action with 
--v or --minloglevel set to something else.
3. Ideally we'd show --v and --minloglevel on every action's help, but adding 
it to every action manually is a pain in the butt. This is where "inheriting" 
parameters from modes in the mode chain would be a nice feature to have. You 
don't need to implement that, though; I'm just thinking out loud.


-- 
To view, visit http://gerrit.cloudera.org:8080/4447
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I882a340d4c1d205e4e998c888f487b7185000e3c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] Python - Bump package version to 0.4.0

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: Python - Bump package version to 0.4.0
..


Python - Bump package version to 0.4.0

There have been additional changes to the python client since the
0.3.0 release so the version number needs bumped before the next
release.

Change-Id: Iff66be56a55f5f75c20bd2183d4ffad54edf46f1
Reviewed-on: http://gerrit.cloudera.org:8080/
Tested-by: Kudu Jenkins
Reviewed-by: David Ribeiro Alves 
---
M python/setup.py
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  David Ribeiro Alves: Looks good to me, approved
  Kudu Jenkins: Verified



-- 
To view, visit http://gerrit.cloudera.org:8080/
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Iff66be56a55f5f75c20bd2183d4ffad54edf46f1
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jordan Birdsell 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-1623. Properly handle UPSERTS that only include PK column

[Todd Lipcon (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/4441

to look at the new patch set (#3).

Change subject: KUDU-1623. Properly handle UPSERTS that only include PK column
..

KUDU-1623. Properly handle UPSERTS that only include PK column

This fixes a bug reported by Chris George in which the tablet server
would crash when handling an UPSERT which was converted into an empty
UPDATE. For example, in pseudo-SQL:

  CREATE TABLE t (
x INT NOT NULL PRIMARY KEY
  );

  UPSERT INTO t VALUES (1);
  UPSERT INTO t VALUES (1);

Here the second upsert would detect the primary key already existed and
convert into an update containing any non-PK columns. Since there are no
non-PK columns, the update was "empty", and ended up as an empty
RowChangeList. In DEBUG builds, this fired a DCHECK, but in RELEASE
builds, we ended up with an empty RowChangeList in the DMS. At flush
time, this would cause other assertions to fire or crash.

The fix is relatively simple: if we see that an upsert converted into an
empty update, we just drop the delta rather than continuing to insert
into the delta store. This required loosing a check at bootstrap time,
where we previously assumed that every mutation must have been recorded
in at least one store.

The test changes are much larger than the fix itself due to some
refactoring. The main gist of the change is to introduce operations in
tablet_random_access-test and fuzz-itest where we send an UPSERT which
contains only the primary key column. These tests crashed prior to the
fix and pass now.

Change-Id: Ic878f6e51ead5bf91335ddb47536e7c29de11ac4
---
M src/kudu/integration-tests/fuzz-itest.cc
A src/kudu/tablet/key_value_test_schema.h
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_random_access-test.cc
5 files changed, 229 insertions(+), 73 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/4441/3
-- 
To view, visit http://gerrit.cloudera.org:8080/4441
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic878f6e51ead5bf91335ddb47536e7c29de11ac4
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] row operations-test: reduce iterations in ASAN build

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: row_operations-test: reduce iterations in ASAN build
..


Patch Set 1: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/4449
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6378acb0a53a1401d1341f13350cc1255eea404f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] Separated Dead and Live tablet server count in master web ui.

[Ninad Shringarpure (Code Review)]

Ninad Shringarpure has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/4450

Change subject: Separated Dead and Live tablet server count in master web ui.
..

Separated Dead and Live tablet server count in master web ui.

Change-Id: I479fad5c2db61949f7d67bde7901e7a59c60b786
---
M src/kudu/master/master-path-handlers.cc
1 file changed, 12 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/50/4450/1
-- 
To view, visit http://gerrit.cloudera.org:8080/4450
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I479fad5c2db61949f7d67bde7901e7a59c60b786
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Ninad Shringarpure 

[kudu-CR] [tools]: Keep the verbosity of CLI at WARNING and above

[Dinesh Bhat (Code Review)]

Hello David Ribeiro Alves, Adar Dembo, Alexey Serbin, Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/4447

to look at the new patch set (#2).

Change subject: [tools]: Keep the verbosity of CLI at WARNING and above
..

[tools]: Keep the verbosity of CLI at WARNING and above

This keeps the verbosity of the 'kudu' commands at WARNING
and above if the user had not specified 'minloglevel' or
'--v' on the command line options. Both stdout and stderr
will suppress INFO level output without having to explicitly
append '2>/dev/null' to cli commands.

Change-Id: I882a340d4c1d205e4e998c888f487b7185000e3c
---
M src/kudu/tools/tool_main.cc
1 file changed, 9 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/47/4447/2
-- 
To view, visit http://gerrit.cloudera.org:8080/4447
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I882a340d4c1d205e4e998c888f487b7185000e3c
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] Improve the debuggability of LogBlockContainer::CheckBlockRecord()

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has submitted this change and it was merged.

Change subject: Improve the debuggability of 
LogBlockContainer::CheckBlockRecord()
..


Improve the debuggability of LogBlockContainer::CheckBlockRecord()

We're getting an error come up in the flaky tests that is hard to debug
because we don't print the data file size, which is the condition tripping
the check. This also includes the filename of the data file where the
offending record was found in the printout.

Change-Id: I3e74537ea258e8c6a0ffa00b9f5d529a04ecd583
Reviewed-on: http://gerrit.cloudera.org:8080/4451
Reviewed-by: Todd Lipcon 
Tested-by: Kudu Jenkins
---
M src/kudu/fs/log_block_manager.cc
1 file changed, 3 insertions(+), 1 deletion(-)

Approvals:
  Todd Lipcon: Looks good to me, approved
  Kudu Jenkins: Verified



-- 
To view, visit http://gerrit.cloudera.org:8080/4451
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I3e74537ea258e8c6a0ffa00b9f5d529a04ecd583
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [tools]: Keep the verbosity of CLI at FATAL and above

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: [tools]: Keep the verbosity of CLI at FATAL and above
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4447/1/src/kudu/tools/tool_main.cc
File src/kudu/tools/tool_main.cc:

PS1, Line 215:   // Set the verbosity of the commands to WARNING(2) and above.
 :   // If the user had explicitly specified verbosity, then user's
 :   // verbosity level is honored. Since '--v' depends on 
minloglevel
 :   // specifying either of them on CLI will override this setting.
 :   if 
(google::GetCommandLineFlagInfoOrDie("minloglevel").is_default &&
 :   google::GetCommandLineFlagInfoOrDie("v").is_default) {
 : google::SetCommandLineOption("minloglevel",
 :  
SimpleItoa(google::GLOG_WARNING).c_str());
 :   }
> It's difficult to corral INFO (and even WARNING) messages; they can come fr
I agree it's a little unusual looking, but given that many of the usages of the 
tool are for debugging, etc, I think starting with verbosity (and perhaps 
offering a standard -quiet) is better than the other way around.

As an example, if you try to run 'list_tables' against a master which is down 
right now, your commend just hangs as it generates retries. I would actually 
prefer it to be _more_ verbose here, saying that it's trying to contact a 
server and getting Connection Refused. I'd rather have the verbosity there by 
default and if people want to silence it they can pipe or --quiet


-- 
To view, visit http://gerrit.cloudera.org:8080/4447
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I882a340d4c1d205e4e998c888f487b7185000e3c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Add a basic .clang-tidy configuration

[Todd Lipcon (Code Review)]

Hello David Ribeiro Alves,

I'd like you to do a code review.  Please visit

http://gerrit.cloudera.org:8080/4453

to review the following change.

Change subject: Add a basic .clang-tidy configuration
..

Add a basic .clang-tidy configuration

This configuration enables just the most important checks for now, and
disables the more "stylistic" ones for the most part.

Change-Id: I7b22195b11265959768c07fb60e9f97feeba95c7
---
A src/kudu/.clang-tidy
1 file changed, 19 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/4453/1
-- 
To view, visit http://gerrit.cloudera.org:8080/4453
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7b22195b11265959768c07fb60e9f97feeba95c7
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 

[kudu-CR] consensus: remove some unimplemented methods

[Todd Lipcon (Code Review)]

Hello David Ribeiro Alves,

I'd like you to do a code review.  Please visit

http://gerrit.cloudera.org:8080/4452

to review the following change.

Change subject: consensus: remove some unimplemented methods
..

consensus: remove some unimplemented methods

Change-Id: I82089fa5a4d1e62e63c2656cc6ffeefccf10376c
---
M src/kudu/consensus/raft_consensus_state.h
1 file changed, 0 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/52/4452/1
-- 
To view, visit http://gerrit.cloudera.org:8080/4452
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I82089fa5a4d1e62e63c2656cc6ffeefccf10376c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 

[kudu-CR] consensus: remove some unimplemented methods

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: consensus: remove some unimplemented methods
..


Patch Set 1: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/4452
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I82089fa5a4d1e62e63c2656cc6ffeefccf10376c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] Add a basic .clang-tidy configuration

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: Add a basic .clang-tidy configuration
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4453/1/src/kudu/.clang-tidy
File src/kudu/.clang-tidy:

Line 18: Checks:  
'-*,clang-diagnostic-*,-clang-diagnostic-unused-const-variable,readability-*,-readability-implicit-bool-cast,-readability-braces-around-statements,-readability-redundant-string-init,-readability-inconsistent-declaration-parameter-name,performance-*,google-*,-google-readability-todo,-google-readability-braces-around-statements,misc-*,-misc-unused-parameters'
Is it possible to split this across multiple lines with backslashes or some 
such? It would simplify adding/removing checks.


-- 
To view, visit http://gerrit.cloudera.org:8080/4453
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7b22195b11265959768c07fb60e9f97feeba95c7
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] Add a basic .clang-tidy configuration

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Add a basic .clang-tidy configuration
..


Patch Set 1:

any docs on how to use this locally?

-- 
To view, visit http://gerrit.cloudera.org:8080/4453
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7b22195b11265959768c07fb60e9f97feeba95c7
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] KUDU-1614 - [python] Enable Set/Get of unixtime micros

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: KUDU-1614 - [python] Enable Set/Get of unixtime_micros
..


Patch Set 8:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/4417/8/python/kudu/client.pyx
File python/kudu/client.pyx:

Line 1772: #  eg: ("2016-01-01", "%Y-%m-%d")
where are these capabilities documented? maybe PartialRow should have a pydoc 
with some example usage, and documentation regarding timestamps in particular 
since it's non-obvious?


Line 1779: to_unixtime_micros(value))
while we're here, maybe we should add a blanket else: raise Exception("Unable 
to set kudu type ") type thing so we don't have silent errors in the 
future?

also, this line should be indented a bit


http://gerrit.cloudera.org:8080/#/c/4417/8/python/kudu/util.py
File python/kudu/util.py:

Line 26: timezone provided. 
nit: trailing space


Line 68: if timestamp.tzinfo:
https://docs.python.org/2/library/datetime.html#datetime.datetime.now says that 
a timestamp is considered "naive" unless it has both a tzinfo and utcoffset() 
returns something other than None. So, I guess this should also check that 
utcoffset() is not None?


-- 
To view, visit http://gerrit.cloudera.org:8080/4417
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jordan Birdsell 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jordan Birdsell 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1623. Properly handle UPSERTS that only include PK column

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: KUDU-1623. Properly handle UPSERTS that only include PK column
..


Patch Set 2:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/4441/2/src/kudu/tablet/key_value_test_schema.h
File src/kudu/tablet/key_value_test_schema.h:

Line 33: struct KeyValueTestRow {
> nit: I find this choice of name confusing. How about: ExpectedTestKeyValueR
changed to ExpectedKeyValueRow


http://gerrit.cloudera.org:8080/#/c/4441/2/src/kudu/tablet/tablet.cc
File src/kudu/tablet/tablet.cc:

PS2, Line 462: bet
> typo
Done


Line 465:   if (buf.size() == 0) {
> it would be slightly clearer to do enc->is_empty() instead of testing the b
Done


http://gerrit.cloudera.org:8080/#/c/4441/2/src/kudu/tablet/tablet_random_access-test.cc
File src/kudu/tablet/tablet_random_access-test.cc:

Line 119:   VLOG(1) << RowOperationsPB::Type_Name(op.type) << " " << 
op.row->ToString();
> add log output before this to explain what this output is?
Done


Line 169:int val,
> hum, re wrapping in this case don't we usually align the variables with the
yea, just got messed up during some renames


-- 
To view, visit http://gerrit.cloudera.org:8080/4441
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic878f6e51ead5bf91335ddb47536e7c29de11ac4
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] cli tool: List all tablets/replica uuids with 'kudu table list'

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: cli tool: List all tablets/replica_uuids with 'kudu table list'
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4440/3/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:

Line 192: TEST_F(AdminCliTest, TestListTables) {
No change here for multiple tables? Or in the new test?


-- 
To view, visit http://gerrit.cloudera.org:8080/4440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8f8e0dfb8e7ba9f67d5926199a9b831351585a7
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] Improve the debuggability of LogBlockContainer::CheckBlockRecord()

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: Improve the debuggability of 
LogBlockContainer::CheckBlockRecord()
..


Patch Set 1: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/4451
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I3e74537ea258e8c6a0ffa00b9f5d529a04ecd583
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] cli tool: List all tablets/replica uuids with 'kudu table list'

[Dinesh Bhat (Code Review)]

Dinesh Bhat has posted comments on this change.

Change subject: cli tool: List all tablets/replica_uuids with 'kudu table list'
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4440/3/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:

Line 192: TEST_F(AdminCliTest, TestListTables) {
> No change here for multiple tables? Or in the new test?
Yeah, I want to make below test populated with mulitple tables. Do you have any 
examples/inputs how to create multiple tables without re-inventing wheel here ? 
The way I see current structure of TabletIntegrationTestBase, it seems to be 
written for single cluster/table scenario. Perhaps I can add another routine to 
that CreateTable(string table-name) ? It prolly also means that we also need 
additional instances of KuduTable and tablet_id_ to track.


-- 
To view, visit http://gerrit.cloudera.org:8080/4440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8f8e0dfb8e7ba9f67d5926199a9b831351585a7
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] [tools]: Keep the verbosity of CLI at FATAL and above

[Dinesh Bhat (Code Review)]

Hello David Ribeiro Alves, Adar Dembo, Alexey Serbin, Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/4447

to look at the new patch set (#4).

Change subject: [tools]: Keep the verbosity of CLI at FATAL and above
..

[tools]: Keep the verbosity of CLI at FATAL and above

This keeps the verbosity of the 'kudu' commands at FATAL
if the user had not specified '--minloglevel' or '--v' on the
command line options. Both stdout and stderr will suppress
INFO level output without having to explicitly append '2>/dev/null'
to cli commands.

Change-Id: I882a340d4c1d205e4e998c888f487b7185000e3c
---
M src/kudu/tools/tool_main.cc
1 file changed, 8 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/47/4447/4
-- 
To view, visit http://gerrit.cloudera.org:8080/4447
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I882a340d4c1d205e4e998c888f487b7185000e3c
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [tools]: Keep the verbosity of CLI at FATAL and above

[Dinesh Bhat (Code Review)]

Dinesh Bhat has posted comments on this change.

Change subject: [tools]: Keep the verbosity of CLI at FATAL and above
..


Patch Set 3:

(1 comment)

Thanks, updated.

http://gerrit.cloudera.org:8080/#/c/4447/3/src/kudu/tools/tool_main.cc
File src/kudu/tools/tool_main.cc:

PS3, Line 215: google::SetCommandLineOption("minloglevel",
 :  
SimpleItoa(google::GLOG_FATAL).c_str());
> Can this just be:
Done, declaration is already available under logging.h


-- 
To view, visit http://gerrit.cloudera.org:8080/4447
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I882a340d4c1d205e4e998c888f487b7185000e3c
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] Improve the debuggability of LogBlockContainer::CheckBlockRecord()

[David Ribeiro Alves (Code Review)]

Hello Adar Dembo, Todd Lipcon,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/4451

to look at the new patch set (#2).

Change subject: Improve the debuggability of 
LogBlockContainer::CheckBlockRecord()
..

Improve the debuggability of LogBlockContainer::CheckBlockRecord()

We're getting an error come up in the flaky tests that is hard to debug
because we don't print the data file size, which is the condition tripping
the check. This also includes the filename of the data file where the
offending record was found in the printout.

Change-Id: I3e74537ea258e8c6a0ffa00b9f5d529a04ecd583
---
M src/kudu/fs/log_block_manager.cc
1 file changed, 3 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/51/4451/2
-- 
To view, visit http://gerrit.cloudera.org:8080/4451
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3e74537ea258e8c6a0ffa00b9f5d529a04ecd583
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Improve the debuggability of LogBlockContainer::CheckBlockRecord()

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Improve the debuggability of 
LogBlockContainer::CheckBlockRecord()
..


Patch Set 2: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/4451
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I3e74537ea258e8c6a0ffa00b9f5d529a04ecd583
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] cli tool: List all tablets/replica uuids with 'kudu table list'

[Dinesh Bhat (Code Review)]

Dinesh Bhat has posted comments on this change.

Change subject: cli tool: List all tablets/replica_uuids with 'kudu table list'
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4440/3/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:

Line 192: TEST_F(AdminCliTest, TestListTables) {
> Yeah, I want to make below test populated with mulitple tables. Do you have
Never mind, I was able to create another table using same client handle and 
schema, I will try to wrap this up with mutliple table output.


-- 
To view, visit http://gerrit.cloudera.org:8080/4440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8f8e0dfb8e7ba9f67d5926199a9b831351585a7
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] [tools]: Keep the verbosity of CLI at FATAL and above

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: [tools]: Keep the verbosity of CLI at FATAL and above
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4447/1/src/kudu/tools/tool_main.cc
File src/kudu/tools/tool_main.cc:

PS1, Line 215:   // Set the verbosity of the commands to WARNING(2) and above.
 :   // If the user had explicitly specified verbosity, then user's
 :   // verbosity level is honored. Since '--v' depends on 
minloglevel
 :   // specifying either of them on CLI will override this setting.
 :   if 
(google::GetCommandLineFlagInfoOrDie("minloglevel").is_default &&
 :   google::GetCommandLineFlagInfoOrDie("v").is_default) {
 : google::SetCommandLineOption("minloglevel",
 :  
SimpleItoa(google::GLOG_WARNING).c_str());
 :   }
> No, we wouldn't have to show _all_ gflags help. Imagine that the root mode 
hrm, I disagree with the idea to not log WARNING/ERROR... isn't the whole point 
of those log levels that they are things that should be seen by the user? I can 
understand not logging INFO... but even then, why not just make sure that we 
aren't unnecessarily verbose in INFO messages?


-- 
To view, visit http://gerrit.cloudera.org:8080/4447
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I882a340d4c1d205e4e998c888f487b7185000e3c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [tools]: Keep the verbosity of CLI at WARNING and above

[Dinesh Bhat (Code Review)]

Dinesh Bhat has posted comments on this change.

Change subject: [tools]: Keep the verbosity of CLI at WARNING and above
..


Patch Set 1:

(1 comment)

TFTR Adar, please see below.

http://gerrit.cloudera.org:8080/#/c/4447/1/src/kudu/tools/tool_main.cc
File src/kudu/tools/tool_main.cc:

PS1, Line 215:   // Set the verbosity of the commands to WARNING(2) and above.
 :   // If the user had explicitly specified verbosity, then user's
 :   // verbosity level is honored. Since '--v' depends on 
minloglevel
 :   // specifying either of them on CLI will override this setting.
 :   if 
(google::GetCommandLineFlagInfoOrDie("minloglevel").is_default &&
 :   google::GetCommandLineFlagInfoOrDie("v").is_default) {
 : google::SetCommandLineOption("minloglevel",
 :  
SimpleItoa(google::GLOG_WARNING).c_str());
 :   }
> Couple of comments:
1. Done.
2. I was wondering if we want to keep the errors visible - for eg, if we run 
cluster ksck on a cluster which is partitioned, some RPC failures 
indicate(although not user-friendly output) that those hosts are not reachable. 
But this makes the output slightly less machine-readable. So keeping the level 
to FATAL for now, and let's decipher the status from the output itself rather 
than from error/warning logs.
3. When you say 'parameters', I am thinking about optional mode params(which 
could be gflags too in our case). But inheriting them means exposing all the 
ugly output of gflags help again, isn't it ?


-- 
To view, visit http://gerrit.cloudera.org:8080/4447
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I882a340d4c1d205e4e998c888f487b7185000e3c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] [tools]: Keep the verbosity of CLI at FATAL and above

[Dinesh Bhat (Code Review)]

Hello David Ribeiro Alves, Adar Dembo, Alexey Serbin, Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/4447

to look at the new patch set (#3).

Change subject: [tools]: Keep the verbosity of CLI at FATAL and above
..

[tools]: Keep the verbosity of CLI at FATAL and above

This keeps the verbosity of the 'kudu' commands at FATAL
if the user had not specified '--minloglevel' or '--v' on the
command line options. Both stdout and stderr will suppress
INFO level output without having to explicitly append '2>/dev/null'
to cli commands.

Change-Id: I882a340d4c1d205e4e998c888f487b7185000e3c
---
M src/kudu/tools/tool_main.cc
1 file changed, 9 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/47/4447/3
-- 
To view, visit http://gerrit.cloudera.org:8080/4447
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I882a340d4c1d205e4e998c888f487b7185000e3c
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [tools]: Keep the verbosity of CLI at FATAL and above

[Dinesh Bhat (Code Review)]

Dinesh Bhat has posted comments on this change.

Change subject: [tools]: Keep the verbosity of CLI at FATAL and above
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4447/1/src/kudu/tools/tool_main.cc
File src/kudu/tools/tool_main.cc:

PS1, Line 215: FLAGS_minloglevel = google::GLOG_FATAL;
 :   }
 :   return show_help;
 : }
 : 
 : int main(int argc, char** argv) {
 :   bool show_help = ParseCommandLineFlags(, );
 :   FLAGS_logtostderr = true;
 :   k
> I agree it's a little unusual looking, but given that many of the usages of
One of the motivations of being less verbose here was to keep the output more 
machine-readable(I should update that in commit-message). Also binaries 
retrying on those RPC errors are difficult to apprehend for folks other than 
developers. Given that this tool may be used by support folks as well, we were 
inclining to go with default=quiet as opposed to default=verbose mode.


-- 
To view, visit http://gerrit.cloudera.org:8080/4447
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I882a340d4c1d205e4e998c888f487b7185000e3c
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] cli tool: List all tablets/replica uuids with 'kudu table list'

[Dinesh Bhat (Code Review)]

Dinesh Bhat has posted comments on this change.

Change subject: cli tool: List all tablets/replica_uuids with 'kudu table list'
..


Patch Set 4:

(1 comment)

TFTR Adar, updated the patch with new test added.

http://gerrit.cloudera.org:8080/#/c/4440/3/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:

Line 192: }
> Sure, you could add another CreateTable() variant. Alternatively, you could
Thanks, since the tablet/tserver data were available in place, I tested their 
output too.


-- 
To view, visit http://gerrit.cloudera.org:8080/4440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8f8e0dfb8e7ba9f67d5926199a9b831351585a7
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] Separated Dead and Live tablet server count in master web ui.

[Ninad Shringarpure (Code Review)]

Ninad Shringarpure has restored this change.

Change subject: Separated Dead and Live tablet server count in master web ui.
..


Restored

Made the required change and comiting the patch.

-- 
To view, visit http://gerrit.cloudera.org:8080/4450
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: restore
Gerrit-Change-Id: I479fad5c2db61949f7d67bde7901e7a59c60b786
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Ninad Shringarpure 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot

[kudu-CR] Separated Dead and Live tablet server count in master web ui.

[Ninad Shringarpure (Code Review)]

Ninad Shringarpure has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/4455

Change subject: Separated Dead and Live tablet server count in master web ui.
..

Separated Dead and Live tablet server count in master web ui.

Change-Id: I47a2f402e83a7a16a6c9fa9d90974abe3d865ca3
---
M src/kudu/master/master-path-handlers.cc
1 file changed, 1 insertion(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/55/4455/1
-- 
To view, visit http://gerrit.cloudera.org:8080/4455
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I47a2f402e83a7a16a6c9fa9d90974abe3d865ca3
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Ninad Shringarpure 

[kudu-CR] cli tool: List all tablets/replica uuids with 'kudu table list'

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: cli tool: List all tablets/replica_uuids with 'kudu table list'
..


Patch Set 4: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4440/4/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:

PS4, Line 251:   ASSERT_EQ(kAnotherTableId, stdout_lines[0]);
 :   ASSERT_EQ(kTableId, stdout_lines[2]);
Is there a guarantee that the two tables will be printed in this order?


-- 
To view, visit http://gerrit.cloudera.org:8080/4440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8f8e0dfb8e7ba9f67d5926199a9b831351585a7
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] Separated Dead and Live tablet server count in master web ui.

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Separated Dead and Live tablet server count in master web ui.
..


Patch Set 1:

hmm, were you looking at KUDU-1619? I think the intention here was to separate 
the tables which list the tablet servers into two sections, not the "version 
summary" at the top.

Also, would be good to mention the JIRA in the commit message (see how other 
commits in the git log refer to JIRAs)

-- 
To view, visit http://gerrit.cloudera.org:8080/4450
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I479fad5c2db61949f7d67bde7901e7a59c60b786
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Ninad Shringarpure 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Separated Dead and Live tablet server count in master web ui.

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Separated Dead and Live tablet server count in master web ui.
..


Patch Set 2:

Also, it seems like you've now got two reviews stacked on top of each other. 
Please use 'git rebase -i' to merge them into a single commit, and in the 
future use 'git commit --amend' to amend patches between revisions.

-- 
To view, visit http://gerrit.cloudera.org:8080/4450
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I479fad5c2db61949f7d67bde7901e7a59c60b786
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Ninad Shringarpure 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No