[kudu-CR] [java client] KUDU-1643 Prune hash partitions based on IN-list predicates

2017-02-07 Thread Dan Burkert (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: [java client] KUDU-1643 Prune hash partitions based on IN-list 
predicates
..

[java client] KUDU-1643 Prune hash partitions based on IN-list predicates

PartitionPruner is updated to search all combinations of in-list column
values and prune hash partitions where possible.

This also fixes a small issue in the C++ version of the algorithm:
previously the implementation would always consider the final hash
component to be constrained. The bug (as far as I can tell) doesn't lead
to erroneous results, but does cause more partition ranges to be
created, which results in more memory overhead and compute overhead to
compute the pruning.

Change-Id: I8a793c23ff00d19b3d3d062bb222d2c725a93724
---
M java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartitionPruner.java
M src/kudu/common/partial_row.h
M src/kudu/common/partition_pruner-test.cc
M src/kudu/common/partition_pruner.cc
M src/kudu/common/partition_pruner.h
M src/kudu/common/scan_spec.cc
7 files changed, 520 insertions(+), 332 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/5677/7
-- 
To view, visit http://gerrit.cloudera.org:8080/5677
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8a793c23ff00d19b3d3d062bb222d2c725a93724
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Haijie Hong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] Fix TSAN segfault on Centos 6

2017-02-07 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: Fix TSAN segfault on Centos 6
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ea3ad372f98ff423425858c09cb0977f372b2e5
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] Fix TSAN segfault on Centos 6

2017-02-07 Thread Dan Burkert (Code Review)
Hello Todd Lipcon,

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

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

to review the following change.

Change subject: Fix TSAN segfault on Centos 6
..

Fix TSAN segfault on Centos 6

The OpenSSL version shipped with Centos 6 appears to segfault if a
nullptr is passed to OBJ_find_sigid_algs with TSAN. I haven't been able
to actually figure out why, I haven't been succesful in finding the
Centos 6 source for OpenSSL, and the version it's based on correctly
checks for null[1]. Attempting to set a breakpoint on the function
yielded no source, even with the debuginfo OpenSSL rpm installed.

[1] 
https://github.com/openssl/openssl/blob/OpenSSL_1_0_1e/crypto/objects/obj_xref.c#L115

Change-Id: I5ea3ad372f98ff423425858c09cb0977f372b2e5
---
M src/kudu/security/cert.cc
1 file changed, 2 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I5ea3ad372f98ff423425858c09cb0977f372b2e5
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] Add KuduTable::formatted range partitions in cpp client.

2017-02-07 Thread zhen.zhang (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: Add KuduTable::formatted_range_partitions in cpp client.
..

Add KuduTable::formatted_range_partitions in cpp client.

This add the KuduTable::formatted_range_partitions method in cpp
client to retrive the list of range partitions in a table.

Change-Id: I7a379fb29932c4160724fe8c71f85f2c8e994469
---
M src/kudu/client/client-test.cc
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/meta_cache.cc
M src/kudu/client/meta_cache.h
5 files changed, 167 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7a379fb29932c4160724fe8c71f85f2c8e994469
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: zhen.zhang 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] java: remove unused parts of SecureRpcHelper

2017-02-07 Thread Todd Lipcon (Code Review)
Hello Dan Burkert, Jean-Daniel Cryans,

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

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

to review the following change.

Change subject: java: remove unused parts of SecureRpcHelper
..

java: remove unused parts of SecureRpcHelper

This removes the unused wrap/unwrap code from SecureRpcHelper. I also
removed an unnecessary function in TabletClient.

Change-Id: I3b70e0319060e3f2d9cc9a0ed23da7fe51b174df
---
M java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java
M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java
2 files changed, 3 insertions(+), 55 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I3b70e0319060e3f2d9cc9a0ed23da7fe51b174df
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 


[kudu-CR] Make SecureRpcHelper a Netty pipeline stage

2017-02-07 Thread Todd Lipcon (Code Review)
Hello Dan Burkert, Jean-Daniel Cryans,

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

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

to review the following change.

Change subject: Make SecureRpcHelper a Netty pipeline stage
..

Make SecureRpcHelper a Netty pipeline stage

This decouples SecureRpcHelper from TabletClient. When negotiation is
complete, it sends an "event" upstream to the TabletClient and removes
itself from the pipeline.

I'm hoping this decoupling will make it easier to test SecureRpcHelper
in isolation as we add more functionality like TLS negotiation.

After this is committed, I'm also hoping to rename SecureRpcHelper to
Negotiator.

Change-Id: I5f4b4516219b8eebf24786b4ceb13f2e6260f03b
---
M java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java
M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java
2 files changed, 85 insertions(+), 76 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I5f4b4516219b8eebf24786b4ceb13f2e6260f03b
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 


[kudu-CR] java: use a netty frame decoder instead of replaying decoder

2017-02-07 Thread Todd Lipcon (Code Review)
Hello Dan Burkert, Jean-Daniel Cryans,

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

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

to review the following change.

Change subject: java: use a netty frame decoder instead of replaying decoder
..

java: use a netty frame decoder instead of replaying decoder

All of our inbound packets are length-prefixed. Netty provides a nice
class for handing length-prefixed messages. This avoids a bunch of more
custom logic we were doing for length checking, etc.

Change-Id: I1ec1e6f9cbbdf694fe85b7668a3c349efc26c08f
---
M java/kudu-client/src/main/java/org/apache/kudu/client/CallResponse.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.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/SecureRpcHelper.java
M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java
5 files changed, 48 insertions(+), 125 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I1ec1e6f9cbbdf694fe85b7668a3c349efc26c08f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 


[kudu-CR] java: use a netty frame decoder instead of replaying decoder

2017-02-07 Thread Jean-Daniel Cryans (Code Review)
Jean-Daniel Cryans has posted comments on this change.

Change subject: java: use a netty frame decoder instead of replaying decoder
..


Patch Set 1:

(2 comments)

Good on you for addressing this.  I'd be curious to see how it improves 
performance.

http://gerrit.cloudera.org:8080/#/c/5926/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java:

Line 72:   
I see red.


http://gerrit.cloudera.org:8080/#/c/5926/1/java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java:

Line 174: // TODO(todd): shouldn't this return here?
Yes.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ec1e6f9cbbdf694fe85b7668a3c349efc26c08f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes


[kudu-CR] java: remove unused parts of SecureRpcHelper

2017-02-07 Thread Jean-Daniel Cryans (Code Review)
Jean-Daniel Cryans has posted comments on this change.

Change subject: java: remove unused parts of SecureRpcHelper
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3b70e0319060e3f2d9cc9a0ed23da7fe51b174df
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No


[kudu-CR] Make SecureRpcHelper a Netty pipeline stage

2017-02-07 Thread Jean-Daniel Cryans (Code Review)
Jean-Daniel Cryans has posted comments on this change.

Change subject: Make SecureRpcHelper a Netty pipeline stage
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5927/1/java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java
File java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java:

Line 113:   
RED
And same farther below.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5f4b4516219b8eebf24786b4ceb13f2e6260f03b
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes


[kudu-CR] master rpc: pass back more details from ConnectToCluster

2017-02-07 Thread Todd Lipcon (Code Review)
Todd Lipcon has submitted this change and it was merged.

Change subject: master_rpc: pass back more details from ConnectToCluster
..


master_rpc: pass back more details from ConnectToCluster

This plumbs back the ConnectToMasterResponsePB from the ConnectToCluster
RPC. This brings us one step closer to inserting the CA cert into the
TlsContext and passing the token somewhere useful.

Along the way, I switched to std::function instead of gutil Callback
since it allows the slightly easier lambda syntax.

No new tests here since all the existing client tests cover this code
path.

Change-Id: I3b5056c9f31237249516a2e7ae68d641c9f6bd02
Reviewed-on: http://gerrit.cloudera.org:8080/5892
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin 
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/master_rpc.cc
M src/kudu/client/master_rpc.h
M src/kudu/integration-tests/external_mini_cluster.cc
5 files changed, 63 insertions(+), 56 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I3b5056c9f31237249516a2e7ae68d641c9f6bd02
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] master: issue authentication tokens and CA certs to clients

2017-02-07 Thread Todd Lipcon (Code Review)
Todd Lipcon has submitted this change and it was merged.

Change subject: master: issue authentication tokens and CA certs to clients
..


master: issue authentication tokens and CA certs to clients

This adds the code to the ConnectToMaster RPC to issue an authentication
token for the connecting user, as well as to send it the current master
CA cert.

The client side currently ignores both things, but a new unit test
verifies that they are getting properly set.

As with most of these early integration patches, there are plenty of
TODOs about integrating with the catalog manager, etc, but this should
be enough to unblock other client-side work of propagating the CA and
tokens into the RPC system.

