[kudu-CR] [WIP] [tools] ksck improvements [6/n]: Refactor printing

[Will Berkeley (Code Review)]

Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10151 )

Change subject: [WIP] [tools] ksck improvements [6/n]: Refactor printing
..


Patch Set 6:

(15 comments)

http://gerrit.cloudera.org:8080/#/c/10151/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10151/5//COMMIT_MSG@17
PS5, Line 17: - Removes the --consensus flag. ksck is far less useful without 
it and
:   it doesn't seem like having it brings any benefit.
> Any concerns about that in terms of backward compatibility?
I don't think we make guarantees on backwards compat for tools. Also, I don't 
think anyone besides me ever used that flag. The good info comes from the 
consensus state :)

I see the point about removing it separately but if I make it a patch prior  to 
this one I'm spending time working in the "old way" and have to rejigger the 
patch to match those changes. Seems wasteful. If I make it a patch after this 
one I have to redo pieces of this patch to re-introduce it, just so I can take 
it out.

Ergo, unless you have a big problem with keeping it part of this patch, I'd 
rather leave it in.


http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/integration-tests/cluster_verifier.cc
File src/kudu/integration-tests/cluster_verifier.cc:

http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/integration-tests/cluster_verifier.cc@112
PS5, Line 112: RunAndPrintResults
> Would it make sense to separate that in two methods:
Done


http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck-test.cc
File src/kudu/tools/ksck-test.cc:

http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck-test.cc@845
PS5, Line 845:
/*underreplicated_tables=*/ 3,
 :
/*consensus_mismatch_tables=*/ 0,
 :
/*unavailable_tables=*/ 0));
 : }
> Here an in other places:
Done. Plus some changes due to how I wrote RunAndPrintResults().


http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck-test.cc@858
PS5, Line 858:   // Now engineer a consensus conflict.
 :   const shared_ptr& ts = 
FindOrDie(cluster_->tablet_servers_, "ts-id-1");
 :   auto& cstate
> nit: why not just
Done


http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck-test.cc@878
PS5, Line 878:   ASSERT_STR_CONTAINS(err_stream_.str(),
 :   ExpectedKsckTableSummary("test",
 :
> ASSERT_OK(RunKsck()) ?
Done


http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck_results.h
File src/kudu/tools/ksck_results.h:

http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck_results.h@31
PS5, Line 31: "kudu/util/status.h"
> why angle brackets instead of quotes?
Oops.


http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck_results.h@126
PS5, Line 126: const char* const ServerHealthToString(KsckServerHealth sh);
 :
> It would be nice to mention what higher score value means, i.e. if server A
Done


http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck_results.h@133
PS5, Line 133: f a ser
> What if server has multiple RPC endpoints?
AFAIK we routinely don't handle this situation in our code. I don't think we 
support it overall even though some places are set up for it. What would end up 
here is what the master reports as the RPC endpoint.


http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck_results.h@257
PS5, Line 257: ch table and tablet.
> nit: why not constant reference?
Oops.


http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck_results.cc
File src/kudu/tools/ksck_results.cc:

http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck_results.cc@88
PS5, Line 88:   CHECK(uuid_label_mapping);
> static?  It seems this function can be entered multiple times during kudu C
First point: It's in an anonymous namespace, so it's functionally equivalent to 
a static function.

Second: Done.


http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck_results.cc@124
PS5, Line 124:   switch (cr) {
> nit: maybe, return 'const char* const' instead?
Done. Also done for function above.


http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck_results.cc@177
PS5, Line 177:   // First, report on the masters and master tablet.
> nit: add a stop
Done


http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck_results.cc@268
PS5, Line 268: }
> nit: two extra spaces
Done


http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck_results.cc@269
PS5, Line 269: uid : server_uuids) {
 : const string label(1, FindOrDie(replica_labels, uuid));
> consider adding replication factor, as it's done in https://gerrit.cloudera
Done


http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck_results.cc@273
PS5, Line 273:  

[kudu-CR] [WIP] [tools] ksck improvements [6/n]: Refactor printing

[Will Berkeley (Code Review)]

Hello Alexey Serbin, Kudu Jenkins,

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

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

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

Change subject: [WIP] [tools] ksck improvements [6/n]: Refactor printing
..

[WIP] [tools] ksck improvements [6/n]: Refactor printing

This patch significantly refactors printing in ksck. It separates where
checks are done from where results are printed. This separation will
make it easier to provide machine-readable output and create tools
based on the results of ksck checks, like auto-repair.

There's also a couple of bonus changes:
- Simplifies running all ksck checks and printing the results, as done
  by the ksck tool and ClusterVerifier.
- Removes the --consensus flag. ksck is far less useful without it and
  it doesn't seem like having it brings any benefit.
- Add the changes in https://gerrit.cloudera.org/#/c/10054/, which were
  about to be committed but would've needed to be redone a bit to fit
  with this refactor.

Change-Id: Id8de619996b6cd753e6a9c01b1b60810a873e609
---
M src/kudu/integration-tests/cluster_verifier.cc
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote-test.cc
A src/kudu/tools/ksck_results.cc
A src/kudu/tools/ksck_results.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/tool_action_cluster.cc
M src/kudu/tools/tool_action_tablet.cc
11 files changed, 1,206 insertions(+), 887 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id8de619996b6cd753e6a9c01b1b60810a873e609
Gerrit-Change-Number: 10151
Gerrit-PatchSet: 6
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] [docs] Add a navigation link to the scaling guide

[Grant Henke (Code Review)]

Grant Henke has removed a vote on this change.

Change subject: [docs] Add a navigation link to the scaling guide
..


Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/10181
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I3119d39b14c682fa40022ea5e5c199df15a033b2
Gerrit-Change-Number: 10181
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] [docs] Add a navigation link to the scaling guide

[Grant Henke (Code Review)]

Grant Henke has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10181 )

Change subject: [docs] Add a navigation link to the scaling guide
..

[docs] Add a navigation link to the scaling guide

Change-Id: I3119d39b14c682fa40022ea5e5c199df15a033b2
Reviewed-on: http://gerrit.cloudera.org:8080/10181
Reviewed-by: Will Berkeley 
Tested-by: Grant Henke 
---
M docs/support/jekyll-templates/document.html.erb
1 file changed, 1 insertion(+), 0 deletions(-)

