date:20170127

Mike Percy has posted comments on this change.

Change subject: Control mutex stack walking in DEBUG mode with a gflag
..


Patch Set 7:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5741/2//COMMIT_MSG
Commit Message:

PS2, Line 9: In the case of a CHECK() failure, both the crashing thread and the
   : AsyncLogger thread may simultaneously attempt to symbolize their 
stacks.
   : AsyncLogger does this simply by acquiring a Mutex in RunThread(), 
which
   : triggers stack collection in DEBUG mode, while the crashing thread 
is
   : collecting the stack trace in order to print an important error 
message.
> Could you address this?
Done


http://gerrit.cloudera.org:8080/#/c/5741/7/src/kudu/util/mutex.cc
File src/kudu/util/mutex.cc:

Line 73:   << "; Owner stack: " << std::endl << stack_trace_->Symbolize();
> Could we at least drop the (empty) stack part of the line? Maybe if the val
Done


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

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

[kudu-CR] Control mutex stack walking in DEBUG mode with a gflag

Hello Kudu Jenkins,

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

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

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

Change subject: Control mutex stack walking in DEBUG mode with a gflag
..

Control mutex stack walking in DEBUG mode with a gflag

This patch disables the Mutex owner stack trace collection on DEBUG
builds by default, only enabling it when a certain gflag is set.

In DEBUG mode, our Mutex implementation collects a stack trace of the
owning thread each time the Mutex is acquired. It does this by calling
google::GetStackTrace() from glog, which in the context of the Kudu
build environment calls into libunwind to collect that stack trace.

At the time of writing, google::GetStackTrace() only allows access by
one thread at a time. If more than one thread attempts to invoke this
function simultaneously, there is a CAS that determines excluvisity. The
"loser" of this contest gets a short-circuit return along with an empty
stack trace, indicating a failure to collect the stack trace.

NB: I have filed a glog issue about that behavior upstream. For more
information, see https://github.com/google/glog/issues/160

This situation becomes a problem when there are one or more Mutexes
constantly being acquired. When that happens, there is always a thread
collecting a stack trace, and so the probability of being able to
successfully collect a stack trace at any given moment is greatly
reduced.

One important caller of google::GetStackTrace() is the glog failure
function and SIGABRT signal handler that is called when a CHECK() fails
or a LOG(FATAL) call is invoked. I have observed that this crash handler
will often print an empty stack trace in DEBUG mode. Investigating this
issue led me to discover that we had a thread (our AsyncLogger thread)
constantly acquiring a Mutex and racing on the above-mentioned CAS check
inside google::GetStackTrace(). Depriving our DEBUG builds of stack
traces on LOG(FATAL) or CHECK failures, especially on Jenkins runs, is
counterproductive. One simple solution to this problem is to disable
this behavior by default.

Change-Id: Ie4593cf7173867ce2f6151e03df0be94f97d95d2
---
M src/kudu/util/mutex.cc
M src/kudu/util/mutex.h
2 files changed, 37 insertions(+), 10 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie4593cf7173867ce2f6151e03df0be94f97d95d2
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] [security] method to extract public part of RSA key

Alexey Serbin has posted comments on this change.

Change subject: [security] method to extract public part of RSA key
..


Patch Set 6:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/5783/6/src/kudu/security/ca/cert_management.cc
File src/kudu/security/ca/cert_management.cc:

Line 89:   "error setting X509 public key");
> Indentation here and below.
Done


http://gerrit.cloudera.org:8080/#/c/5783/6/src/kudu/security/crypto.cc
File src/kudu/security/crypto.cc:

PS6, Line 94:   // The 'using' is here to overcome issues with 
RETURN_NOT_OK_PREPEND() macro.
:   using func = Status(*)(BIO*, DataFormat, 
c_unique_ptr*);
:   func f = ::kudu::security::FromBIO;
:  
> huh, that's weird... what's going on?
The pre-processor stuck with parsing that otherwise -- it cannot distinguish 
template parameter from function parameters.

Yep, the auto will do it, right.  I was playing with ways doing that call, auto 
seems to be better.