Change-Id: I5969b8e125633b3b14364b98c0d0a992b162f302
Reviewed-on: http://gerrit.cloudera.org:8080/5871
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin 
---
M src/kudu/master/CMakeLists.txt
A src/kudu/master/authn_token_manager.cc
A src/kudu/master/authn_token_manager.h
M src/kudu/master/master-test.cc
M src/kudu/master/master.cc
M src/kudu/master/master.h
M src/kudu/master/master.proto
M src/kudu/master/master_cert_authority.cc
M src/kudu/master/master_cert_authority.h
M src/kudu/master/master_service.cc
M src/kudu/rpc/outbound_call.h
M src/kudu/security/token.proto
12 files changed, 256 insertions(+), 3 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I5969b8e125633b3b14364b98c0d0a992b162f302
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] client: trust master's cert, adopt authn token

2017-02-07 Thread Todd Lipcon (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: client: trust master's cert, adopt authn token
..

client: trust master's cert, adopt authn token

This changes the client to actually act on the security-related data
coming back from the ConnectToMaster() RPC. The client now adds the
master's CA cert to its trusted list, and adopts the authentication
token in a class member.

The token isn't used yet -- that's waiting on some concurrent work by
Dan to use tokens during RPC negotiation. But, a unit test verifies that
the token is getting set.

Change-Id: Iaa3eec178110b77d81608cfdce989e0fe494ee05
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/client-test.cc
M src/kudu/client/client.h
M src/kudu/master/master.proto
M src/kudu/rpc/messenger.h
M src/kudu/security/tls_context.cc
M src/kudu/security/tls_context.h
8 files changed, 79 insertions(+), 4 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iaa3eec178110b77d81608cfdce989e0fe494ee05
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] client: trust master's cert, adopt authn token

2017-02-07 Thread Todd Lipcon (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: client: trust master's cert, adopt authn token
..

client: trust master's cert, adopt authn token

This changes the client to actually act on the security-related data
coming back from the ConnectToMaster() RPC. The client now adds the
master's CA cert to its trusted list, and adopts the authentication
token in a class member.

The token isn't used yet -- that's waiting on some concurrent work by
Dan to use tokens during RPC negotiation. But, a unit test verifies that
the token is getting set.

Change-Id: Iaa3eec178110b77d81608cfdce989e0fe494ee05
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/client-test.cc
M src/kudu/client/client.h
M src/kudu/master/master.proto
M src/kudu/rpc/messenger.h
M src/kudu/security/tls_context.cc
M src/kudu/security/tls_context.h
8 files changed, 79 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/99/5899/7
-- 
To view, visit http://gerrit.cloudera.org:8080/5899
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iaa3eec178110b77d81608cfdce989e0fe494ee05
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] client: trust master's cert, adopt authn token

2017-02-07 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change.

Change subject: client: trust master's cert, adopt authn token
..


Patch Set 7: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaa3eec178110b77d81608cfdce989e0fe494ee05
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] WIP: fixes for java client kerberos against a real cluster

2017-02-07 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change.

Change subject: WIP: fixes for java client kerberos against a real cluster
..


Patch Set 2:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/5922/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:

PS2, Line 1834:  "
use {} substitution


Line 1850: options.put("debug", "" + 
Boolean.getBoolean("kudu.jaas.debug"));
Boolean.toString(Boolean.getBoolean("kudu.jaas.debug"))


Line 1871: LOG.debug("Logged in as subject: " + subject.toString());
{} substitution


http://gerrit.cloudera.org:8080/#/c/5922/2/java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java
File java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java:

Line 116: if (saslClient == null || !saslClient.isComplete() || 
negoUnderway) {
Looks like this could this be simplified to just 'if (negoUnderway)'


Line 206: for (RpcHeader.NegotiatePB.SaslAuth auth : 
response.getAuthsList()) {
This shouldn't be done in a loop.  The Java client should intersect the 
server-supported mechs with it's own, and then preferentially pick from that 
set (GSSAPI over PLAIN).  See C++ client.

https://github.com/apache/kudu/blob/master/src/kudu/rpc/client_negotiation.cc#L329-L377


Line 270:   if (e instanceof SaslException) {
This would be simpler as a separate catch clause.

} catch (RuntimeException|SaslException e) {
throw e;
} catch (Exception e) {
throw new RuntimeException(e);
}


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6b96fad3cfb40500d7a75e5070ea21bc8e00cbd8
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes


[kudu-CR] Make SecureRpcHelper a Netty pipeline stage

2017-02-07 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change.

Change subject: Make SecureRpcHelper a Netty pipeline stage
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5927/1/java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java
File java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java:

Line 127: RpcHeader.NegotiatePB response = parseSaslMsgResponse(buf);
This needs to check for an error response, or maybe punt and add a TODO.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5f4b4516219b8eebf24786b4ceb13f2e6260f03b
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes


[kudu-CR] compaction: Flush tablet metadata before updating stores

2017-02-07 Thread Mike Percy (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: compaction: Flush tablet metadata before updating stores
..

compaction: Flush tablet metadata before updating stores

We should always flush the rowset metadata before making changes visible
to the delta tracker. In this case, because we are just doing a
compaction, it is likely not a crash consistency issue, however we
should always follow the same order of operations when operating on
tablet metadata.

This change also allows us to defer a crash to a point higher in the
call stack.

Also, add LogPrefix() to the log lines in the delta tracker.

Change-Id: I958f4744f1b95c6a63e8c9105bd1c7552e43e6b9
---
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/delta_tracker.h
2 files changed, 56 insertions(+), 34 deletions(-)


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

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


[kudu-CR] Add Google Breakpad support to Kudu

2017-02-07 Thread Mike Percy (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: Add Google Breakpad support to Kudu
..

Add Google Breakpad support to Kudu

Breakpad creates minidumps upon crash, which are small files that
include stack-based information for each thread. These do not include
heap information, so they complement, not replace, core files for
debugging purposes.

This patch enables Google Breakpad for the kudu-tserver and kudu-master
processes.

Breakpad is part of the crash-reporting infrastructure open-sourced by
Google. It is also used by Mozilla Firefox and Google Chrome.

When a process crashes, Breakpad writes a small dump file called a
minidump to disk. Since these files are typically only a few MB in size,
this allows users to share critical crash information with developers
for offline analysis.

For more information, see the breakpad getting started docs at
http://chromium.googlesource.com/breakpad/breakpad/+/master/docs/getting_started_with_breakpad.md

By default, on crash, minidump files are written to a subdirectory of
the log directory for a given Kudu daemon process. This was chosen for
the following reasons:

1. The minidump files are relatively small, potentially comparable in
   size to log files, and the log directory is a place an administrator
   would look in when debugging.
2. This convention is consistent with what Apache Impala (incubating)
   does for its minidump files.

Changes in this patch:

* Add breakpad to thirdparty.
* Add breakpad source archive creation script (Breakpad does not do
  releases).
* Add a "hack" to the breakpad thirdparty installation routine to
  forcibly add a breakpad/ prefix to the breakpad header files. This
  frees us from having to modify our include path when building.
* Pull in (heavily modified) util/minidump.{cc,h} from Apache Impala
  (incubating). The modifications consist of removing use of
  boost::filesystem, conformance to the Kudu code style guidelines, and
  some significant refactoring.
* Enable breakpad support for the kudu-tserver and kudu-master
  processes.
* By default, invoke previously-installed signal handler if installed.
* Handle SIGUSR1 by safely using sigwait() to create minidumps on demand.
* Add #ifdefs and CMake logic for Linux.
* Add test for all the deadly signals to ensure we get both a stack
  trace and a minidump, except for the case of SIGTERM, where we should
  not get a minidump (by design).
* Change a few library unit tests that relied on SIGUSR1 to use another
  signal for their testing, such as SIGHUP or SIGUSR2.
* Remove unused includes from mini_tablet_server.cc
* Clean up signal handling logic when forking a subprocess.
* Hide breakpad symbols from libkuduclient.so
* Install failure function that simply calls abort() instead of first
  printing a stacktrace.

Change-Id: I495695cc38b75377f20b0497093a81ed5baa887f
---
M CMakeLists.txt
M build-support/jenkins/build-and-test.sh
A cmake_modules/FindBreakpadClient.cmake
M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java
M src/kudu/client/client_samples-test.sh
M src/kudu/client/symbols.map
M src/kudu/integration-tests/CMakeLists.txt
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/integration-tests/external_mini_cluster.h
A src/kudu/integration-tests/minidump_generation-itest.cc
M src/kudu/master/mini_master.cc
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/debug-util-test.cc
M src/kudu/util/env_posix.cc
M src/kudu/util/logging.cc
A src/kudu/util/minidump-test.cc
A src/kudu/util/minidump.cc
A src/kudu/util/minidump.h
M src/kudu/util/subprocess-test.cc
M src/kudu/util/subprocess.cc
M src/kudu/util/test_main.cc
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
M thirdparty/download-thirdparty.sh
M thirdparty/preflight.py
A thirdparty/scripts/make-breakpad-src-archive.sh
M thirdparty/vars.sh
30 files changed, 994 insertions(+), 84 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/5620/30
-- 
To view, visit http://gerrit.cloudera.org:8080/5620
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I495695cc38b75377f20b0497093a81ed5baa887f
Gerrit-PatchSet: 30
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] [thirdparty] Make Boost a regular dependency

2017-02-07 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change.

Change subject: [thirdparty] Make Boost a regular dependency
..


Patch Set 10: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I277c4fda15575e271c426735552762884cb28c43
Gerrit-PatchSet: 10
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: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] Add Google Breakpad support to Kudu

2017-02-07 Thread Mike Percy (Code Review)
Hello Adar Dembo, Kudu Jenkins,

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

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

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

Change subject: Add Google Breakpad support to Kudu
..

Add Google Breakpad support to Kudu

Breakpad creates minidumps upon crash, which are small files that
include stack-based information for each thread. These do not include
heap information, so they complement, not replace, core files for
debugging purposes.

This patch enables Google Breakpad for the kudu-tserver and kudu-master
processes.

Breakpad is part of the crash-reporting infrastructure open-sourced by
Google. It is also used by Mozilla Firefox and Google Chrome.

When a process crashes, Breakpad writes a small dump file called a
minidump to disk. Since these files are typically only a few MB in size,
this allows users to share critical crash information with developers
for offline analysis.

For more information, see the breakpad getting started docs at
http://chromium.googlesource.com/breakpad/breakpad/+/master/docs/getting_started_with_breakpad.md

By default, on crash, minidump files are written to a subdirectory of
the log directory for a given Kudu daemon process. This was chosen for
the following reasons:

1. The minidump files are relatively small, potentially comparable in
   size to log files, and the log directory is a place an administrator
   would look in when debugging.
2. This convention is consistent with what Apache Impala (incubating)
   does for its minidump files.

Changes in this patch:

* Add breakpad to thirdparty.
* Add breakpad source archive creation script (Breakpad does not do
  releases).
* Add a "hack" to the breakpad thirdparty installation routine to
  forcibly add a breakpad/ prefix to the breakpad header files. This
  frees us from having to modify our include path when building.
* Pull in (heavily modified) util/minidump.{cc,h} from Apache Impala
  (incubating). The modifications consist of removing use of
  boost::filesystem, conformance to the Kudu code style guidelines, and
  some significant refactoring.
* Enable breakpad support for the kudu-tserver and kudu-master
  processes.
* By default, invoke previously-installed signal handler if installed.
* Handle SIGUSR1 by safely using sigwait() to create minidumps on demand.
* Add #ifdefs and CMake logic for Linux.
* Add test for all the deadly signals to ensure we get both a stack
  trace and a minidump, except for the case of SIGTERM, where we should
  not get a minidump (by design).
* Change a few library unit tests that relied on SIGUSR1 to use another
  signal for their testing, such as SIGHUP or SIGUSR2.
* Remove unused includes from mini_tablet_server.cc
* Clean up signal handling logic when forking a subprocess.
* Hide breakpad symbols from libkuduclient.so
* Install failure function that simply calls abort() instead of first
  printing a stacktrace.

Change-Id: I495695cc38b75377f20b0497093a81ed5baa887f
---
M CMakeLists.txt
M build-support/jenkins/build-and-test.sh
A cmake_modules/FindBreakpadClient.cmake
M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java
M src/kudu/client/client_samples-test.sh
M src/kudu/client/symbols.map
M src/kudu/integration-tests/CMakeLists.txt
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/integration-tests/external_mini_cluster.h
A src/kudu/integration-tests/minidump_generation-itest.cc
M src/kudu/master/mini_master.cc
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/debug-util-test.cc
M src/kudu/util/env_posix.cc
M src/kudu/util/logging.cc
A src/kudu/util/minidump-test.cc
A src/kudu/util/minidump.cc
A src/kudu/util/minidump.h
M src/kudu/util/subprocess-test.cc
M src/kudu/util/subprocess.cc
M src/kudu/util/test_main.cc
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
M thirdparty/download-thirdparty.sh
M thirdparty/preflight.py
A thirdparty/scripts/make-breakpad-src-archive.sh
M thirdparty/vars.sh
30 files changed, 994 insertions(+), 84 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/5620/31
-- 
To view, visit http://gerrit.cloudera.org:8080/5620
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I495695cc38b75377f20b0497093a81ed5baa887f
Gerrit-PatchSet: 31
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] WIP [security] tailored TokenSigner for system catalog

2017-02-07 Thread Alexey Serbin (Code Review)
Alexey Serbin has uploaded a new change for review.

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

Change subject: WIP [security] tailored TokenSigner for system catalog
..

WIP [security] tailored TokenSigner for system catalog

Updated the TokenSigner class in preparation to loading
public part of token signing keys from the system catalog.

The expected use-case for the TokenSigner is calling
AddTokenSigningPublicKey() multiple times while loading public part
of TSKs from the system catalog and subsequent call of Init().
That's the sequence to be exercised by a master server upon becoming
a leader.  It's possible to run this sequence multiple times
on the same instance of TokenSigner, generating new signing keys
only when already existing signing keys are about to expire.

Also, the TokenSigner class now embeds the logic to rotate its
signing key on Init(), if necessary.

Change-Id: Ie2417e2ccba6a1114db366b2f642f95362bf479c
---
M src/kudu/security/token-test.cc
M src/kudu/security/token_signer.cc
M src/kudu/security/token_signer.h
M src/kudu/security/token_signing_key.cc
M src/kudu/security/token_signing_key.h
5 files changed, 293 insertions(+), 48 deletions(-)


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

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


[kudu-CR] [java client] KUDU-1643 Prune hash partitions based on IN-list predicates

2017-02-07 Thread Dan Burkert (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: [java client] KUDU-1643 Prune hash partitions based on IN-list 
predicates
..

[java client] KUDU-1643 Prune hash partitions based on IN-list predicates

PartitionPruner is updated to search all combinations of in-list column
values and prune hash partitions where possible.

This also fixes a small issue in the C++ version of the algorithm:
previously the implementation would always consider the final hash
component to be constrained. The bug (as far as I can tell) doesn't lead
to erroneous results, but does cause more partition ranges to be
created, which results in more memory overhead and compute overhead to
compute the pruning.

Change-Id: I8a793c23ff00d19b3d3d062bb222d2c725a93724
---
M java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartitionPruner.java
M src/kudu/common/partial_row.h
M src/kudu/common/partition_pruner-test.cc
M src/kudu/common/partition_pruner.cc
M src/kudu/common/partition_pruner.h
M src/kudu/common/scan_spec.cc
7 files changed, 535 insertions(+), 339 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/5677/8
-- 
To view, visit http://gerrit.cloudera.org:8080/5677
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8a793c23ff00d19b3d3d062bb222d2c725a93724
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Haijie Hong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] [java client] KUDU-1643 Prune hash partitions based on IN-list predicates

2017-02-07 Thread Jean-Daniel Cryans (Code Review)
Jean-Daniel Cryans has posted comments on this change.

Change subject: [java client] KUDU-1643 Prune hash partitions based on IN-list 
predicates
..


Patch Set 7:

(2 comments)

Two nits and please add a comment regarding string allocations in 
PartitionPruner as we discussed elsewhere.

http://gerrit.cloudera.org:8080/#/c/5677/7/java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java
File java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java:

PS7, Line 51: .
nit: remove


PS7, Line 488: .
nit


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8a793c23ff00d19b3d3d062bb222d2c725a93724
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Haijie Hong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] KUDU-1856: always truncate containers when they get full

2017-02-07 Thread Mike Percy (Code Review)
Mike Percy has posted comments on this change.

Change subject: KUDU-1856: always truncate containers when they get full
..


Patch Set 3: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5852/3/src/kudu/util/env_posix.cc
File src/kudu/util/env_posix.cc:

Line 137:   CHECK_EQ(mode, 0);
nit: expected comes before actual in CHECK_EQ()


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic959c59489a08f92efa2df5c85b22e56740f1346
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes


[kudu-CR] KUDU-1856: always truncate containers when they get full

2017-02-07 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: KUDU-1856: always truncate containers when they get full
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5852/3/src/kudu/util/env_posix.cc
File src/kudu/util/env_posix.cc:

Line 137:   CHECK_EQ(mode, 0);
> That's what I thought too, but Todd asked me to change it. I'm not going to
It seems we're not super consistent. Here's some greps looking at whether the 
member variable comes first or second in various CHECK_EQ:

todd@todd-ThinkPad-T540p:~/git/kudu$ git grep 'CHECK_EQ([^,]*, [a-zA-z]*_)' | 
wc -l
50
todd@todd-ThinkPad-T540p:~/git/kudu$ git grep 'CHECK_EQ([a-zA-z]*_, [^,]*)' | 
wc -l
114


so seems like we usually prefer CHECK_EQ(foo_, ) rather than the 
other way around, but not overwhelmingly so

I'd misremembered the output from a failed CHECK_EQ though. It seems it doesn't 
really matter the order because the output doesn't purport to call one side or 
the other "expected" or "actual" (whereas gtest does). Instead you just get 
Check failed: 1 == 2 (1 vs. 2)