Approvals:
  Will Berkeley: Looks good to me, approved
  Grant Henke: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I3119d39b14c682fa40022ea5e5c199df15a033b2
Gerrit-Change-Number: 10181
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] [docs] Add a navigation link to the scaling guide

[Grant Henke (Code Review)]

Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10181 )

Change subject: [docs] Add a navigation link to the scaling guide
..


Patch Set 1: Verified+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3119d39b14c682fa40022ea5e5c199df15a033b2
Gerrit-Change-Number: 10181
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Wed, 25 Apr 2018 02:57:40 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2416: Fix PartialRow.setMin and add a unit test

[Grant Henke (Code Review)]

Grant Henke has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10185


Change subject: KUDU-2416: Fix PartialRow.setMin and add a unit test
..

KUDU-2416: Fix PartialRow.setMin and add a unit test

* Fixes a missing break after the decimal type that could
   lead to IllegalArgumentExceptions.
* Fixes INTEGER.MIN_VALUE being used for LONG colums
  that could lead to incorrect partition prunning.
* Adds a unit test for all types.

Change-Id: I510743385cc567a8443efc8b0c9f5b641cf04c31
---
M java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java
2 files changed, 22 insertions(+), 1 deletion(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I510743385cc567a8443efc8b0c9f5b641cf04c31
Gerrit-Change-Number: 10185
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 

[kudu-CR] Re-enable IWYU checks on HMS module

[Alexey Serbin (Code Review)]

Alexey Serbin has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10102 )

Change subject: Re-enable IWYU checks on HMS module
..

Re-enable IWYU checks on HMS module

IWYU previously had some issues with the HMS module[1], probably due to
the Thrift generated classes. Now that IWYU has been upgraded[2] this is
an attempt to re-enable the IWYU checks.  This commit also includes some
associated fixups.

[1]: 
https://github.com/apache/kudu/commit/c09a6296181ecbb7d91687536f70fdbaac9fff5e
[2]: 
https://github.com/apache/kudu/commit/bba339b11795d84da9c9d1473366ca561730c141

Change-Id: I63e5f1ca6a480f22ede83acde6baea8e2f563e8a
Reviewed-on: http://gerrit.cloudera.org:8080/10102
Tested-by: Alexey Serbin 
Reviewed-by: Alexey Serbin 
---
M src/kudu/hms/hms_catalog-test.cc
M src/kudu/hms/hms_catalog.cc
M src/kudu/hms/hms_client-test.cc
M src/kudu/hms/hms_client.cc
M src/kudu/hms/mini_hms.cc
M src/kudu/hms/mini_hms.h
M src/kudu/integration-tests/master_hms-itest.cc
M src/kudu/master/catalog_manager.h
8 files changed, 43 insertions(+), 37 deletions(-)

Approvals:
  Alexey Serbin: Looks good to me, approved; Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I63e5f1ca6a480f22ede83acde6baea8e2f563e8a
Gerrit-Change-Number: 10102
Gerrit-PatchSet: 4
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Re-enable IWYU checks on HMS module

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10102 )

Change subject: Re-enable IWYU checks on HMS module
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I63e5f1ca6a480f22ede83acde6baea8e2f563e8a
Gerrit-Change-Number: 10102
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 25 Apr 2018 02:29:23 +
Gerrit-HasComments: No

[kudu-CR] Re-enable IWYU checks on HMS module

[Alexey Serbin (Code Review)]

Alexey Serbin has removed Kudu Jenkins from this change.  ( 
http://gerrit.cloudera.org:8080/10102 )

Change subject: Re-enable IWYU checks on HMS module
..


Removed reviewer Kudu Jenkins with the following votes:

* Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/10102
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: I63e5f1ca6a480f22ede83acde6baea8e2f563e8a
Gerrit-Change-Number: 10102
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] WIP: java: enable error-prone for java builds

[Grant Henke (Code Review)]

Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/4425 )

Change subject: WIP: java: enable error-prone for java builds
..


Patch Set 3:

(3 comments)

How does error-prone compare to findbugs/spotbugs?

http://gerrit.cloudera.org:8080/#/c/4425/3/java/build.gradle
File java/build.gradle:

http://gerrit.cloudera.org:8080/#/c/4425/3/java/build.gradle@24
PS3, Line 24:   id 'net.ltgt.errorprone' version '0.0.13' apply false
I have been putting all of these static analysis checks into quality.gradle


http://gerrit.cloudera.org:8080/#/c/4425/3/java/build.gradle@48
PS3, Line 48:   apply plugin: 'java'
Any reason you changed this to single quotes?


http://gerrit.cloudera.org:8080/#/c/4425/3/java/build.gradle@51
PS3, Line 51:   apply from: "$rootDir/gradle/errorprone.gradle"
Did you add this file? I have been putting all of these static analysis checks 
into quality.gradle



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
Gerrit-Change-Number: 4425
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 25 Apr 2018 01:31:18 +
Gerrit-HasComments: Yes

[kudu-CR] Re-enable IWYU checks on HMS module

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10102 )

Change subject: Re-enable IWYU checks on HMS module
..


Patch Set 3:

> > Rebased.  I'm a little confused, because I'm not getting iwyu
 > > errors before _or_ after applying this patch on an internal build
 > > machine.  We'll see what gerrit says.  I think the code cleanups
 > in
 > > this patch are worth landing.
 >
 > Yeah, I think the reason you didn't see errors on your local build
 > machine is that most likely Thrift-related files were already
 > generated when you ran the 'iwyu' target.  However, in case of
 > Jenkins slaves for pre-commit builds, the IWYU configuration just
 > run 'make iwyu' target before as is, without building the rest of
 > the sources.

I just verified on ve0518 with pre-72847ab (i.e. pre-based): if first building 
everything (i.e. just run 'make') and then run 'make iwyu',  there were no 
warnings from IWYU.  However, if doing 'make iwyu' on the clean build 
directory, there were nonsense-style warnings from IWYU.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I63e5f1ca6a480f22ede83acde6baea8e2f563e8a
Gerrit-Change-Number: 10102
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 25 Apr 2018 01:07:17 +
Gerrit-HasComments: No