Another workaround is to enclose the call of the specialized template function 
into extra-braces.  Probably, that's the best option.


http://gerrit.cloudera.org:8080/#/c/5783/6/src/kudu/security/crypto.h
File src/kudu/security/crypto.h:

Line 29: class RSAPublicKey : public BasicWrapper {
> Are these classes specific to RSA keys?  Would we want to reuse them if we 
There is some specific due to FromBIO/ToBIO at least.  Yes, making this 
template is an option, and I was thinking to do that later when adding EC keys. 
 At that stage, we would have better understanding what is needed here.

Let me just change RSAPrivateKey --> RsaPrivateKey at this phase.


PS6, Line 33: RawDataType
> I think you should be able to use EVP_PKEY instead of RawDataType everywher
Done


Line 40:   Status FromBIO(BIO* bio, DataFormat format) override;
> given that this is just moved code I think my preference is not to change i
ok -- let me to that in a separate patch: I'll do that for all crypto- and 
cert-related code in the security library.


PS6, Line 71: template
: S
> hrm, I think I remember asking but I can't find the answer now... why's thi
This is a bit of forward thinking -- I was thinking that using ECDHA keys for 
token signing might be a good option.  That's why the RSA prefix for keys and 
template method here.


http://gerrit.cloudera.org:8080/#/c/5783/6/src/kudu/security/openssl_util.h
File src/kudu/security/openssl_util.h:

Line 95: enum class DataFormat {
> eh, I think this is used everywhere, like keys too, right?
Yep, I would prefer to keep it here since it's common for both keys & certs.


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

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

[kudu-CR] WIP: Add a new TIMESTAMP type

2017-01-27 Thread David Ribeiro Alves (Code Review)

David Ribeiro Alves has uploaded a new patch set (#2).

Change subject: WIP: Add a new TIMESTAMP type
..

WIP: Add a new TIMESTAMP type

This adds a new timestamp type that matches Impala's.
The new type can be used as a key and has all the facilities
of the other types except on-disk encodings.

This might still need some cleanup but should mostly be
gtg.

Change-Id: I63271adf6b87f048e30bf2d6d04c758884752afc
---
M src/kudu/cfile/type_encodings.cc
M src/kudu/client/scan_batch.cc
M src/kudu/client/scan_batch.h
M src/kudu/client/scanner-internal.cc
M src/kudu/client/schema.cc
M src/kudu/client/schema.h
M src/kudu/client/value-internal.h
M src/kudu/client/value.cc
M src/kudu/client/value.h
M src/kudu/common/common.proto
M src/kudu/common/encoded_key-test.cc
M src/kudu/common/key_encoder.cc
M src/kudu/common/key_encoder.h
M src/kudu/common/key_util-test.cc
M src/kudu/common/key_util.cc
M src/kudu/common/partial_row.cc
M src/kudu/common/partial_row.h
M src/kudu/common/partition.cc
M src/kudu/common/types.cc
M src/kudu/common/types.h
M src/kudu/integration-tests/all_types-itest.cc
M src/kudu/util/CMakeLists.txt
A src/kudu/util/timestamp_value-test.cc
A src/kudu/util/timestamp_value.cc
A src/kudu/util/timestamp_value.h
25 files changed, 843 insertions(+), 9 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I63271adf6b87f048e30bf2d6d04c758884752afc
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-1845: Kerberos client keytab should be periodically renewed

2017-01-27 Thread Sailesh Mukil (Code Review)

Sailesh Mukil has uploaded a new change for review.

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

Change subject: KUDU-1845: Kerberos client keytab should be periodically renewed
..

KUDU-1845: Kerberos client keytab should be periodically renewed

With the current kerberos support, if a ticket expires, all
communication on a secure cluster would stop as we do not attempt
to renew the ticket.

This patch adds a renewal thread which periodically wakes up and
either renews the Ticket Granting Ticket or acquires a new one if
the renewal window is closed. The renew interval is controlled by
a newly introduced flag called 'kerberos_reinit_interval'.

Change-Id: Ic4c072c1210216369e60eac88be4a20d9b166b2d
TODO: Add tests. Still working on it.
---
M src/kudu/security/init.cc
1 file changed, 137 insertions(+), 27 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic4c072c1210216369e60eac88be4a20d9b166b2d
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil

[kudu-CR] [security] do actual token signing/verification

Dan Burkert has posted comments on this change.

Change subject: [security] do actual token signing/verification
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5812/2/src/kudu/security/token_signing_key.cc
File src/kudu/security/token_signing_key.cc:

Line 35:   CHECK(pb_.has_rsa_key_der());
These CHECKs in the ctors are a little fishy.  Maybe we could add an Init 
function that takes the pb, or perhaps delay extracting the key until the Sign 
call?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf035c64032320a450731ae921e92615bf2efd98
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [security] method to extract public part of RSA key

2017-01-27 Thread Todd Lipcon (Code Review)

Todd Lipcon has posted comments on this change.

Change subject: [security] method to extract public part of RSA key
..


Patch Set 6:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/5783/6/src/kudu/security/crypto.cc
File src/kudu/security/crypto.cc:

PS6, Line 94:   // The 'using' is here to overcome issues with 
RETURN_NOT_OK_PREPEND() macro.
:   using func = Status(*)(BIO*, DataFormat, 
c_unique_ptr*);
:   func f = ::kudu::security::FromBIO;
:  
huh, that's weird... what's going on?

This seems like a nice case for 'auto' btw


http://gerrit.cloudera.org:8080/#/c/5783/6/src/kudu/security/crypto.h
File src/kudu/security/crypto.h:

Line 40:   Status FromBIO(BIO* bio, DataFormat format) override;
> Status returning methods should have WARN_UNUSED_RESULT
given that this is just moved code I think my preference is not to change it 
much in the same patch


PS6, Line 71: template
: S
hrm, I think I remember asking but I can't find the answer now... why's this a 
template?


http://gerrit.cloudera.org:8080/#/c/5783/6/src/kudu/security/openssl_util.h
File src/kudu/security/openssl_util.h:

Line 52: class Status;
no need for forward decl since it's included


Line 95: enum class DataFormat {
> Consider moving this to cert.h as well.
eh, I think this is used everywhere, like keys too, right?


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

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

[kudu-CR] [security] sign/verify data using RSA key pair

Dan Burkert has posted comments on this change.

Change subject: [security] sign/verify data using RSA key pair
..


Patch Set 6: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3fa04bb7d09aa416363998e2f8d7ccbdea625e4f
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
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] [security] introduced crypto-test

Dan Burkert has posted comments on this change.

Change subject: [security] introduced crypto-test
..


Patch Set 6: Code-Review+2

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

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

[kudu-CR] [security] method to extract public part of RSA key

Dan Burkert has posted comments on this change.

Change subject: [security] method to extract public part of RSA key
..


Patch Set 6:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/5783/6/src/kudu/security/ca/cert_management.cc
File src/kudu/security/ca/cert_management.cc:

Line 89:   "error setting X509 public key");
Indentation here and below.


http://gerrit.cloudera.org:8080/#/c/5783/6/src/kudu/security/crypto.h
File src/kudu/security/crypto.h:

Line 29: class RSAPublicKey : public BasicWrapper {
Are these classes specific to RSA keys?  Would we want to reuse them if we 
moved to, say, elliptic curve keys in the future?  If so it may be better to 
call them 'PublicKey' and 'PrivateKey'.  Also per the style guide RSA should be 
Rsa in identifiers.


PS6, Line 33: RawDataType
I think you should be able to use EVP_PKEY instead of RawDataType everywhere 
but the typedef.  I've found that a bit easier to follow.


Line 40:   Status FromBIO(BIO* bio, DataFormat format) override;
Status returning methods should have WARN_UNUSED_RESULT


http://gerrit.cloudera.org:8080/#/c/5783/6/src/kudu/security/openssl_util.h
File src/kudu/security/openssl_util.h:

Line 95: enum class DataFormat {
Consider moving this to cert.h as well.


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

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

[kudu-CR] [timestamp] Build and and add boost's date time lib

2017-01-27 Thread David Ribeiro Alves (Code Review)

David Ribeiro Alves has uploaded a new change for review.

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

Change subject: [timestamp] Build and and add boost's date_time lib
..

[timestamp] Build and and add boost's date_time lib

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


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I277c4fda15575e271c426735552762884cb28c43
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves

[kudu-CR] WIP: Add a new TIMESTAMP type

2017-01-27 Thread David Ribeiro Alves (Code Review)

David Ribeiro Alves has uploaded a new change for review.

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

Change subject: WIP: Add a new TIMESTAMP type
..

WIP: Add a new TIMESTAMP type

This adds a new timestamp type that matches Impala's.
The new type can be used as a key and has all the facilities
of the other types except on-disk encodings.

This might still need some cleanup but should mostly be
gtg.

Change-Id: I63271adf6b87f048e30bf2d6d04c758884752afc
---
M src/kudu/cfile/type_encodings.cc
M src/kudu/client/scan_batch.cc
M src/kudu/client/scan_batch.h
M src/kudu/client/scanner-internal.cc
M src/kudu/client/schema.cc
M src/kudu/client/schema.h
M src/kudu/client/value-internal.h
M src/kudu/client/value.cc
M src/kudu/client/value.h
M src/kudu/common/common.proto
M src/kudu/common/encoded_key-test.cc
M src/kudu/common/key_encoder.cc
M src/kudu/common/key_encoder.h
M src/kudu/common/key_util-test.cc
M src/kudu/common/key_util.cc
M src/kudu/common/partial_row.cc
M src/kudu/common/partial_row.h
M src/kudu/common/partition.cc
M src/kudu/common/types.cc
M src/kudu/common/types.h
M src/kudu/integration-tests/all_types-itest.cc
M src/kudu/util/CMakeLists.txt
A src/kudu/util/timestamp_value-test.cc
A src/kudu/util/timestamp_value.cc
A src/kudu/util/timestamp_value.h
25 files changed, 843 insertions(+), 9 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I63271adf6b87f048e30bf2d6d04c758884752afc
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves

[kudu-CR] tls: hook up internal PKI system to TlsContext

2017-01-27 Thread Todd Lipcon (Code Review)

Hello Dan Burkert, Alexey Serbin, Kudu Jenkins,

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

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

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

Change subject: tls: hook up internal PKI system to TlsContext
..

tls: hook up internal PKI system to TlsContext

This hooks up the existing ServerCertManager to TlsContext, so that once
a server has a cert signed by the master's CA, that cert will be
configured in the Messenger's TlsContext.

A number of changes were necessary to get this to work:

* The master needs to sign its own server cert with its CA cert. As
  before, this is still done in the "wrong place" -- we eventually need
  to do it somewhere in the CatalogManager after we've loaded the
  persisted CA cert. But, for now, we're using the ephemeral one and
  self-signing at startup.

* When the master signs a tablet server's cert, it needs to send back
  the CA cert as well as the signed TS cert. The TS then needs to add
  the CA cert as a trusted cert in its TlsContext.

* Noticed that we hadn't marked the cert fields as redacted in the proto
  and did so along the way.

* Changed negotiation so that, rather than depending on the global
  configuration flags to know whether TLS was enabled, now consults the
  TlsContext to know if the context has a configured cert and CA.

* Moved the configuration of SSL verification to a per-handshake setting
  rather than in TlsContext. I also added various tests for this. I'll
  break this out into a separate change. The idea here is so that the
  client, when it plans to use tokens, can enable the verification
  before running the TlsHandshake.

* TlsContext now allows passing in the certificate and key using our
  wrapper objects instead of just file paths. I also combined the
  methods to set a cert and set a key, since we'll always use them
  together.

* The new method to set the cert and key now verifies that the cert and
  key match each other, and that the appropriate CA certs are already
  installed such that a full trusted cert chain is established.

This is still a WIP, but I verified that I can run a tablet server and
master, and connect to them from a client, and that all of the traffic
is encrypted (based on tshark output).

Before committing we should also look at whether this causes a big
performance regression since it encrypts all traffic, and have a way to
disable TLS if so.

Change-Id: I6a98ef522e51751562d47907aa90a32c2dac3918
---
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/client_negotiation.cc
M src/kudu/rpc/messenger.cc
M src/kudu/rpc/messenger.h
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/security/openssl_util.cc
M src/kudu/security/openssl_util.h
M src/kudu/security/server_cert_manager.cc
M src/kudu/security/server_cert_manager.h
M src/kudu/security/test/test_certs.h
M src/kudu/security/tls_context.cc
M src/kudu/security/tls_context.h
M src/kudu/security/tls_handshake-test.cc
M src/kudu/security/tls_handshake.cc
M src/kudu/security/tls_handshake.h
M src/kudu/server/server_base.cc
M src/kudu/tserver/heartbeater.cc
23 files changed, 442 insertions(+), 82 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6a98ef522e51751562d47907aa90a32c2dac3918
Gerrit-PatchSet: 2
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] Add Google Breakpad support to Kudu

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 (#25).

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,012 insertions(+), 84 deletions(-)

git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/5620/25
--
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: 25
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

Mike Percy has posted comments on this change.

Change subject: Add Google Breakpad support to Kudu
..


Patch Set 21:

(2 comments)

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

PS21, Line 85: #if defined(ADDRESS_SANITIZER)
 :   // CHECK raises a SIGSEGV under ASAN and fails this test.
 :   return;
 : #endif
> I have investigated this a bit and so far I'm not quite sure why it's gener
I finally figured this out. The failure function must not be nullptr and must 
call abort().


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

PS21, Line 261:   // Send SIGUSR1 signal to thread, which will wake it up.
  :   kill(getpid(), SIGUSR1);
> > Incidentally, a blocked SIGUSR1 will be inherited by all Kudu subprocesse
Done


-- 
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: 21
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] tls: hook up internal PKI system to TlsContext

2017-01-27 Thread Todd Lipcon (Code Review)

Todd Lipcon has posted comments on this change.

Change subject: tls: hook up internal PKI system to TlsContext
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5808/1/src/kudu/master/master_service.cc
File src/kudu/master/master_service.cc:

PS1, Line 146:   if (req->has_csr_der()) {
 : string cert, ca_cert;
 : Status s = 
server_->cert_authority()->SignServerCSR(req->csr_der(), , _cert);
 : if (!s.ok()) {
 :   rpc->RespondFailure(s.CloneAndPrepend("invalid CSR"));
 :   return;
 : }
 : LOG(INFO) << "Signed X509 certificate for tserver " << 
rpc->requestor_string();
 : resp->mutable_signed_cert_der()->swap(cert);
 : resp->mutable_signed_ca_cert_der()->swap(ca_cert);
 :   }
> I should have it asked earlier, but anyway.
yea, one of the items on the list is to use the "service-level authorization" 
stuff to ensure that only tablet servers (identified by a 'kudu/HOST@REALM' 
principal) can send heartbeats. That keeps anyone else (eg todd@REALM) from 
impersonating a tserver.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6a98ef522e51751562d47907aa90a32c2dac3918
Gerrit-PatchSet: 1
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: Yes

[kudu-CR] tls: hook up internal PKI system to TlsContext

Alexey Serbin has posted comments on this change.

Change subject: tls: hook up internal PKI system to TlsContext
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5808/1/src/kudu/master/master_service.cc
File src/kudu/master/master_service.cc:

PS1, Line 146:   if (req->has_csr_der()) {
 : string cert, ca_cert;
 : Status s = 
server_->cert_authority()->SignServerCSR(req->csr_der(), , _cert);
 : if (!s.ok()) {
 :   rpc->RespondFailure(s.CloneAndPrepend("invalid CSR"));
 :   return;
 : }
 : LOG(INFO) << "Signed X509 certificate for tserver " << 
rpc->requestor_string();
 : resp->mutable_signed_cert_der()->swap(cert);
 : resp->mutable_signed_ca_cert_der()->swap(ca_cert);
 :   }
I should have it asked earlier, but anyway.

IFRC, it's guaranteed that the heartbeat is received from an authenticated 
party, right?  I.e., we cannot receive such a heartbeat from a 
non-authenticated peer.

However, how do we verify that the peer is not a rogue server?  I.e., imagine a 
valid authenticated client sending such a heartbeat in attempt to get a 
certificate for a fake tablet server to be able to gain non-restricted access 
to the data?

Do we configure our Kerberos access rules so only tablet servers can be 
authenticated in the context of processing such  RPC call?


http://gerrit.cloudera.org:8080/#/c/5808/1/src/kudu/security/server_cert_manager.h
File src/kudu/security/server_cert_manager.h:

PS1, Line 48: QUID
nit: UUID?


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

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

[kudu-CR] Update Impala docs for upcoming Impala 2.8 release

2017-01-27 Thread Ambreen Kazi (Code Review)

Ambreen Kazi has posted comments on this change.

Change subject: Update Impala docs for upcoming Impala 2.8 release
..


Patch Set 2:

(2 comments)

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

PS2, Line 158: Tables are divided into tablets which are each served by one or 
more tablet
 : servers. Ideally, tablets should split a table's data relatively 
equally. Kudu currently
 : has no mechanism for automatically (or manually) splitting a 
pre-existing tablet.
 : Until this feature has been implemented, *you must specify your 
partitioning when
 : creating a table*. When designing your table schema, consider 
primary keys that will allow you to
 : split your table into partitions which grow at similar rates. 
You can designate
 : partitions using a `PARTITION BY` clause when creating a table 
using Impala:
 : 
 : NOTE: Impala keywords, such as `group`, are enclosed by 
back-tick characters when
 : they are not used in their keyword sense.
This text has been duplicated in the Partitioning Tables section below.


PS2, Line 370: `DISTRIBUTE
 : BY HASH (id, sku)`.
old syntax - DISTRIBUTE BY


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia43d18e8d92c52e5868e1d48b91351bca41b53f8
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1853. Tablet copy: Don't orphan blocks on failure

2017-01-27 Thread Adar Dembo (Code Review)

Adar Dembo has posted comments on this change.

Change subject: KUDU-1853. Tablet copy: Don't orphan blocks on failure
..


Patch Set 4:

(2 comments)

Looks good, though the test failures look real.

http://gerrit.cloudera.org:8080/#/c/5799/4/src/kudu/tserver/tablet_copy_client.cc
File src/kudu/tserver/tablet_copy_client.cc:

PS4, Line 114:   Status s = Abort();
 :   if (PREDICT_FALSE(!s.ok())) {
 : LOG_WITH_PREFIX(FATAL) << "Failed to fully clean up tablet 
after aborted copy: "
 :<< s.ToString();
 :   }
Why not just WARN_NOT_OK()? Why FATAL?


Line 212:   // Set the data state to COPYING by default.
Maybe "Set the data state to COPYING to signify that, on crash, this replica 
should be discarded."


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbc0720d460de8d912a241a5186ef6e8f524b830
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [security] do actual token signing/verification

Hello Kudu Jenkins,

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

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

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

Change subject: [security] do actual token signing/verification
..

[security] do actual token signing/verification

Replaced temporary stub sign/verify calls with calls of actual
cryptographic functions.

Change-Id: Icf035c64032320a450731ae921e92615bf2efd98
---
M src/kudu/security/token_signing_key.cc
M src/kudu/security/token_signing_key.h
2 files changed, 23 insertions(+), 9 deletions(-)


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

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

[kudu-CR] tls: hook up internal PKI system to TlsContext

Dan Burkert has posted comments on this change.

Change subject: tls: hook up internal PKI system to TlsContext
..


Patch Set 1:

(4 comments)

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

Line 28: * Changed negotiation so that, rather than depending on the global
Servers need to be able to accept unauthenticated TLS connections before their 
CSR is complete, but only if GSSAPI mechanism is being used for strong authn.  
I think the way we want to do this is to have the server always advertise TLS 
support, and then change the advertised SASL mechanisms based on whether the 
CSR is complete:

CSR is complete (server has a signed cert):
  Advertise the KUDU_TOKEN and GSSAPI mechs

CSR is incomplete (server has a temporary self-signed cert):
  Advertise the GSSAPI mech


Line 32: * Disabled a lot of the configuration of SSL_CTX_set_verify(). This was
Sounds good, per the above comment we would want to verify in the client when 
using the KUDU_TOKEN mech, and not verify when using the GSSAPI mech.  The 
server will never verify.


Line 38: * TlsContext now allows passing in the certificate and key using our
I think this is a little bit backwards.  The server TlsContext should not need 
the CA cert to be in its trusted store.  The client TlsContext does need the CA 
cert to be in its trusted store.  I realize these cases overlap, but I think 
what it means is that we shouldn't be using heartbeats to propagate the CA 
cert, it should be done explicitly on first connect to the master.  That way 
the mechanism can be reused by clients and servers.


http://gerrit.cloudera.org:8080/#/c/5808/1/src/kudu/master/master.cc
File src/kudu/master/master.cc:

Line 93:   CHECK(csr_der != boost::none);
This can be simplified to

CHECK(csr_der);


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

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

[kudu-CR] [security] do actual signing/verify for tokens

Alexey Serbin has uploaded a new change for review.

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

Change subject: [security] do actual signing/verify for tokens
..

[security] do actual signing/verify for tokens

Replaced temporary stub sign/verify calls with calls of actual
cryptographic functions.

Change-Id: Icf035c64032320a450731ae921e92615bf2efd98
---
M src/kudu/security/token_signing_key.cc
M src/kudu/security/token_signing_key.h
2 files changed, 15 insertions(+), 8 deletions(-)


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

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

[kudu-CR] WIP: KUDU-1713: add a client Partitioner API

2017-01-27 Thread Matthew Jacobs (Code Review)

Matthew Jacobs has posted comments on this change.

Change subject: WIP: KUDU-1713: add a client Partitioner API
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5775/1/src/kudu/client/client.h
File src/kudu/client/client.h:

PS1, Line 2191: 
not necessarily?


http://gerrit.cloudera.org:8080/#/c/5775/2/src/kudu/client/client.h
File src/kudu/client/client.h:

Line 2216:   Status PartitionRow(const KuduPartialRow& row, int* partition);
> I guess my point isn't that the view you get is necessarily a consistent sn
I agree it's a nice property to have, but FWIW for now in Impala we'll end up 
creating these on all hosts in a query so we won't be able to have a consistent 
view even within a query. I don't think we can change that until the metadata 
is something small and that we can serialize.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic08a078c75f15ef4200219b5260cfb99d79b72cc
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [security] method to extract public part of RSA key

Hello Kudu Jenkins,

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

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

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

Change subject: [security] method to extract public part of RSA key
..

[security] method to extract public part of RSA key

Added a method to get the public part of an RSA keypair.
Renamed Key --> RSAPrivateKey.  Reorganized the layout of files in the
src/security sub-directory: separated the common crypto stuff from
openssl_util.h into crypto.h.

Change-Id: I8ac61a08c8165152d0046468c0fa655131ff8fef
---
M src/kudu/master/master_cert_authority.cc
M src/kudu/master/master_cert_authority.h
M src/kudu/security/CMakeLists.txt
R src/kudu/security/ca/cert_management-test.cc
M src/kudu/security/ca/cert_management.cc
M src/kudu/security/ca/cert_management.h
A src/kudu/security/cert.cc
A src/kudu/security/cert.h
A src/kudu/security/crypto.cc
A src/kudu/security/crypto.h
M src/kudu/security/openssl_util.cc
M src/kudu/security/openssl_util.h
A src/kudu/security/openssl_util_bio.h
M src/kudu/security/server_cert_manager.cc
M src/kudu/security/server_cert_manager.h
M src/kudu/security/test/test_certs.cc
M src/kudu/security/test/test_certs.h
M src/kudu/security/tls_handshake-test.cc
M src/kudu/security/tls_handshake.h
M src/kudu/security/tls_socket.cc
M src/kudu/security/token_signer.cc
M src/kudu/security/token_signing_key.cc
M src/kudu/security/token_signing_key.h
23 files changed, 662 insertions(+), 287 deletions(-)


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

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

[kudu-CR] KUDU-1831. Java client does not check if the primary key columns are specified first

2017-01-27 Thread Jean-Daniel Cryans (Code Review)

Jean-Daniel Cryans has posted comments on this change.

Change subject: KUDU-1831. Java client does not check if the primary key 
columns are specified first
..


Patch Set 4: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ca2a572801c964331ed65c630db28436fcaf86a
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jun He 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Jun He 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-1831. Java client does not check if the primary key columns are specified first

2017-01-27 Thread Jean-Daniel Cryans (Code Review)

Jean-Daniel Cryans has submitted this change and it was merged.

Change subject: KUDU-1831. Java client does not check if the primary key 
columns are specified first
..


KUDU-1831. Java client does not check if the primary key columns
are specified first

This commit cleans up the java doc in Schema class, add some info
to the java doc in AsyncKuduClient class, and add a test to cover
the out of order primary key case.

Change-Id: I1ca2a572801c964331ed65c630db28436fcaf86a
Reviewed-on: http://gerrit.cloudera.org:8080/5723
Tested-by: Kudu Jenkins
Reviewed-by: Jean-Daniel Cryans 
---
M java/kudu-client/src/main/java/org/apache/kudu/Schema.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java
3 files changed, 30 insertions(+), 2 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I1ca2a572801c964331ed65c630db28436fcaf86a
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jun He 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Jun He 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] [security] introduced crypto-test

Hello Kudu Jenkins,

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

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

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

Change subject: [security] introduced crypto-test
..

[security] introduced crypto-test

The new crypto-test module is for test scenarios involving common
non-CA crypto entities in the Kudu security library.  The CA-related
scenarios are kept in the cert_management-test.

Change-Id: Ibace36b6336b650879cf3f9139ccf562f82765a7
---
M src/kudu/security/CMakeLists.txt
M src/kudu/security/ca/cert_management-test.cc
A src/kudu/security/crypto-test.cc
M src/kudu/security/test/test_certs.cc
M src/kudu/security/test/test_certs.h
5 files changed, 208 insertions(+), 115 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibace36b6336b650879cf3f9139ccf562f82765a7
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot

[kudu-CR] [security] method to extract public part of RSA key

Hello Kudu Jenkins,

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

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

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

Change subject: [security] method to extract public part of RSA key
..

[security] method to extract public part of RSA key

Added a method to get the public part of an RSA keypair.
Renamed Key --> RSAPrivateKey.  Reorganized the layout of files in the
src/security sub-directory: separated the common crypto stuff from
openssl_util.h into crypto.h.

Change-Id: I8ac61a08c8165152d0046468c0fa655131ff8fef
---
M src/kudu/master/master_cert_authority.cc
M src/kudu/master/master_cert_authority.h
M src/kudu/security/CMakeLists.txt
R src/kudu/security/ca/cert_management-test.cc
M src/kudu/security/ca/cert_management.cc
M src/kudu/security/ca/cert_management.h
A src/kudu/security/cert.cc
A src/kudu/security/cert.h
A src/kudu/security/crypto.cc
A src/kudu/security/crypto.h
M src/kudu/security/openssl_util.cc
M src/kudu/security/openssl_util.h
A src/kudu/security/openssl_util_bio.h
M src/kudu/security/server_cert_manager.cc
M src/kudu/security/server_cert_manager.h
M src/kudu/security/test/test_certs.cc
M src/kudu/security/test/test_certs.h
M src/kudu/security/tls_handshake-test.cc
M src/kudu/security/tls_handshake.h
M src/kudu/security/tls_socket.cc
M src/kudu/security/token_signer.cc
M src/kudu/security/token_signing_key.cc
M src/kudu/security/token_signing_key.h
23 files changed, 663 insertions(+), 287 deletions(-)


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

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

[kudu-CR] [security] sign/verify data using RSA key pair