I lean towards 'CHECK_EQ(mode, 0)' for the same reason I lean towards 'if (mode 
== 0)' - it just is more close to how we usually speak. But don't feel strongly 
now that I know the output isn't confusing.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic959c59489a08f92efa2df5c85b22e56740f1346
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes


[kudu-CR] compaction: Add additional validation in DeltaTracker

2017-02-07 Thread Mike Percy (Code Review)
Mike Percy has submitted this change and it was merged.

Change subject: compaction: Add additional validation in DeltaTracker
..


compaction: Add additional validation in DeltaTracker

This adds DEBUG-mode validation to AtomicUpdateStores() in the
DeltaTracker. It also updates the doc comment on the method.

Change-Id: I05fed515bc5d6a09484fadb61e73ac1346f6654a
Reviewed-on: http://gerrit.cloudera.org:8080/5919
Tested-by: Kudu Jenkins
Reviewed-by: Jean-Daniel Cryans 
Reviewed-by: David Ribeiro Alves 
---
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/delta_tracker.h
M src/kudu/tablet/deltafile.h
M src/kudu/util/status.h
5 files changed, 87 insertions(+), 16 deletions(-)

Approvals:
  David Ribeiro Alves: Looks good to me, approved
  Jean-Daniel Cryans: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I05fed515bc5d6a09484fadb61e73ac1346f6654a
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot


[kudu-CR] compaction: Flush tablet metadata before updating stores

2017-02-07 Thread Mike Percy (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: compaction: Flush tablet metadata before updating stores
..

compaction: Flush tablet metadata before updating stores

We should always flush the rowset metadata before making changes visible
to the delta tracker. In this case, because we are just doing a
compaction, it is likely not a crash consistency issue, however we
should always follow the same order of operations when operating on
tablet metadata.

This change also allows us to defer a crash to a point higher in the
call stack.

Also, add LogPrefix() for the log lines printed from the delta tracker.

Change-Id: I958f4744f1b95c6a63e8c9105bd1c7552e43e6b9
---
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/delta_tracker.h
2 files changed, 60 insertions(+), 38 deletions(-)


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

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


[kudu-CR] java: remove unused parts of SecureRpcHelper

2017-02-07 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change.

Change subject: java: remove unused parts of SecureRpcHelper
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5925/1/java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java
File java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java:

Line 141
We're going to need unwrap to verify channel bindings.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3b70e0319060e3f2d9cc9a0ed23da7fe51b174df
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes


[kudu-CR] WIP [security] tailored TokenSigner for system catalog

2017-02-07 Thread Alexey Serbin (Code Review)
Alexey Serbin has uploaded a new patch set (#2).

Change subject: WIP [security] tailored TokenSigner for system catalog
..

WIP [security] tailored TokenSigner for system catalog

Updated the TokenSigner class in preparation to loading
public part of token signing keys from the system catalog.

The expected use-case for the TokenSigner is calling
AddTokenSigningPublicKey() multiple times while loading public part
of TSKs from the system catalog and subsequent call of Init().
That's the sequence to be exercised by a master server upon becoming
a leader.  It's possible to run this sequence multiple times
on the same instance of TokenSigner, generating new signing keys
only when already existing signing keys are about to expire.

Also, the TokenSigner class now embeds the logic to rotate its
signing key on Init(), if necessary.

Change-Id: Ie2417e2ccba6a1114db366b2f642f95362bf479c
---
M src/kudu/security/token-test.cc
M src/kudu/security/token_signer.cc
M src/kudu/security/token_signer.h
M src/kudu/security/token_signing_key.cc
M src/kudu/security/token_signing_key.h
5 files changed, 302 insertions(+), 50 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie2417e2ccba6a1114db366b2f642f95362bf479c
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] KUDU-1856: always truncate containers when they get full

2017-02-07 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change.

Change subject: KUDU-1856: always truncate containers when they get full
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5852/3/src/kudu/util/env_posix.cc
File src/kudu/util/env_posix.cc:

Line 137:   CHECK_EQ(mode, 0);
> nit: expected comes before actual in CHECK_EQ()
That's what I thought too, but Todd asked me to change it. I'm not going to 
switch it back, but we can talk about what the right order should be and hew to 
that going forward.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic959c59489a08f92efa2df5c85b22e56740f1346
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes


[kudu-CR] Add Google Breakpad support to Kudu

2017-02-07 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change.

Change subject: Add Google Breakpad support to Kudu
..


Patch Set 30: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I495695cc38b75377f20b0497093a81ed5baa887f
Gerrit-PatchSet: 30
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] WIP: enable kerberos support in CSD

2017-02-07 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: WIP: enable kerberos support in CSD
..


Patch Set 1:

yea, that's fine. It's useful to have up on gerrit, though -- makes it much 
easier to test in a kerberized environment, even if you have to do a local 
build.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iea2d36b7a0da38cd6ddd646a32f52517cabeb4a6
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] Add Google Breakpad support to Kudu

2017-02-07 Thread Mike Percy (Code Review)
Mike Percy has posted comments on this change.

Change subject: Add Google Breakpad support to Kudu
..


Patch Set 27:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/5620/27/src/kudu/client/symbols.map
File src/kudu/client/symbols.map:

Line 26: # breakpad
> Can we wildcard some of these? Maybe ConvertUTF* and my_*?
Done


http://gerrit.cloudera.org:8080/#/c/5620/27/src/kudu/util/minidump.cc
File src/kudu/util/minidump.cc:

Line 217:   google::InstallFailureFunction();
> Do you even need the redirection through AbortFailureFunction? abort() has 
Yeah, just passing abort works, but I thought it was helpful to have it jump 
through AbortFailureFunction so it would be obvious that it was triggered 
through this code path.


http://gerrit.cloudera.org:8080/#/c/5620/27/src/kudu/util/subprocess.cc
File src/kudu/util/subprocess.cc:

Line 432: ResetSigPipeHandlerToDefault();
> Why do we only reset SIGPIPE to SIG_DFL and not every signal? What about SI
I added a short comment, but it's for the following reasons:
1. The shell or parent process invoking Kudu may set the signal disposition to 
ignore and they might know better than us what they want, e.g. for HUP.
2. As far as I could find, we only explicitly set the signal disposition to IGN 
for PIPE; we don't ignore any other signals.
3. Setting the signal handler to DFL is only necessary if it was previously set 
to IGN; if it was set to be caught then execve() automatically sets it to DFL.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I495695cc38b75377f20b0497093a81ed5baa887f
Gerrit-PatchSet: 27
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] [java client] KUDU-1643 Prune hash partitions based on IN-list predicates

2017-02-07 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change.

Change subject: [java client] KUDU-1643 Prune hash partitions based on IN-list 
predicates
..


Patch Set 9:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5677/7/java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java
File java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java:

PS7, Line 51: 
> nit: remove
Done


PS7, Line 488: 
> nit
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8a793c23ff00d19b3d3d062bb222d2c725a93724
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Haijie Hong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] [java client] KUDU-1643 Prune hash partitions based on IN-list predicates

2017-02-07 Thread Dan Burkert (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: [java client] KUDU-1643 Prune hash partitions based on IN-list 
predicates
..

[java client] KUDU-1643 Prune hash partitions based on IN-list predicates

PartitionPruner is updated to search all combinations of in-list column
values and prune hash partitions where possible.

This also fixes a small issue in the C++ version of the algorithm:
previously the implementation would always consider the final hash
component to be constrained. The bug (as far as I can tell) doesn't lead
to erroneous results, but does cause more partition ranges to be
created, which results in more memory overhead and compute overhead to
compute the pruning.

Change-Id: I8a793c23ff00d19b3d3d062bb222d2c725a93724
---
M java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartitionPruner.java
M src/kudu/common/partial_row.h
M src/kudu/common/partition_pruner-test.cc
M src/kudu/common/partition_pruner.cc
M src/kudu/common/partition_pruner.h
M src/kudu/common/scan_spec.cc
7 files changed, 535 insertions(+), 339 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8a793c23ff00d19b3d3d062bb222d2c725a93724
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Haijie Hong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] [java client] KUDU-1643 Prune hash partitions based on IN-list predicates

2017-02-07 Thread Jean-Daniel Cryans (Code Review)
Jean-Daniel Cryans has posted comments on this change.

Change subject: [java client] KUDU-1643 Prune hash partitions based on IN-list 
predicates
..


Patch Set 9: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8a793c23ff00d19b3d3d062bb222d2c725a93724
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Haijie Hong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] Add Google Breakpad support to Kudu

2017-02-07 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change.

Change subject: Add Google Breakpad support to Kudu
..


Patch Set 32: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I495695cc38b75377f20b0497093a81ed5baa887f
Gerrit-PatchSet: 32
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] java: use a netty frame decoder instead of replaying decoder

2017-02-07 Thread Todd Lipcon (Code Review)
Hello Dan Burkert, Jean-Daniel Cryans, Kudu Jenkins,

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

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

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

Change subject: java: use a netty frame decoder instead of replaying decoder
..

java: use a netty frame decoder instead of replaying decoder

All of our inbound packets are length-prefixed. Netty provides a nice
class for handing length-prefixed messages. This avoids a bunch of more
custom logic we were doing for length checking, etc.