[kudu-CR] WIP: java: enable error-prone for java builds

[Todd Lipcon (Code Review)]

Hello Dan Burkert, Jean-Daniel Cryans, Kudu Jenkins, Grant Henke,

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

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

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

Change subject: WIP: java: enable error-prone for java builds
..

WIP: java: enable error-prone for java builds

error-prone (http://errorprone.info/) is a compiler extension by Google
which checks for a bunch of common Java errors. One of the most useful
features is that it verifies JCIP annotations like GuardedBy.

This includes some fixes to get it to pass. We should break these fixes
out into separate commits.

WIP: not quite done fixing all errors yet, and likely worth breaing out
some fixes as above

Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
---
M java/build.gradle
M java/gradle/dependencies.gradle
M 
java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/ImportParquet.java
M 
java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/IntegrationTestBigLinkedList.java
M java/kudu-client/src/main/java/org/apache/kudu/Type.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Bytes.java
M 
java/kudu-client/src/main/java/org/apache/kudu/client/ColumnRangePredicate.java
M 
java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToMasterRequest.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
M java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java
M java/kudu-client/src/main/java/org/apache/kudu/client/DeadlineTracker.java
M 
java/kudu-client/src/main/java/org/apache/kudu/client/ExternalConsistencyMode.java
M 
java/kudu-client/src/main/java/org/apache/kudu/client/IsAlterTableDoneRequest.java
M 
java/kudu-client/src/main/java/org/apache/kudu/client/IsCreateTableDoneRequest.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/LocatedTablet.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
M java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java
M 
java/kudu-client/src/main/java/org/apache/kudu/client/PleaseThrottleException.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RequestTracker.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowError.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowResultIterator.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RpcProxy.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RpcTraceFrame.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Statistics.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Status.java
M java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java
M java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java
M java/kudu-client/src/main/java/org/apache/kudu/util/Slice.java
M java/kudu-client/src/main/java/org/apache/kudu/util/Slices.java
M java/kudu-client/src/test/java/org/apache/kudu/client/FakeDNS.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITClientStress.java
M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/TestAuthnTokenReacquire.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestBytes.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectToCluster.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduPredicate.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestRowResult.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestTestUtils.java
M java/kudu-client/src/test/java/org/apache/kudu/util/TestAsyncUtil.java
M 

[kudu-CR] Re-enable IWYU checks on HMS module

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10102 )

Change subject: Re-enable IWYU checks on HMS module
..


Patch Set 3:

> Rebased.  I'm a little confused, because I'm not getting iwyu
 > errors before _or_ after applying this patch on an internal build
 > machine.  We'll see what gerrit says.  I think the code cleanups in
 > this patch are worth landing.

Yeah, I think the reason you didn't see errors on your local build machine is 
that most likely Thrift-related files were already generated when you ran the 
'iwyu' target.  However, in case of Jenkins slaves for pre-commit builds, the 
IWYU configuration just run 'make iwyu' target before as is, without building 
the rest of the sources.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I63e5f1ca6a480f22ede83acde6baea8e2f563e8a
Gerrit-Change-Number: 10102
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 25 Apr 2018 00:59:26 +
Gerrit-HasComments: No

[kudu-CR] Re-enable IWYU checks on HMS module

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10102 )

Change subject: Re-enable IWYU checks on HMS module
..


Patch Set 3:

Rebased.  I'm a little confused, because I'm not getting iwyu errors before 
_or_ after applying this patch on an internal build machine.  We'll see what 
gerrit says.  I think the code cleanups in this patch are worth landing.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I63e5f1ca6a480f22ede83acde6baea8e2f563e8a
Gerrit-Change-Number: 10102
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 25 Apr 2018 00:45:38 +
Gerrit-HasComments: No

[kudu-CR] Re-enable IWYU checks on HMS module

[Dan Burkert (Code Review)]

Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, Todd Lipcon,

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

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

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

Change subject: Re-enable IWYU checks on HMS module
..

Re-enable IWYU checks on HMS module

IWYU previously had some issues with the HMS module[1], probably due to
the Thrift generated classes. Now that IWYU has been upgraded[2] this is
an attempt to re-enable the IWYU checks.  This commit also includes some
associated fixups.

[1]: 
https://github.com/apache/kudu/commit/c09a6296181ecbb7d91687536f70fdbaac9fff5e
[2]: 
https://github.com/apache/kudu/commit/bba339b11795d84da9c9d1473366ca561730c141

Change-Id: I63e5f1ca6a480f22ede83acde6baea8e2f563e8a
---
M src/kudu/hms/hms_catalog-test.cc
M src/kudu/hms/hms_catalog.cc
M src/kudu/hms/hms_client-test.cc
M src/kudu/hms/hms_client.cc
M src/kudu/hms/mini_hms.cc
M src/kudu/hms/mini_hms.h
M src/kudu/integration-tests/master_hms-itest.cc
M src/kudu/master/catalog_manager.h
8 files changed, 43 insertions(+), 37 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I63e5f1ca6a480f22ede83acde6baea8e2f563e8a
Gerrit-Change-Number: 10102
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Re-enable IWYU checks on HMS module

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10102 )

Change subject: Re-enable IWYU checks on HMS module
..


Patch Set 2:

Is this still an issue after 72847ab5607b64d64bbe43edd942889e4e9f9fbf was 
committed?


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I63e5f1ca6a480f22ede83acde6baea8e2f563e8a
Gerrit-Change-Number: 10102
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 25 Apr 2018 00:24:53 +
Gerrit-HasComments: No

[kudu-CR] java: prohibit use of a KuduTable from an unassociated KuduClient

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/7362 )

Change subject: java: prohibit use of a KuduTable from an unassociated 
KuduClient
..


Patch Set 3: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7362/3/java/kudu-client/src/main/java/org/apache/kudu/client/RequestTracker.java
File java/kudu-client/src/main/java/org/apache/kudu/client/RequestTracker.java:

http://gerrit.cloudera.org:8080/#/c/7362/3/java/kudu-client/src/main/java/org/apache/kudu/client/RequestTracker.java@76
PS3, Line 76: This operation is idempotent.
Nit: with this change this statement becomes a little funky if debug is turned 
on.  Also, with this change the code says 'don't call this method twice'.  
Maybe, remove this statement to avoid confusion?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4a977f36b9c7ba3758322a2216ce90208b5d014
Gerrit-Change-Number: 7362
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Anonymous Coward #314
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 25 Apr 2018 00:23:32 +
Gerrit-HasComments: Yes

[kudu-CR] [WIP] [tools] ksck improvements [6/n]: Refactor printing

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10151 )

Change subject: [WIP] [tools] ksck improvements [6/n]: Refactor printing
..


Patch Set 5:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck_results.cc
File src/kudu/tools/ksck_results.cc:

http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck_results.cc@88
PS5, Line 88:   const string labels = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
static?  It seems this function can be entered multiple times during kudu CLI 
lifecycle.

Also, maybe change this to 'static const char* const' and use (sizeof(xxx)-1) 
instead of size().


http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck_results.cc@124
PS5, Line 124: string ServerHealthToString(KsckServerHealth sh) {
nit: maybe, return 'const char* const' instead?


http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck_results.cc@177
PS5, Line 177:   // Then, summarize the tablets by table
nit: add a stop


http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck_results.cc@268
PS5, Line 268:
nit: two extra spaces


http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck_results.cc@269
PS5, Line 269: "Name", "Status", "Total Tablets",
 : "Healthy", "Recovering", "Under-replicated", 
"Unavailable"
consider adding replication factor, as it's done in 
https://gerrit.cloudera.org/#/c/10054/


http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck_results.cc@273
PS5, Line 273: switch (ts.TableStatus()) {
 :   case KsckCheckResult::HEALTHY:
 : status = "HEALTHY";
 : break;
 :   case KsckCheckResult::RECOVERING:
 : status = "RECOVERING";
 : break;
 :   case KsckCheckResult::UNDER_REPLICATED:
 : status = "UNDER-REPLICATED";
 : break;
 :   default:
 : status = "UNAVAILABLE";
 : break;
 : }
maybe, separate this into a function?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id8de619996b6cd753e6a9c01b1b60810a873e609
Gerrit-Change-Number: 10151
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Tue, 24 Apr 2018 23:52:34 +
Gerrit-HasComments: Yes

[kudu-CR] java: prohibit use of a KuduTable from an unassociated KuduClient

[Todd Lipcon (Code Review)]

Hello Alexey Serbin, David Ribeiro Alves, Jean-Daniel Cryans, Kudu Jenkins, 
Anonymous Coward #314,

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

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

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

Change subject: java: prohibit use of a KuduTable from an unassociated 
KuduClient
..

java: prohibit use of a KuduTable from an unassociated KuduClient

This fixes a request-tracking issue with the following code
anti-pattern in which a KuduTable associated with one client is used to
create operations applied to another client's session:

  KuduClient client1 = KuduClientBuildernewClient();
  KuduTable t = client1.openTable(...);
  KuduClient client2 = KuduClientBuildernewClient();
  KuduSession s = client2.newSession();
  s.apply(t.newUpdate());

This would cause sequence numbers to be generated out of the session's
client's RequestTracker, but then marked complete in the operation's
client's RequestTracker. This could cause any number of issues:

- a cache "hit" on the server side might cause an operation to get an
  unrelated operation's response
- a cache "miss" on the server side might result in an operation
  incorrectly being marked as already-responded and garbage collected,
  causing it to fail.

This patch adds a Preconditions check for this issue and fixes some tests
where it was triggered.

Change-Id: Ib4a977f36b9c7ba3758322a2216ce90208b5d014
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RequestTracker.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITClientStress.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestRequestTracker.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/TestScannerMultiTablet.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java
9 files changed, 103 insertions(+), 57 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib4a977f36b9c7ba3758322a2216ce90208b5d014
Gerrit-Change-Number: 7362
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Anonymous Coward #314
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [docs] Add a navigation link to the scaling guide

[Will Berkeley (Code Review)]

Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10181 )

Change subject: [docs] Add a navigation link to the scaling guide
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3119d39b14c682fa40022ea5e5c199df15a033b2
Gerrit-Change-Number: 10181
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Tue, 24 Apr 2018 22:50:19 +
Gerrit-HasComments: No

[kudu-CR] [WIP] [tools] ksck improvements [6/n]: Refactor printing

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10151 )

Change subject: [WIP] [tools] ksck improvements [6/n]: Refactor printing
..


Patch Set 5:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/10151/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10151/5//COMMIT_MSG@17
PS5, Line 17: - Removes the --consensus flag. ksck is far less useful without 
it and
:   it doesn't seem like having it brings any benefit.
Any concerns about that in terms of backward compatibility?

Also, maybe it's separating that change in a stand-alone changelist?


http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/integration-tests/cluster_verifier.cc
File src/kudu/integration-tests/cluster_verifier.cc:

http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/integration-tests/cluster_verifier.cc@112
PS5, Line 112: RunAndPrintResults
Would it make sense to separate that in two methods:

1. Run()
2. PrintResults()

?


http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck-test.cc
File src/kudu/tools/ksck-test.cc:

http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck-test.cc@845
PS5, Line 845:   Status s = RunKsck();
 :
 :   // Every table we checked was healthy ;).
 :   ASSERT_OK(s);
Here an in other places:

ASSERT_OK(RunKsck())


http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck-test.cc@858
PS5, Line 858:   Status s = RunKsck();
 :
 :   ASSERT_OK(s);
nit: why not just

ASSERT_OK(RunKsck()) ?


http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck-test.cc@878
PS5, Line 878:   Status s = RunKsck();
 :
 :   ASSERT_OK(s);
ASSERT_OK(RunKsck()) ?


http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck_results.h
File src/kudu/tools/ksck_results.h:

http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck_results.h@31
PS5, Line 31: 
why angle brackets instead of quotes?


http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck_results.h@126
PS5, Line 126: // Returns an int signifying the "unhealthiness level" of 'sh'.
 : // Useful for sorting or comparing.
It would be nice to mention what higher score value means, i.e. if server A has 
score 10 and server B has score 20, which server is 'healthier'?


http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck_results.h@133
PS5, Line 133: address
What if server has multiple RPC endpoints?


http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck_results.h@257
PS5, Line 257: const std::vector
nit: why not constant reference?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id8de619996b6cd753e6a9c01b1b60810a873e609
Gerrit-Change-Number: 10151
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Tue, 24 Apr 2018 22:39:27 +
Gerrit-HasComments: Yes

[kudu-CR] [docs] Add a navigation link to the scaling guide

[Grant Henke (Code Review)]

Grant Henke has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10181


Change subject: [docs] Add a navigation link to the scaling guide
..

[docs] Add a navigation link to the scaling guide

Change-Id: I3119d39b14c682fa40022ea5e5c199df15a033b2
---
M docs/support/jekyll-templates/document.html.erb
1 file changed, 1 insertion(+), 0 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3119d39b14c682fa40022ea5e5c199df15a033b2
Gerrit-Change-Number: 10181
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 

[kudu-CR] KUDU-2191: Metadata Upgrade Tool

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10075 )

Change subject: KUDU-2191: Metadata Upgrade Tool
..


Patch Set 6:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/10075/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10075/6//COMMIT_MSG@9
PS6, Line 9: a
nit: an


http://gerrit.cloudera.org:8080/#/c/10075/6//COMMIT_MSG@13
PS6, Line 13: expect
nit: expected


http://gerrit.cloudera.org:8080/#/c/10075/6/build-support/iwyu.py
File build-support/iwyu.py:

http://gerrit.cloudera.org:8080/#/c/10075/6/build-support/iwyu.py@86
PS6, Line 86:   "src/kudu/tools/tool_action_hms.cc",
:   "src/kudu/tools/kudu-tool-test.cc",
nit: just in case if you haven't looked at that yet, Todd added a dependency of 
the IWYU-related targets on related Thrift auto-generated stuff, so this might 
be not necessary anymore.


http://gerrit.cloudera.org:8080/#/c/10075/6/src/kudu/tools/tool_action_hms.cc
File src/kudu/tools/tool_action_hms.cc:

http://gerrit.cloudera.org:8080/#/c/10075/6/src/kudu/tools/tool_action_hms.cc@92
PS6, Line 92: table_name
What if 'table_name' is already a 'new' one?  I.e., my concern is that this 
methods does not look re-enterable, but I might be missing something.  As I 
understand, kudu_client->ListTables() would output not only legacy tables, 
right?


http://gerrit.cloudera.org:8080/#/c/10075/6/src/kudu/tools/tool_action_hms.cc@142
PS6, Line 142: Uris
nit: URIs


http://gerrit.cloudera.org:8080/#/c/10075/6/src/kudu/tools/tool_action_hms.cc@188
PS6, Line 188:   .AddOptionalParameter("hive_metastore_uris")
 :   .AddOptionalParameter("hive_metastore_sasl_enabled")
 :   .AddOptionalParameter("hive_metastore_retry_count")
 :   .AddOptionalParameter("hive_metastore_send_timeout")
 :   .AddOptionalParameter("hive_metastore_recv_timeout")
 :   .AddOptionalParameter("hive_metastore_conn_timeout")
usability nit: does it make sense to remove the 'hive_metastore_' prefix ?  If 
the sub-command is 'hms', doesn't it imply that all those options are about 
Hive metastore?

If not, then maybe at least replace 'hive_metastore_' with 'hms_'?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1c8488e1c7b04b8bedc31c83b1496b53bae49cb3
Gerrit-Change-Number: 10075
Gerrit-PatchSet: 6
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Tue, 24 Apr 2018 20:57:44 +
Gerrit-HasComments: Yes

[kudu-CR] [WIP] [tools] ksck improvements [6/n]: Refactor printing

[Will Berkeley (Code Review)]

Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10151 )

Change subject: [WIP] [tools] ksck improvements [6/n]: Refactor printing
..


Patch Set 5:

> Build Failed

Unrelated flakes.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id8de619996b6cd753e6a9c01b1b60810a873e609
Gerrit-Change-Number: 10151
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Tue, 24 Apr 2018 20:43:10 +
Gerrit-HasComments: No

[kudu-CR] iwyu: add proper dependency on thrift generated code, check for iwyu errors

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10161 )

Change subject: iwyu: add proper dependency on thrift generated code, check for 
iwyu errors
..


Patch Set 2: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10161/1/build-support/iwyu.py
File build-support/iwyu.py:

http://gerrit.cloudera.org:8080/#/c/10161/1/build-support/iwyu.py@52
PS1, Line 52: r'^.+?:\d+:\d+:\s*'
> the FATAL_ERROR case is already handled explicitly in _run_iwyu_tool. I add
Ah, yes -- I found that yesterday as well after reviewing that patch.

Thank you for addressing this tiny nit.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie44f25ebc160c587eb11b4b4730095ee1e6c4fde
Gerrit-Change-Number: 10161
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 24 Apr 2018 19:59:41 +
Gerrit-HasComments: Yes

[kudu-CR] iwyu: add proper dependency on thrift generated code, check for iwyu errors

[Alexey Serbin (Code Review)]

Alexey Serbin has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10161 )

Change subject: iwyu: add proper dependency on thrift generated code, check for 
iwyu errors
..

iwyu: add proper dependency on thrift generated code, check for iwyu errors

IWYU was giving incorrect results on the HMS modules when running on
Jenkins. This was due to the 'iwyu' CMake target not depending on the
thrift code generation. IWYU was continuing to run despite having
'missing include' errors.

This patch adds the appropriate CMake dependency and also makes our
IWYU wrapper check for such errors in the IWYU output.

I also changed the IWYU wrapper to explicitly build the IWYU
dependencies before running so that even if a user invokes it directly,
they won't hit this issue.

Change-Id: Ie44f25ebc160c587eb11b4b4730095ee1e6c4fde
Reviewed-on: http://gerrit.cloudera.org:8080/10161
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin 
---
M CMakeLists.txt
M build-support/iwyu.py
2 files changed, 19 insertions(+), 5 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Alexey Serbin: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie44f25ebc160c587eb11b4b4730095ee1e6c4fde
Gerrit-Change-Number: 10161
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [WIP] [tools] ksck improvements [6/n]: Refactor printing