Change-Id: I1ec1e6f9cbbdf694fe85b7668a3c349efc26c08f
---
M java/kudu-client/src/main/java/org/apache/kudu/client/CallResponse.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.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/SecureRpcHelper.java
M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java
5 files changed, 48 insertions(+), 125 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1ec1e6f9cbbdf694fe85b7668a3c349efc26c08f
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] java: fix ability to connect to a real Kerberized cluster

2017-02-07 Thread Todd Lipcon (Code Review)
Hello Dan Burkert, Jean-Daniel Cryans, Kudu Jenkins,

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

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

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

Change subject: java: fix ability to connect to a real Kerberized cluster
..

java: fix ability to connect to a real Kerberized cluster

This fixes a couple issues seen when I tried to run the Java client
from within Impala against a Kerberized Kudu cluster:

* Previously, the ServerInfo class remembered the already-resolved
  address of the server rather than its hostname. This meant that we
  would try to connect to a Kerberos principal "kudu/1.2.3.4" rather
  than "kudu/foo.example.com". This typically would not match the actual
  principal the server was using, resulting in an error that the server
  principal was not found in the database.

* We previously were using 'subject.doAs()' when initializing the SASL
  server, but not when evaluating challenges. It turns out that the
  GSSAPI mechanism only looks for the Kerberos credentials while
  evaluating the challenge. This moves the 'doAs()' to wrap the
  challenge.

* Another issue with the SASL setup was that we were passing all of the
  available client mechanisms into Sasl.createSaslClient() before seeing
  which mechanisms the server was advertised. the SASL library seems to
  always prefer GSSAPI over PLAIN when available. This meant that, if
  the server had Kerberos credentials, it would attempt to use GSSAPI
  and not PLAIN even if connecting to a server which only advertised
  plain. This fixes the negotiation to no longer ignore the server-side
  advertised mechanisms, and instead actually negotiate by picking the
  best mechanism which is both advertised by the server and
  initializable by the client.

* We previously assumed that Kerberos credentials would be available
  from the 'Subject' without any explicit login call. This isn't the
  case: we have to explicitly set up a LoginContext and Configuration to
  log in from the credentials cache. This changes the client constructor
  to login from the ccache if there is no Subject available with
  Kerberos credentials.

With these changes I was able to run an Impala query against a cluster
with -server_require_kerberos.

Change-Id: I6b96fad3cfb40500d7a75e5070ea21bc8e00cbd8
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java
M java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java
A java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java
M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKdc.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/TestConnectionCache.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestMiniKdc.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestMiniKuduCluster.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java
11 files changed, 281 insertions(+), 129 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6b96fad3cfb40500d7a75e5070ea21bc8e00cbd8
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] Make SecureRpcHelper a Netty pipeline stage

2017-02-07 Thread Todd Lipcon (Code Review)
Hello Dan Burkert, Jean-Daniel Cryans, Kudu Jenkins,

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

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

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

Change subject: Make SecureRpcHelper a Netty pipeline stage
..

Make SecureRpcHelper a Netty pipeline stage

This decouples SecureRpcHelper from TabletClient. When negotiation is
complete, it sends an "event" upstream to the TabletClient and removes
itself from the pipeline.

I'm hoping this decoupling will make it easier to test SecureRpcHelper
in isolation as we add more functionality like TLS negotiation.

After this is committed, I'm also hoping to rename SecureRpcHelper to
Negotiator.

Change-Id: I5f4b4516219b8eebf24786b4ceb13f2e6260f03b
---
M java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java
M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java
2 files changed, 84 insertions(+), 75 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5f4b4516219b8eebf24786b4ceb13f2e6260f03b
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] Add Google Breakpad support to Kudu

2017-02-07 Thread Mike Percy (Code Review)
Hello Adar Dembo, Kudu Jenkins,

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

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

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

Change subject: Add Google Breakpad support to Kudu
..

Add Google Breakpad support to Kudu

Breakpad creates minidumps upon crash, which are small files that
include stack-based information for each thread. These do not include
heap information, so they complement, not replace, core files for
debugging purposes.

This patch enables Google Breakpad for the kudu-tserver and kudu-master
processes.

Breakpad is part of the crash-reporting infrastructure open-sourced by
Google. It is also used by Mozilla Firefox and Google Chrome.

When a process crashes, Breakpad writes a small dump file called a
minidump to disk. Since these files are typically only a few MB in size,
this allows users to share critical crash information with developers
for offline analysis.

For more information, see the breakpad getting started docs at
http://chromium.googlesource.com/breakpad/breakpad/+/master/docs/getting_started_with_breakpad.md

By default, on crash, minidump files are written to a subdirectory of
the log directory for a given Kudu daemon process. This was chosen for
the following reasons:

1. The minidump files are relatively small, potentially comparable in
   size to log files, and the log directory is a place an administrator
   would look in when debugging.
2. This convention is consistent with what Apache Impala (incubating)
   does for its minidump files.

Changes in this patch:

* Add breakpad to thirdparty.
* Add breakpad source archive creation script (Breakpad does not do
  releases).
* Add a "hack" to the breakpad thirdparty installation routine to
  forcibly add a breakpad/ prefix to the breakpad header files. This
  frees us from having to modify our include path when building.
* Pull in (heavily modified) util/minidump.{cc,h} from Apache Impala
  (incubating). The modifications consist of removing use of
  boost::filesystem, conformance to the Kudu code style guidelines, and
  some significant refactoring.
* Enable breakpad support for the kudu-tserver and kudu-master
  processes.
* By default, invoke previously-installed signal handler if installed.
* Handle SIGUSR1 by safely using sigwait() to create minidumps on demand.
* Add #ifdefs and CMake logic for Linux.
* Add test for all the deadly signals to ensure we get both a stack
  trace and a minidump, except for the case of SIGTERM, where we should
  not get a minidump (by design).
* Change a few library unit tests that relied on SIGUSR1 to use another
  signal for their testing, such as SIGHUP or SIGUSR2.
* Remove unused includes from mini_tablet_server.cc
* Clean up signal handling logic when forking a subprocess.
* Hide breakpad symbols from libkuduclient.so
* Install failure function that simply calls abort() instead of first
  printing a stacktrace.

Change-Id: I495695cc38b75377f20b0497093a81ed5baa887f
---
M CMakeLists.txt
M build-support/jenkins/build-and-test.sh
A cmake_modules/FindBreakpadClient.cmake
M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java
M src/kudu/client/client_samples-test.sh
M src/kudu/client/symbols.map
M src/kudu/integration-tests/CMakeLists.txt
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/integration-tests/external_mini_cluster.h
A src/kudu/integration-tests/minidump_generation-itest.cc
M src/kudu/master/mini_master.cc
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/debug-util-test.cc
M src/kudu/util/env_posix.cc
M src/kudu/util/logging.cc
A src/kudu/util/minidump-test.cc
A src/kudu/util/minidump.cc
A src/kudu/util/minidump.h
M src/kudu/util/subprocess-test.cc
M src/kudu/util/subprocess.cc
M src/kudu/util/test_main.cc
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
M thirdparty/download-thirdparty.sh
M thirdparty/preflight.py
A thirdparty/scripts/make-breakpad-src-archive.sh
M thirdparty/vars.sh
30 files changed, 1,027 insertions(+), 85 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/5620/35
-- 
To view, visit http://gerrit.cloudera.org:8080/5620
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I495695cc38b75377f20b0497093a81ed5baa887f
Gerrit-PatchSet: 35
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] java: use a netty frame decoder instead of replaying decoder

2017-02-07 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: java: use a netty frame decoder instead of replaying decoder
..


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/5926/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java:

Line 72:   
> I see red.
Done


Line 78:* The Hadoop RPC protocol doesn't do any checksumming as they 
probably
> For my own edification: is this comment implying that the protocol should h
yea, I think so. This is just a moved comment. I think it dates back to the 
OpenTSDB original source, which has plenty of "editorializing".


http://gerrit.cloudera.org:8080/#/c/5926/1/java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java:

Line 174: // TODO(todd): shouldn't this return here?
> Yes.
ok i'll make the TODO more serious and point out there is no coverage of this.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ec1e6f9cbbdf694fe85b7668a3c349efc26c08f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] Add Google Breakpad support to Kudu

2017-02-07 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change.

Change subject: Add Google Breakpad support to Kudu
..


Patch Set 34:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5620/34/src/kudu/util/minidump.h
File src/kudu/util/minidump.h:

Line 86:   __attribute__((unused)) std::atomic 
user_signal_handler_thread_running_;
Hmm, I thought this typically goes after the variable declaration, not before.

Also, use ATTRIBUTE_UNUSED from gutil/port.h, I think that's preferable.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I495695cc38b75377f20b0497093a81ed5baa887f
Gerrit-PatchSet: 34
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] Add missing pb util proto dependency to token proto

2017-02-07 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change.

Change subject: Add missing pb_util_proto dependency to token_proto
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I63f743bcf4b36a884c58a292a7eb03bf9098f78b
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] [java client] KUDU-1643 Prune hash partitions based on IN-list predicates

2017-02-07 Thread Dan Burkert (Code Review)
Dan Burkert has submitted this change and it was merged.

Change subject: [java client] KUDU-1643 Prune hash partitions based on IN-list 
predicates
..


[java client] KUDU-1643 Prune hash partitions based on IN-list predicates

PartitionPruner is updated to search all combinations of in-list column
values and prune hash partitions where possible.

This also fixes a small issue in the C++ version of the algorithm:
previously the implementation would always consider the final hash
component to be constrained. The bug (as far as I can tell) doesn't lead
to erroneous results, but does cause more partition ranges to be
created, which results in more memory overhead and compute overhead to
compute the pruning.

Change-Id: I8a793c23ff00d19b3d3d062bb222d2c725a93724
Reviewed-on: http://gerrit.cloudera.org:8080/5677
Tested-by: Kudu Jenkins
Reviewed-by: Jean-Daniel Cryans 
---
M java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartitionPruner.java
M src/kudu/common/partial_row.h
M src/kudu/common/partition_pruner-test.cc
M src/kudu/common/partition_pruner.cc
M src/kudu/common/partition_pruner.h
M src/kudu/common/scan_spec.cc
7 files changed, 535 insertions(+), 339 deletions(-)

Approvals:
  Jean-Daniel Cryans: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I8a793c23ff00d19b3d3d062bb222d2c725a93724
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Haijie Hong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] Make SecureRpcHelper a Netty pipeline stage

2017-02-07 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: Make SecureRpcHelper a Netty pipeline stage
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5927/1/java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java
File java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java:

Line 113:   
> RED
yea, I need to figure out how to configure eclipse not to do this.


Line 127: RpcHeader.NegotiatePB response = parseSaslMsgResponse(buf);
> This needs to check for an error response, or maybe punt and add a TODO.
added a TODO


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5f4b4516219b8eebf24786b4ceb13f2e6260f03b
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] java: remove unused parts of SecureRpcHelper

2017-02-07 Thread Todd Lipcon (Code Review)
Hello Dan Burkert, Jean-Daniel Cryans, Kudu Jenkins,

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

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

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

Change subject: java: remove unused parts of SecureRpcHelper
..

java: remove unused parts of SecureRpcHelper

This removes the unused wrap/unwrap code from SecureRpcHelper. I also
removed an unnecessary function in TabletClient.

Change-Id: I3b70e0319060e3f2d9cc9a0ed23da7fe51b174df
---
M java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java
M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java
2 files changed, 3 insertions(+), 52 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3b70e0319060e3f2d9cc9a0ed23da7fe51b174df
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] Add Google Breakpad support to Kudu

2017-02-07 Thread Mike Percy (Code Review)
Hello Adar Dembo, Kudu Jenkins,

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

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

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

Change subject: Add Google Breakpad support to Kudu
..

Add Google Breakpad support to Kudu

Breakpad creates minidumps upon crash, which are small files that
include stack-based information for each thread. These do not include
heap information, so they complement, not replace, core files for
debugging purposes.

This patch enables Google Breakpad for the kudu-tserver and kudu-master
processes.

Breakpad is part of the crash-reporting infrastructure open-sourced by
Google. It is also used by Mozilla Firefox and Google Chrome.

When a process crashes, Breakpad writes a small dump file called a
minidump to disk. Since these files are typically only a few MB in size,
this allows users to share critical crash information with developers
for offline analysis.

For more information, see the breakpad getting started docs at
http://chromium.googlesource.com/breakpad/breakpad/+/master/docs/getting_started_with_breakpad.md

By default, on crash, minidump files are written to a subdirectory of
the log directory for a given Kudu daemon process. This was chosen for
the following reasons:

1. The minidump files are relatively small, potentially comparable in
   size to log files, and the log directory is a place an administrator
   would look in when debugging.
2. This convention is consistent with what Apache Impala (incubating)
   does for its minidump files.

Changes in this patch:

* Add breakpad to thirdparty.
* Add breakpad source archive creation script (Breakpad does not do
  releases).
* Add a "hack" to the breakpad thirdparty installation routine to
  forcibly add a breakpad/ prefix to the breakpad header files. This
  frees us from having to modify our include path when building.
* Pull in (heavily modified) util/minidump.{cc,h} from Apache Impala
  (incubating). The modifications consist of removing use of
  boost::filesystem, conformance to the Kudu code style guidelines, and
  some significant refactoring.
* Enable breakpad support for the kudu-tserver and kudu-master
  processes.
* By default, invoke previously-installed signal handler if installed.
* Handle SIGUSR1 by safely using sigwait() to create minidumps on demand.
* Add #ifdefs and CMake logic for Linux.
* Add test for all the deadly signals to ensure we get both a stack
  trace and a minidump, except for the case of SIGTERM, where we should
  not get a minidump (by design).
* Change a few library unit tests that relied on SIGUSR1 to use another
  signal for their testing, such as SIGHUP or SIGUSR2.
* Remove unused includes from mini_tablet_server.cc
* Clean up signal handling logic when forking a subprocess.
* Hide breakpad symbols from libkuduclient.so
* Install failure function that simply calls abort() instead of first
  printing a stacktrace.

Change-Id: I495695cc38b75377f20b0497093a81ed5baa887f
---
M CMakeLists.txt
M build-support/jenkins/build-and-test.sh
A cmake_modules/FindBreakpadClient.cmake
M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java
M src/kudu/client/client_samples-test.sh
M src/kudu/client/symbols.map
M src/kudu/integration-tests/CMakeLists.txt
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/integration-tests/external_mini_cluster.h
A src/kudu/integration-tests/minidump_generation-itest.cc
M src/kudu/master/mini_master.cc
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/debug-util-test.cc
M src/kudu/util/env_posix.cc
M src/kudu/util/logging.cc
A src/kudu/util/minidump-test.cc
A src/kudu/util/minidump.cc
A src/kudu/util/minidump.h
M src/kudu/util/subprocess-test.cc
M src/kudu/util/subprocess.cc
M src/kudu/util/test_main.cc
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
M thirdparty/download-thirdparty.sh
M thirdparty/preflight.py
A thirdparty/scripts/make-breakpad-src-archive.sh
M thirdparty/vars.sh
30 files changed, 1,027 insertions(+), 85 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/5620/32
-- 
To view, visit http://gerrit.cloudera.org:8080/5620
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I495695cc38b75377f20b0497093a81ed5baa887f
Gerrit-PatchSet: 32
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] Add Google Breakpad support to Kudu

2017-02-07 Thread David Ribeiro Alves (Code Review)
Hello Mike Percy, Adar Dembo, Kudu Jenkins,

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

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

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

Change subject: Add Google Breakpad support to Kudu
..

Add Google Breakpad support to Kudu

Breakpad creates minidumps upon crash, which are small files that
include stack-based information for each thread. These do not include
heap information, so they complement, not replace, core files for
debugging purposes.

This patch enables Google Breakpad for the kudu-tserver and kudu-master
processes.

Breakpad is part of the crash-reporting infrastructure open-sourced by
Google. It is also used by Mozilla Firefox and Google Chrome.

When a process crashes, Breakpad writes a small dump file called a
minidump to disk. Since these files are typically only a few MB in size,
this allows users to share critical crash information with developers
for offline analysis.

For more information, see the breakpad getting started docs at
http://chromium.googlesource.com/breakpad/breakpad/+/master/docs/getting_started_with_breakpad.md

By default, on crash, minidump files are written to a subdirectory of
the log directory for a given Kudu daemon process. This was chosen for
the following reasons:

1. The minidump files are relatively small, potentially comparable in
   size to log files, and the log directory is a place an administrator
   would look in when debugging.
2. This convention is consistent with what Apache Impala (incubating)
   does for its minidump files.

Changes in this patch:

* Add breakpad to thirdparty.
* Add breakpad source archive creation script (Breakpad does not do
  releases).
* Add a "hack" to the breakpad thirdparty installation routine to
  forcibly add a breakpad/ prefix to the breakpad header files. This
  frees us from having to modify our include path when building.
* Pull in (heavily modified) util/minidump.{cc,h} from Apache Impala
  (incubating). The modifications consist of removing use of
  boost::filesystem, conformance to the Kudu code style guidelines, and
  some significant refactoring.
* Enable breakpad support for the kudu-tserver and kudu-master
  processes.
* By default, invoke previously-installed signal handler if installed.
* Handle SIGUSR1 by safely using sigwait() to create minidumps on demand.
* Add #ifdefs and CMake logic for Linux.
* Add test for all the deadly signals to ensure we get both a stack
  trace and a minidump, except for the case of SIGTERM, where we should
  not get a minidump (by design).
* Change a few library unit tests that relied on SIGUSR1 to use another
  signal for their testing, such as SIGHUP or SIGUSR2.
* Remove unused includes from mini_tablet_server.cc
* Clean up signal handling logic when forking a subprocess.
* Hide breakpad symbols from libkuduclient.so
* Install failure function that simply calls abort() instead of first
  printing a stacktrace.

Change-Id: I495695cc38b75377f20b0497093a81ed5baa887f
---
M CMakeLists.txt
M build-support/jenkins/build-and-test.sh
A cmake_modules/FindBreakpadClient.cmake
M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java
M src/kudu/client/client_samples-test.sh
M src/kudu/client/symbols.map
M src/kudu/integration-tests/CMakeLists.txt
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/integration-tests/external_mini_cluster.h
A src/kudu/integration-tests/minidump_generation-itest.cc
M src/kudu/master/mini_master.cc
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/debug-util-test.cc
M src/kudu/util/env_posix.cc
M src/kudu/util/logging.cc
A src/kudu/util/minidump-test.cc
A src/kudu/util/minidump.cc
A src/kudu/util/minidump.h
M src/kudu/util/subprocess-test.cc
M src/kudu/util/subprocess.cc
M src/kudu/util/test_main.cc
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
M thirdparty/download-thirdparty.sh
M thirdparty/preflight.py
A thirdparty/scripts/make-breakpad-src-archive.sh
M thirdparty/vars.sh
30 files changed, 1,034 insertions(+), 85 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/5620/34
-- 
To view, visit http://gerrit.cloudera.org:8080/5620
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I495695cc38b75377f20b0497093a81ed5baa887f
Gerrit-PatchSet: 34
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd 

[kudu-CR] Add missing pb util proto dependency to token proto

2017-02-07 Thread Dan Burkert (Code Review)
Hello Todd Lipcon,

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

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

to review the following change.

Change subject: Add missing pb_util_proto dependency to token_proto
..

Add missing pb_util_proto dependency to token_proto

Change-Id: I63f743bcf4b36a884c58a292a7eb03bf9098f78b
---
M src/kudu/security/CMakeLists.txt
1 file changed, 1 insertion(+), 1 deletion(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I63f743bcf4b36a884c58a292a7eb03bf9098f78b
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] Add missing pb util proto dependency to token proto

2017-02-07 Thread David Ribeiro Alves (Code Review)
David Ribeiro Alves has submitted this change and it was merged.

Change subject: Add missing pb_util_proto dependency to token_proto
..


Add missing pb_util_proto dependency to token_proto

Change-Id: I63f743bcf4b36a884c58a292a7eb03bf9098f78b
Reviewed-on: http://gerrit.cloudera.org:8080/5932
Reviewed-by: Alexey Serbin 
Tested-by: Kudu Jenkins
---
M src/kudu/security/CMakeLists.txt
1 file changed, 1 insertion(+), 1 deletion(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I63f743bcf4b36a884c58a292a7eb03bf9098f78b
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] java: remove unused parts of SecureRpcHelper

2017-02-07 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: java: remove unused parts of SecureRpcHelper
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5925/1/java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java
File java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java:

Line 141
> We're going to need unwrap to verify channel bindings.
yea, but it doesn't need all this netty junk associated with it - we'll just be 
unwrapping/wrapping a byte[], which is just the underlying 
saslClient.unwrap/wrap call


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3b70e0319060e3f2d9cc9a0ed23da7fe51b174df
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] WIP: fixes for java client kerberos against a real cluster

2017-02-07 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: WIP: fixes for java client kerberos against a real cluster
..


Patch Set 2:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/5922/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:

Line 1830: private static Subject getSubjectOrLogin() {
> Seems like this could be extracted into some helper class.
Done


PS2, Line 1834:  "
> use {} substitution
Done


Line 1840:   @Override
> Pretty sure we normally use double indentation for anonymous classes.
Done


Line 1850: options.put("debug", "" + 
Boolean.getBoolean("kudu.jaas.debug"));
> Boolean.toString(Boolean.getBoolean("kudu.jaas.debug"))
Done


Line 1871: LOG.debug("Logged in as subject: " + subject.toString());
> {} substitution
Done


PS2, Line 1874: c
> Could*
Done


http://gerrit.cloudera.org:8080/#/c/5922/2/java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java
File java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java:

Line 116: if (saslClient == null || !saslClient.isComplete() || 
negoUnderway) {
> Looks like this could this be simplified to just 'if (negoUnderway)'
Done


Line 206: for (RpcHeader.NegotiatePB.SaslAuth auth : 
response.getAuthsList()) {
> This shouldn't be done in a loop.  The Java client should intersect the ser
Done


Line 270:   if (e instanceof SaslException) {
> This would be simpler as a separate catch clause.
it complains about that because the Subject.doAs doesn't say it "throws 
SaslException". I used Throwables from guava here which is cleaner.


http://gerrit.cloudera.org:8080/#/c/5922/2/java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java
File java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java:

Line 85:   public InetAddress getResolvedAddress() {
> nit: javadoc
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6b96fad3cfb40500d7a75e5070ea21bc8e00cbd8
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] java: use a netty frame decoder instead of replaying decoder

2017-02-07 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change.

Change subject: java: use a netty frame decoder instead of replaying decoder
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5926/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java:

Line 78:* The Hadoop RPC protocol doesn't do any checksumming as they 
probably
For my own edification: is this comment implying that the protocol should have 
included a CRC of the length immediately following the length bytes?  I think 
you would still want this check in order to limit the damage of a bad actor.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ec1e6f9cbbdf694fe85b7668a3c349efc26c08f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes


[kudu-CR] [security] clean-up on cert management-test.cc

2017-02-07 Thread Alexey Serbin (Code Review)
Alexey Serbin has uploaded a new change for review.

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

Change subject: [security] clean-up on cert_management-test.cc
..

[security] clean-up on cert_management-test.cc

A minor clean-up on cert-related tests: moved other than
X509 CSR-related tests from cert_management-test.cc into cert-test.cc.
Also, removed duplicated key-specific tests from cert_management.cc:
they are in crypto-test.cc now (probably, the duplication was the result
of merge conflicts resolution).

There are no functional changes in this patch.

Change-Id: I3e42d8545e783fbc657de9bf2d4d231265cf3f3f
---
M src/kudu/security/ca/cert_management-test.cc
M src/kudu/security/cert-test.cc
2 files changed, 22 insertions(+), 62 deletions(-)


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

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


[kudu-CR] [security] method to check if X509 cert matches key

2017-02-07 Thread Alexey Serbin (Code Review)
Alexey Serbin has uploaded a new change for review.

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

Change subject: [security] method to check if X509 cert matches key
..

[security] method to check if X509 cert matches key

Added a method to security::Cert interface to check whether X509
certificate matches the specified private key.

Change-Id: I6732f8a1ad9ed1b1ab26b94b582af50e6b6d24b1
---
M src/kudu/security/CMakeLists.txt
A src/kudu/security/cert-test.cc
M src/kudu/security/cert.cc
M src/kudu/security/cert.h
4 files changed, 96 insertions(+), 0 deletions(-)


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

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


[kudu-CR] java: fix ability to connect to a real Kerberized cluster

2017-02-07 Thread Jean-Daniel Cryans (Code Review)
Jean-Daniel Cryans has posted comments on this change.

Change subject: java: fix ability to connect to a real Kerberized cluster
..


Patch Set 3:

(4 comments)

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

PS3, Line 210:  
nit


Line 245: 
nit


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

PS3, Line 86: .
remove


http://gerrit.cloudera.org:8080/#/c/5922/3/java/kudu-client/src/test/java/org/apache/kudu/client/MiniKdc.java
File java/kudu-client/src/test/java/org/apache/kudu/client/MiniKdc.java:

PS3, Line 328: .
remove


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6b96fad3cfb40500d7a75e5070ea21bc8e00cbd8
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] java: use a netty frame decoder instead of replaying decoder

2017-02-07 Thread Jean-Daniel Cryans (Code Review)
Jean-Daniel Cryans has posted comments on this change.

Change subject: java: use a netty frame decoder instead of replaying decoder
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5926/2/java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java:

Line 174: // TODO(todd): this should return here. We seem to lack test 
coverage!
Wouldn't that require a server that doesn't support feature flags... ?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ec1e6f9cbbdf694fe85b7668a3c349efc26c08f
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] WIP [security] load TSK from system table

2017-02-07 Thread Alexey Serbin (Code Review)
Alexey Serbin has uploaded a new change for review.

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

Change subject: WIP [security] load TSK from system table
..

WIP [security] load TSK from system table

Change-Id: Ie91d4129bda0ca49e81988c28385895a2abcd201
---
M src/kudu/master/authn_token_manager.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.cc
M src/kudu/master/master.proto
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/security/token_signer.cc
M src/kudu/security/token_signer.h
9 files changed, 263 insertions(+), 10 deletions(-)


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

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


[kudu-CR] WIP [security] tailored TokenSigner for system catalog

2017-02-07 Thread Alexey Serbin (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: WIP [security] tailored TokenSigner for system catalog
..

WIP [security] tailored TokenSigner for system catalog

Updated the TokenSigner class in preparation to loading
public part of token signing keys from the system catalog.

The expected use-case for the TokenSigner is calling
AddTokenSigningPublicKey() multiple times while loading public part
of TSKs from the system catalog and subsequent call of Init().
That's the sequence to be exercised by a master server upon becoming
a leader.  It's possible to run this sequence multiple times
on the same instance of TokenSigner, generating new signing keys
only when already existing signing keys are about to expire.

Also, the TokenSigner class now embeds the logic to rotate its
signing key on Init(), if necessary.

Change-Id: Ie2417e2ccba6a1114db366b2f642f95362bf479c
---
M src/kudu/master/authn_token_manager.cc
M src/kudu/master/authn_token_manager.h
M src/kudu/master/master.cc
M src/kudu/security/token-test.cc
M src/kudu/security/token_signer.cc
M src/kudu/security/token_signer.h
M src/kudu/security/token_signing_key.cc
M src/kudu/security/token_signing_key.h
8 files changed, 307 insertions(+), 54 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie2417e2ccba6a1114db366b2f642f95362bf479c
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] Fix TSAN segfault on Centos 6

2017-02-07 Thread Dan Burkert (Code Review)
Dan Burkert has submitted this change and it was merged.

Change subject: Fix TSAN segfault on Centos 6
..


Fix TSAN segfault on Centos 6

The OpenSSL version shipped with Centos 6 appears to segfault if a
nullptr is passed to OBJ_find_sigid_algs with TSAN. I haven't been able
to actually figure out why, I haven't been succesful in finding the
Centos 6 source for OpenSSL, and the version it's based on correctly
checks for null[1]. Attempting to set a breakpoint on the function
yielded no source, even with the debuginfo OpenSSL rpm installed.

[1] 
https://github.com/openssl/openssl/blob/OpenSSL_1_0_1e/crypto/objects/obj_xref.c#L115

Change-Id: I5ea3ad372f98ff423425858c09cb0977f372b2e5
Reviewed-on: http://gerrit.cloudera.org:8080/5924
Reviewed-by: Todd Lipcon 
Tested-by: Kudu Jenkins
---
M src/kudu/security/cert.cc
1 file changed, 2 insertions(+), 1 deletion(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I5ea3ad372f98ff423425858c09cb0977f372b2e5
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] compaction: Add additional validation in DeltaTracker

2017-02-07 Thread Jean-Daniel Cryans (Code Review)
Jean-Daniel Cryans has posted comments on this change.

Change subject: compaction: Add additional validation in DeltaTracker
..


Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I05fed515bc5d6a09484fadb61e73ac1346f6654a
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: No


[kudu-CR] tablet: Include peer uuid in log prefix

2017-02-07 Thread Jean-Daniel Cryans (Code Review)
Jean-Daniel Cryans has posted comments on this change.

Change subject: tablet: Include peer uuid in log prefix
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I21c8f568ab3e05cc9e9376f391477c5c35336983
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No


[kudu-CR] tablet: Include peer uuid in log prefix

2017-02-07 Thread Jean-Daniel Cryans (Code Review)
Jean-Daniel Cryans has submitted this change and it was merged.

Change subject: tablet: Include peer uuid in log prefix
..


tablet: Include peer uuid in log prefix

Also, add LogPrefix() to the tablet mm op.

Change-Id: I21c8f568ab3e05cc9e9376f391477c5c35336983
Reviewed-on: http://gerrit.cloudera.org:8080/5918
Tested-by: Kudu Jenkins
Reviewed-by: Jean-Daniel Cryans 
---
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_mm_ops.cc
M src/kudu/tablet/tablet_mm_ops.h
4 files changed, 43 insertions(+), 25 deletions(-)

Approvals:
  Jean-Daniel Cryans: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I21c8f568ab3e05cc9e9376f391477c5c35336983
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR] compaction: Add additional validation in DeltaTracker

2017-02-07 Thread David Ribeiro Alves (Code Review)
David Ribeiro Alves has posted comments on this change.

Change subject: compaction: Add additional validation in DeltaTracker
..


Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I05fed515bc5d6a09484fadb61e73ac1346f6654a
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: No


[kudu-CR] client: trust master's cert, adopt authn token

2017-02-07 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change.

Change subject: client: trust master's cert, adopt authn token
..


Patch Set 7: Code-Review+2

restarted verify, I think it was fallout from the workspace issues.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaa3eec178110b77d81608cfdce989e0fe494ee05
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] WIP: enable kerberos support in CSD

2017-02-07 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change.

Change subject: WIP: enable kerberos support in CSD
..


Patch Set 1:

I think we should hold off on committing this until we review all of the new 
security flags comprehensively.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iea2d36b7a0da38cd6ddd646a32f52517cabeb4a6
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No


[kudu-CR] [thirdparty] Make Boost a regular dependency

2017-02-07 Thread David Ribeiro Alves (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: [thirdparty] Make Boost a regular dependency
..

[thirdparty] Make Boost a regular dependency

For the new TIMESTAMP type we must mimic Impala's
TimestampValue format to make sure that the types have
the same memory format. This will allow to avoid costly
conversions at scan time. Impala's type uses boost's ptime
and nanosecond resolution and so must Kudu.

Previously our use of boost was header-only, but the
date_time library (which includes ptime) needs to be compiled.
This requires that we actually compile libboost_date_time in
thirparty. This patch does that and also changes the way we
handle boost to treat it more like a regular dependency, both
regarding the new static and shared library files and regarding
headers.

Since boost uses its own build system this required tinkering
with jamfiles to make sure that the library is correctly
built under tsan.

Finally this patch also addresses a couple of other issues:
- Adds "thirdparty/installed/common/include" to the include
dirs explicitly (we were doing this indirectly when we added
the boost include dir before).
- Since FindKuduBoost.cmake now explicitly sets BOOST_INCLUDE_DIR
to point to wherever boost was installed, this removes the
logic that makes sure we didn't pick up boost from the
system path.

Change-Id: I277c4fda15575e271c426735552762884cb28c43
---
M CMakeLists.txt
A cmake_modules/FindKuduBoost.cmake
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
4 files changed, 125 insertions(+), 28 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I277c4fda15575e271c426735552762884cb28c43
Gerrit-PatchSet: 9
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: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] [thirdparty] Make Boost a regular dependency

2017-02-07 Thread David Ribeiro Alves (Code Review)
David Ribeiro Alves has posted comments on this change.

Change subject: [thirdparty] Make Boost a regular dependency
..


Patch Set 8:

(2 comments)

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

PS5, Line 9: However the
   : new timestamp type makes extensive use of boost's date_time
   : and, as such, we need to compile at least libboostdate_time.
> Makes sense. Could you add this information to the commit message?
Done


http://gerrit.cloudera.org:8080/#/c/5818/8/cmake_modules/FindKuduBoost.cmake
File cmake_modules/FindKuduBoost.cmake:

PS8, Line 18: BOOST_DATE_TIME - libboost_date_time.a, libboost_date_time.so,
: # and libboost_date_time.so.1
> Nit: this will be an onerous list to maintain once other boost components a
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I277c4fda15575e271c426735552762884cb28c43
Gerrit-PatchSet: 8
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: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] [thirdparty] Make Boost a regular dependency

2017-02-07 Thread David Ribeiro Alves (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: [thirdparty] Make Boost a regular dependency
..

[thirdparty] Make Boost a regular dependency

For the new TIMESTAMP type we must mimic Impala's
TimestampValue format to make sure that the types have
the same memory format. This will allow to avoid costly
conversions at scan time. Impala's type uses boost's ptime
and nanosecond resolution and so must Kudu.

Previously our use of boost was header-only, but the
date_time library (which includes ptime) needs to be compiled.
This requires that we actually compile libboost_date_time in
thirparty. This patch does that and also changes the way we
handle boost to treat it more like a regular dependency, both
regarding the new static and shared library files and regarding
headers.

Since boost uses its own build system this required tinkering
with jamfiles to make sure that the library is correctly
built under tsan.

Finally this patch also addresses a couple of other issues:
- Adds "thirdparty/installed/common/include" to the include
dirs explicitly (we were doing this indirectly when we added
the boost include dir before).
- Since FindKuduBoost.cmake now explicitly sets BOOST_INCLUDE_DIR
to point to wherever boost was installed, this removes the
logic that makes sure we didn't pick up boost from the
system path.

Change-Id: I277c4fda15575e271c426735552762884cb28c43
---
M CMakeLists.txt
A cmake_modules/FindKuduBoost.cmake
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
4 files changed, 124 insertions(+), 28 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/18/5818/10
-- 
To view, visit http://gerrit.cloudera.org:8080/5818
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I277c4fda15575e271c426735552762884cb28c43
Gerrit-PatchSet: 10
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: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] master: issue authentication tokens and CA certs to clients

2017-02-07 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change.

Change subject: master: issue authentication tokens and CA certs to clients
..


Patch Set 7: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5969b8e125633b3b14364b98c0d0a992b162f302
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] master rpc: pass back more details from ConnectToCluster

2017-02-07 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change.

Change subject: master_rpc: pass back more details from ConnectToCluster
..


Patch Set 6: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3b5056c9f31237249516a2e7ae68d641c9f6bd02
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No