[Will Berkeley (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: [WIP] [tools] ksck improvements [6/n]: Refactor printing
..

[WIP] [tools] ksck improvements [6/n]: Refactor printing

This patch significantly refactors printing in ksck. It separates where
checks are done from where results are printed. This separation will
make it easier to provide machine-readable output and create tools
based on the results of ksck checks, like auto-repair tools.

There's also a couple of bonus changes:
- Simplifies running all ksck checks and printing the results, as done
  by the ksck tool and ClusterVerifier.
- Removes the --consensus flag. ksck is far less useful without it and
  it doesn't seem like having it brings any benefit.

Change-Id: Id8de619996b6cd753e6a9c01b1b60810a873e609
---
M src/kudu/integration-tests/cluster_verifier.cc
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote-test.cc
A src/kudu/tools/ksck_results.cc
A src/kudu/tools/ksck_results.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/tool_action_cluster.cc
M src/kudu/tools/tool_action_tablet.cc
11 files changed, 966 insertions(+), 781 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id8de619996b6cd753e6a9c01b1b60810a873e609
Gerrit-Change-Number: 10151
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] WIP: bootstrap: don't re-acquire row locks for operations that are skipped

[Todd Lipcon (Code Review)]

Todd Lipcon has abandoned this change. ( http://gerrit.cloudera.org:8080/3040 )

Change subject: WIP: bootstrap: don't re-acquire row locks for operations that 
are skipped
..


Abandoned

This still applies but I couldn't benchmark a very useful improvement here, so 
abandoning until we have better data
--
To view, visit http://gerrit.cloudera.org:8080/3040
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I8d050965b8e882b91463af96714b558d7a6652de
Gerrit-Change-Number: 3040
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] WIP: KUDU-1366: allow building against jemalloc

[Todd Lipcon (Code Review)]

Todd Lipcon has abandoned this change. ( http://gerrit.cloudera.org:8080/6918 )

Change subject: WIP: KUDU-1366: allow building against jemalloc
..


Abandoned

With recent bug fixes in tcmalloc I think this isn't worth pursuing in the near 
or medium term
--
To view, visit http://gerrit.cloudera.org:8080/6918
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I59f77e9f1ebdf6f1451e0f723a91cda4790c434b
Gerrit-Change-Number: 6918
Gerrit-PatchSet: 10
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] tool: improve format for dumping a rowset

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/3946 )

Change subject: tool: improve format for dumping a rowset
..


Patch Set 3:

IWYU error due to a transitive include of the thrift stuff. I'll rebase this on 
the fix


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0f1d08e08d2a3d20a87e49bb5338bf0585bd8e40
Gerrit-Change-Number: 3946
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Tue, 24 Apr 2018 18:58:39 +
Gerrit-HasComments: No

[kudu-CR] iwyu: add proper dependency on thrift generated code, check for iwyu errors

[Todd Lipcon (Code Review)]

Hello Alexey Serbin, Kudu Jenkins,

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

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

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

Change subject: iwyu: add proper dependency on thrift generated code, check for 
iwyu errors
..

iwyu: add proper dependency on thrift generated code, check for iwyu errors

IWYU was giving incorrect results on the HMS modules when running on
Jenkins. This was due to the 'iwyu' CMake target not depending on the
thrift code generation. IWYU was continuing to run despite having
'missing include' errors.

This patch adds the appropriate CMake dependency and also makes our
IWYU wrapper check for such errors in the IWYU output.

I also changed the IWYU wrapper to explicitly build the IWYU
dependencies before running so that even if a user invokes it directly,
they won't hit this issue.

Change-Id: Ie44f25ebc160c587eb11b4b4730095ee1e6c4fde
---
M CMakeLists.txt
M build-support/iwyu.py
2 files changed, 19 insertions(+), 5 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie44f25ebc160c587eb11b4b4730095ee1e6c4fde
Gerrit-Change-Number: 10161
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] iwyu: add proper dependency on thrift generated code, check for iwyu errors

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10161 )

Change subject: iwyu: add proper dependency on thrift generated code, check for 
iwyu errors
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10161/1/build-support/iwyu.py
File build-support/iwyu.py:

http://gerrit.cloudera.org:8080/#/c/10161/1/build-support/iwyu.py@52
PS1, Line 52: r'^.+?:\d+:\d+: (fatal )?error:'
> In $IWYU_SRC_ROOT/iwyu_test_util.py there is pattern for expected error/war
the FATAL_ERROR case is already handled explicitly in _run_iwyu_tool. I added 
the \s* instead of ' '



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie44f25ebc160c587eb11b4b4730095ee1e6c4fde
Gerrit-Change-Number: 10161
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 24 Apr 2018 18:03:28 +
Gerrit-HasComments: Yes

[kudu-CR] [WIP] [tools] ksck improvements [6/n]: Refactor printing

[Will Berkeley (Code Review)]

Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10151 )

Change subject: [WIP] [tools] ksck improvements [6/n]: Refactor printing
..


Patch Set 4:

> Uploaded patch set 4.

Junk rev with some logging to identify the cause of a test failure I can't 
reproduce.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id8de619996b6cd753e6a9c01b1b60810a873e609
Gerrit-Change-Number: 10151
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Tue, 24 Apr 2018 17:59:26 +
Gerrit-HasComments: No

[kudu-CR] [WIP] [tools] ksck improvements [6/n]: Refactor printing

[Will Berkeley (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: [WIP] [tools] ksck improvements [6/n]: Refactor printing
..

[WIP] [tools] ksck improvements [6/n]: Refactor printing

This patch significantly refactors printing in ksck. It separates where
checks are done from where results are printed. This separation will
make it easier to provide machine-readable output and create tools
based on the results of ksck checks, like auto-repair tools.

There's also a couple of bonus changes:
- Simplifies running all ksck checks and printing the results, as done
  by the ksck tool and ClusterVerifier.
- Removes the --consensus flag. ksck is far less useful without it and
  it doesn't seem like having it brings any benefit.

Change-Id: Id8de619996b6cd753e6a9c01b1b60810a873e609
---
M src/kudu/integration-tests/cluster_verifier.cc
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote-test.cc
A src/kudu/tools/ksck_results.cc
A src/kudu/tools/ksck_results.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/tool_action_cluster.cc
M src/kudu/tools/tool_action_tablet.cc
11 files changed, 968 insertions(+), 781 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id8de619996b6cd753e6a9c01b1b60810a873e609
Gerrit-Change-Number: 10151
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] tool: improve format for dumping a rowset

[Todd Lipcon (Code Review)]

Hello Will Berkeley, Tidy Bot, Kudu Jenkins, Adar Dembo, Dinesh Bhat,

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

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

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

Change subject: tool: improve format for dumping a rowset
..

tool: improve format for dumping a rowset

This changes the output for 'kudu local_replica dump rowset' to be more
human-readable. The output now looks like this:

RowIdxInBlock: 0; Base: (int32 key=0, int32 int_val=0, string 
string_val="HelloWorld"); Undo Mutations: [@1(DELETE)]; Redo Mutations: [];
RowIdxInBlock: 1; Base: (int32 key=1, int32 int_val=10, string 
string_val="HelloWorld"); Undo Mutations: [@2(DELETE)]; Redo Mutations: [];
RowIdxInBlock: 2; Base: (int32 key=2, int32 int_val=20, string 
string_val="HelloWorld"); Undo Mutations: [@3(DELETE)]; Redo Mutations: [];
...

rather than separately dumping each column block. Dumping individual blocks is
still possible by using the cfile dump commands.

Change-Id: I0f1d08e08d2a3d20a87e49bb5338bf0585bd8e40
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_local_replica.cc
2 files changed, 23 insertions(+), 217 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0f1d08e08d2a3d20a87e49bb5338bf0585bd8e40
Gerrit-Change-Number: 3946
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] KUDU-2214 Update logging to differentiate voting while copying/tombstoned

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10169 )

Change subject: KUDU-2214 Update logging to differentiate voting while 
copying/tombstoned
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10169/1/src/kudu/consensus/raft_consensus.cc
File src/kudu/consensus/raft_consensus.cc:

http://gerrit.cloudera.org:8080/#/c/10169/1/src/kudu/consensus/raft_consensus.cc@1561
PS1, Line 1561:   LOG_WITH_PREFIX_UNLOCKED(INFO) << "voting based on 
last-logged opid "
> Sounds good. Thanks. I'll try this. Could you tell me why doing this btw? I
Sure. The reason is that the log message today is misleading -- it says that 
it's voting while tombstoned but it's not tombstoned. This may lead the 
operator to thinking that a tablet is in a different state than it actually is.

I would also be OK with just removing the words "while tombstoned" above and 
not changing anything else. Maybe Mike has an opinion about that.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I07007601d0a86d6161065629ba167121a33635d6
Gerrit-Change-Number: 10169
Gerrit-PatchSet: 1
Gerrit-Owner: Fengling Wang 
Gerrit-Reviewer: Fengling Wang 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Tue, 24 Apr 2018 17:09:14 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2214 Update logging to differentiate voting while copying/tombstoned

[Fengling Wang (Code Review)]

Fengling Wang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10169 )

Change subject: KUDU-2214 Update logging to differentiate voting while 
copying/tombstoned
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10169/1/src/kudu/consensus/raft_consensus.cc
File src/kudu/consensus/raft_consensus.cc:

http://gerrit.cloudera.org:8080/#/c/10169/1/src/kudu/consensus/raft_consensus.cc@1561
PS1, Line 1561:   LOG_WITH_PREFIX_UNLOCKED(INFO) << "voting based on 
last-logged opid "
> One idea here is to change the signature of this method so that instead of
Sounds good. Thanks. I'll try this. Could you tell me why doing this btw? I 
feel like changing the signature is going affect quite a few use cases.?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I07007601d0a86d6161065629ba167121a33635d6
Gerrit-Change-Number: 10169
Gerrit-PatchSet: 1
Gerrit-Owner: Fengling Wang 
Gerrit-Reviewer: Fengling Wang 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Tue, 24 Apr 2018 16:49:36 +
Gerrit-HasComments: Yes

[kudu-CR] iwyu: mute non-source files when using compilation DB

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10160 )

Change subject: iwyu: mute non-source files when using compilation DB
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10160/1/build-support/iwyu.py
File build-support/iwyu.py:

http://gerrit.cloudera.org:8080/#/c/10160/1/build-support/iwyu.py@149
PS1, Line 149: not rel.startswith('src/')
> Is that for various auto-generated stuff under build directory?
yea



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd00ab084536353d2a9a733fc0a11197b8757df4
Gerrit-Change-Number: 10160
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 24 Apr 2018 16:44:19 +
Gerrit-HasComments: Yes

[kudu-CR] iwyu: mute non-source files when using compilation DB

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10160 )

Change subject: iwyu: mute non-source files when using compilation DB
..

iwyu: mute non-source files when using compilation DB

When running 'iwyu.py --all', it would report various IWYU violations in
included header files. This mutes any reports of issues outside of our
src directory.

Change-Id: Ibd00ab084536353d2a9a733fc0a11197b8757df4
Reviewed-on: http://gerrit.cloudera.org:8080/10160
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin 
---
M build-support/iwyu.py
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Kudu Jenkins: Verified
  Alexey Serbin: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ibd00ab084536353d2a9a733fc0a11197b8757df4
Gerrit-Change-Number: 10160
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] iwyu: pass source root into fix includes

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10159 )

Change subject: iwyu: pass source root into fix_includes
..

iwyu: pass source root into fix_includes

fix_includes tries to identify the "main compilation unit header" for
each file using some heuristics. For example, "foo-test.cc" is
associated with "foo.h".

This was previously broken due to the fact that the tool runs from the
build/ directory instead of the source directory, so it saw the CC file
as "../../src/kudu/.../foo.cc" while it saw the header file as
"kudu/.../foo.h".

This adds a --source_root parameter to fix_includes so that the source
files can be relativized to this path prior to fixing.

Change-Id: Iac9f0b47b9a3c8180e178ed59863d33840434433
Reviewed-on: http://gerrit.cloudera.org:8080/10159
Tested-by: Todd Lipcon 
Reviewed-by: Alexey Serbin 
---
M build-support/iwyu.py
M build-support/iwyu/fix_includes.py
2 files changed, 24 insertions(+), 14 deletions(-)

Approvals:
  Todd Lipcon: Verified
  Alexey Serbin: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Iac9f0b47b9a3c8180e178ed59863d33840434433
Gerrit-Change-Number: 10159
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] iwyu: workaround missing IWYU results when using Ninja

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9554 )

Change subject: iwyu: workaround missing IWYU results when using Ninja
..

iwyu: workaround missing IWYU results when using Ninja

This adds a workaround so that IWYU reports the same results whether
cmake is invoked with the Makefile generator or the Ninja generator.

Tested by verifying that IWYU on my laptop now reports the same issues
that Jenkins was reporting precommit on a recent in-flight patch.

Change-Id: I092cab794fd20a39fcfbbbe52f4c325fac2d149c
Reviewed-on: http://gerrit.cloudera.org:8080/9554
Tested-by: Todd Lipcon 
Reviewed-by: Alexey Serbin 
---
M build-support/iwyu/iwyu_tool.py
1 file changed, 43 insertions(+), 0 deletions(-)

Approvals:
  Todd Lipcon: Verified
  Alexey Serbin: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I092cab794fd20a39fcfbbbe52f4c325fac2d149c
Gerrit-Change-Number: 9554
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] iwyu: pass source root into fix includes

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10159 )

Change subject: iwyu: pass source root into fix_includes
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10159/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10159/1//COMMIT_MSG@15
PS1, Line 15: .
> nit: an extra dot?
no, I mean '...' as in 'fill in some path here'



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iac9f0b47b9a3c8180e178ed59863d33840434433
Gerrit-Change-Number: 10159
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 24 Apr 2018 16:43:44 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2214 Update logging to differentiate voting while copying/tombstoned

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10169 )

Change subject: KUDU-2214 Update logging to differentiate voting while 
copying/tombstoned
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10169/1/src/kudu/consensus/raft_consensus.cc
File src/kudu/consensus/raft_consensus.cc:

http://gerrit.cloudera.org:8080/#/c/10169/1/src/kudu/consensus/raft_consensus.cc@1561
PS1, Line 1561:   LOG_WITH_PREFIX_UNLOCKED(INFO) << "voting based on 
last-logged opid "
One idea here is to change the signature of this method so that instead of just 
taking in OpId last_logged_opid, it instead takes some simple struct like:

struct TabletVotingState {
  optional last_logged_op_id;
  TabletDataState data_state;
};

and then you can modify this message to switch on the passed-in data state.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I07007601d0a86d6161065629ba167121a33635d6
Gerrit-Change-Number: 10169
Gerrit-PatchSet: 1
Gerrit-Owner: Fengling Wang 
Gerrit-Reviewer: Fengling Wang 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Tue, 24 Apr 2018 16:21:12 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2214 Update logging to differentiate voting while copying/tombstoned

[Fengling Wang (Code Review)]

Fengling Wang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10169 )

Change subject: KUDU-2214 Update logging to differentiate voting while 
copying/tombstoned
..


Patch Set 1:

(1 comment)

So far I don't know the best approach to solve this issue. Hoping for 
suggestions. Thanks.

http://gerrit.cloudera.org:8080/#/c/10169/1/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

http://gerrit.cloudera.org:8080/#/c/10169/1/src/kudu/tserver/tablet_service.cc@1006
PS1, Line 1006: LOG(INFO) << "Attempting to vote while "
> does this end up doubling the number of logs during a vote?
yes actually.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I07007601d0a86d6161065629ba167121a33635d6
Gerrit-Change-Number: 10169
Gerrit-PatchSet: 1
Gerrit-Owner: Fengling Wang 
Gerrit-Reviewer: Fengling Wang 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Tue, 24 Apr 2018 15:44:47 +
Gerrit-HasComments: Yes

[kudu-CR] Use Gradle by default in build-and-test.sh

[Grant Henke (Code Review)]

Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10162 )

Change subject: Use Gradle by default in build-and-test.sh
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10162/3/build-support/jenkins/build-and-test.sh
File build-support/jenkins/build-and-test.sh:

http://gerrit.cloudera.org:8080/#/c/10162/3/build-support/jenkins/build-and-test.sh@392
PS3, Line 392:  GRADLE_FLAGS="$GRADLE_FLAGS --console=plain --no-daemon 
--continue"
> What's the significance of --continue? Why is it needed here?
I will add this in a separate patch, but I am just experimenting here.

continue makes sure the whole build runs regardless of failure so all failures 
are reported.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia919af374f4909d2b8eb38d6c7040704eb24b457
Gerrit-Change-Number: 10162
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 24 Apr 2018 15:27:43 +
Gerrit-HasComments: Yes

[kudu-CR] Use Gradle by default in build-and-test.sh

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10162 )

Change subject: Use Gradle by default in build-and-test.sh
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10162/3/build-support/jenkins/build-and-test.sh
File build-support/jenkins/build-and-test.sh:

http://gerrit.cloudera.org:8080/#/c/10162/3/build-support/jenkins/build-and-test.sh@392
PS3, Line 392:  GRADLE_FLAGS="$GRADLE_FLAGS --console=plain --no-daemon 
--continue"
What's the significance of --continue? Why is it needed here?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia919af374f4909d2b8eb38d6c7040704eb24b457
Gerrit-Change-Number: 10162
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 24 Apr 2018 15:26:17 +
Gerrit-HasComments: Yes

[kudu-CR] Use Gradle by default in build-and-test.sh

[Grant Henke (Code Review)]

Hello Kudu Jenkins, Adar Dembo,

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

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

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

Change subject: Use Gradle by default in build-and-test.sh
..

Use Gradle by default in build-and-test.sh

Change-Id: Ia919af374f4909d2b8eb38d6c7040704eb24b457
---
M build-support/jenkins/build-and-test.sh
1 file changed, 5 insertions(+), 5 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia919af374f4909d2b8eb38d6c7040704eb24b457
Gerrit-Change-Number: 10162
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins