date:20161104

Dinesh Bhat has posted comments on this change.

Change subject: Avoid extra fsyncs of tombstoned tablets during startup
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4915/3/src/kudu/tablet/tablet_metadata.cc
File src/kudu/tablet/tablet_metadata.cc:

Line 173:   return Status::OK();
> OK, I reorganized the code a bit in the new revision of the patch: now we c
Perhaps I may be missing the entire picture here. Do we flush the 
tablet_data_state_ before we delete log/meta ? It appeared like with your patch 
earlier we were skipping tablet_data_state_ flush when a healthy but empty 
tablet was about to get tombstoned. If that's true, the situation I was 
describing was, suppose tserver receives a DeleteTablet rpc on a healthy but 
empty tablet (say due to a change_config triggered), then if the server crashes 
after it deletes the log/meta (but before we delete the superblock), then we 
can land up in that situation, no ? The situation being, a healthy superblock 
with no log/meta when tserver is coming up, not sure what state machine the 
tablet will go through at that point.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I60cb184b8de2a6a381371ddcf2fb938a19757eec
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [OpenSSL] require at least 1.0.0 version

Hello Dan Burkert, Kudu Jenkins,

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

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

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

Change subject: [OpenSSL] require at least 1.0.0 version
..

[OpenSSL] require at least 1.0.0 version

To build the project, it's necessary to have OpenSSL version 1.0.0
or higher: we are using some features which have been introduced
in version 1.0.0 of the OpenSSL framework.

Change-Id: Ie1b60f730f3d6e5645a310fb40df39d04367e688
---
M CMakeLists.txt
1 file changed, 14 insertions(+), 1 deletion(-)


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

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

[kudu-CR] Improve debuggability of the delta/compaction path

Alexey Serbin has posted comments on this change.

Change subject: Improve debuggability of the delta/compaction path
..


Patch Set 2:

(3 comments)

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

PS2, Line 35: type == UNDO ? "UNDO" : "REDO"
nit: consider using the DeltaType_Name() function here instead.


PS2, Line 36: Substitute("$0@tx$1"
The previous format had fixed width TX ids, which is more readable, IMHO.  What 
is the reason for dropping that feature?


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

PS2, Line 612: (delta_type_ == DeltaType::REDO ? "REDO" : "UNDO")
Consider using DeltaType_Name() function instead.


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

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

[kudu-CR] Add a macro to LOG and return on a non-OK status

Alexey Serbin has posted comments on this change.

Change subject: Add a macro to LOG and return on a non-OK status
..


Patch Set 2:

(5 comments)

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

Nit: if possible, please keep the commit message line length under 72 
characters:

https://git-scm.com/book/en/v2/Distributed-Git-Contributing-to-a-Project#_commit_guidelines


PS2, Line 22: beggining
nit: typo, should have been beginning


http://gerrit.cloudera.org:8080/#/c/4927/2/src/kudu/util/status.h
File src/kudu/util/status.h:

PS2, Line 65: and
nit: log it as 'msg' at 'level' and return the status


PS2, Line 66: KUDU_RETURN_NOT_OK_LOG
May be, name it KUDU_LOG_AND_RETURN_MSG() since it's very similar to 
KUDU_LOG_AND_RETURN()?

And re-implement the KUDU_LOG_AND_RETURN() macro via this new macro, dropping 
the additional 'Status: ' prefix?


PS2, Line 69: msg
since it's a macro parameter, it's better to enclose it into parenthesis.


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

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

[kudu-CR] [ssl] disable SSL/TLS compression

Alexey Serbin has posted comments on this change.

Change subject: [ssl] disable SSL/TLS compression
..


Patch Set 1:

(1 comment)

> (1 comment)
 > 
 > Good catch. I wasn't aware of this. Also just FYI, it looks like
 > it's disabled by default from OpenSSLv1.1.0.

I also have found that just recently while trying to understand how much 
penalty adding SSL/TLS costs.  In particular, I was watching
  https://www.youtube.com/watch?v=0EB7zh_7UE4
https://www.youtube.com/watch?v=0EB7zh_7UE4

http://gerrit.cloudera.org:8080/#/c/4962/1/src/kudu/util/net/ssl_factory.cc
File src/kudu/util/net/ssl_factory.cc:

PS1, Line 94: SSL_OP_NO_COMPRESSION
> Do you think adding a comment like this is necessary?
Yep, I think it's good idea, will add.


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

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

[kudu-CR] [ssl] disable SSL/TLS compression

Alexey Serbin has uploaded a new change for review.

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

Change subject: [ssl] disable SSL/TLS compression
..

[ssl] disable SSL/TLS compression

As for the best recommended practices for SSL/TLS deployment,
disable compression even if it's supported the both connection peers.

  https://tools.ietf.org/html/rfc7525#section-3.3

Also, disabling SSL/TLS compression frees CPU resources.

Change-Id: Ib470d1c00abb5a4bdf4650fc3ed19b6d588ea78f
---
M src/kudu/util/net/ssl_factory.cc
1 file changed, 3 insertions(+), 1 deletion(-)


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

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

[kudu-CR] ssl: switch to older APIs for initializing SSL

Todd Lipcon has submitted this change and it was merged.

Change subject: ssl: switch to older APIs for initializing SSL
..


ssl: switch to older APIs for initializing SSL

This enables support for OpenSSL 1.0.0 as found on RHEL 6.4.

Change-Id: I08fd5c3f6f8d2c228f760604dcecd7f1439578fb
Reviewed-on: http://gerrit.cloudera.org:8080/4957
Reviewed-by: Dan Burkert 
Tested-by: Kudu Jenkins
---
M src/kudu/util/net/ssl_factory.cc
1 file changed, 12 insertions(+), 4 deletions(-)

Approvals:
  Dan Burkert: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I08fd5c3f6f8d2c228f760604dcecd7f1439578fb
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] [java client] Implement RPC tracing, part 2

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

Change subject: [java client] Implement RPC tracing, part 2
..


[java client] Implement RPC tracing, part 2

This patch adds pretty printing for the traces, and makes them always part
of KuduRPC.toString. We thus rely on the exceptions' messages to include
the RPC's string representation in order to propagate the traces back to
the user.

In the future we could entertain having the traces in some queryable fashion in
KuduException, either by embedding the KuduRpc or just the list of traces.

Here's a trace snippet where a tserver that had a leader tablet was killed and 
we're
spinning on finding the new leader:

[0ms] sending RPC to server 5df1a37bd7e14380b5a8fceeba6b0d07
[70ms] received from server 5df1a37bd7e14380b5a8fceeba6b0d07 response Network 
error: [Peer 5df1a37bd7e14380b5a8fceeba6b0d07] Connection reset
[70ms] delaying RPC due to Network error: [Peer 
5df1a37bd7e14380b5a8fceeba6b0d07] Connection reset
[101ms] querying master
[101ms] Sub rpc: GetTableLocations sending RPC to server 
e6e499debd804a8183b85d20a33ad560
[104ms] Sub rpc: GetTableLocations sending RPC to server 
e6e499debd804a8183b85d20a33ad560
[104ms] Sub rpc: GetTableLocations received from server 
e6e499debd804a8183b85d20a33ad560 response OK
[109ms] sending RPC to server 5df1a37bd7e14380b5a8fceeba6b0d07
[110ms] received from server 5df1a37bd7e14380b5a8fceeba6b0d07 response Network 
error: [Peer 5df1a37bd7e14380b5a8fceeba6b0d07] Connection reset

Change-Id: I2c37143a587971f0e8985c59f4ab1d0c164c3723
Reviewed-on: http://gerrit.cloudera.org:8080/4950
Reviewed-by: Adar Dembo 
Tested-by: Kudu Jenkins
---
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RpcTraceFrame.java
2 files changed, 68 insertions(+), 6 deletions(-)

Approvals:
  Adar Dembo: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I2c37143a587971f0e8985c59f4ab1d0c164c3723
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] ssl: switch to older APIs for initializing SSL

Dan Burkert has posted comments on this change.

Change subject: ssl: switch to older APIs for initializing SSL
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I08fd5c3f6f8d2c228f760604dcecd7f1439578fb
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: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Fix broken GFlags documentation link

Todd Lipcon has submitted this change and it was merged.

Change subject: Fix broken GFlags documentation link
..


Fix broken GFlags documentation link

Change-Id: Ib67847245608bfe2b8bd827c34aa27ae051d97a3
Reviewed-on: http://gerrit.cloudera.org:8080/4955
Reviewed-by: Todd Lipcon 
Tested-by: Kudu Jenkins
---
M docs/configuration.adoc
1 file changed, 1 insertion(+), 1 deletion(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ib67847245608bfe2b8bd827c34aa27ae051d97a3
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] rpc: move connection header send/receive into negotiation.cc

Hello Dan Burkert, Sailesh Mukil, Alexey Serbin,

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

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

to review the following change.

Change subject: rpc: move connection header send/receive into negotiation.cc
..

rpc: move connection header send/receive into negotiation.cc

These methods were being called as part of the SASL negotiation code,
but really have nothing to do with SASL. This patch refactors them into
negotiation.cc.

This is some clean-up in preparation for making SSL support negotiated
during connection establishment ("StartTLS" style) rather than
pre-determined as on or off prior to creating the connection.

This doesn't affect the wire protocol at all: I verified this by
connecting to an existing cluster from a patched client.

Change-Id: I78a9b592b8cc139cc6db50cf9d0b48dc272d779a
---
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/sasl_helper.cc
M src/kudu/rpc/sasl_helper.h
M src/kudu/rpc/sasl_server.cc
M src/kudu/rpc/sasl_server.h
5 files changed, 24 insertions(+), 34 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I78a9b592b8cc139cc6db50cf9d0b48dc272d779a
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Sailesh Mukil

[kudu-CR] ssl: switch to older APIs for initializing SSL

Hello Dan Burkert, Sailesh Mukil, Alexey Serbin, Kudu Jenkins,

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

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

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

Change subject: ssl: switch to older APIs for initializing SSL
..

ssl: switch to older APIs for initializing SSL

This enables support for OpenSSL 1.0.0 as found on RHEL 6.4.

Change-Id: I08fd5c3f6f8d2c228f760604dcecd7f1439578fb
---
M src/kudu/util/net/ssl_factory.cc
1 file changed, 12 insertions(+), 4 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I08fd5c3f6f8d2c228f760604dcecd7f1439578fb
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: Sailesh Mukil

[kudu-CR] [java client] Redirect KuduExceptions to RowError in KuduSession

Jean-Daniel Cryans has posted comments on this change.

Change subject: [java client] Redirect KuduExceptions to RowError in KuduSession
..


Patch Set 3:

> I'll defer the +2 to Alexey; he's most familiar with the equivalent
 > semantics in the C++ client and should compare them.
 > 
 > Also, perhaps you could update the release notes in this patch?

Yeah, need to do that, but I'll do it in a separate patch unless Alexey has me 
fix more things here.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie871cde658036d04b9c07a3efe2fdfb4a7e98273
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] [java client] Redirect KuduExceptions to RowError in KuduSession

Adar Dembo has posted comments on this change.

Change subject: [java client] Redirect KuduExceptions to RowError in KuduSession
..


Patch Set 3: Code-Review+1

I'll defer the +2 to Alexey; he's most familiar with the equivalent semantics 
in the C++ client and should compare them.

Also, perhaps you could update the release notes in this patch?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie871cde658036d04b9c07a3efe2fdfb4a7e98273
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] [java client] Implement RPC tracing, part 2

Adar Dembo has posted comments on this change.

Change subject: [java client] Implement RPC tracing, part 2
..


Patch Set 4: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2c37143a587971f0e8985c59f4ab1d0c164c3723
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] ssl: switch to older APIs for initializing SSL

Alexey Serbin has posted comments on this change.

Change subject: ssl: switch to older APIs for initializing SSL
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4957/1/src/kudu/util/net/ssl_factory.cc
File src/kudu/util/net/ssl_factory.cc:

PS1, Line 84: ctx_ == nullptr
nit: I didn't notice that earlier, but while you are at it -- since ctx_ is a 
smart pointer, it's more idiomatic to write something like 

if (!ctx_) {
  ...
}

instead of if (ctx_ == nullptr)


PS1, Line 90: on early versions of RHEL6
Does it make sense to specify that it's 6.4 and earlier?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I08fd5c3f6f8d2c228f760604dcecd7f1439578fb
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: Sailesh Mukil 
Gerrit-HasComments: Yes

[kudu-CR] cmake: search for all required Kerberos binaries

Dan Burkert has submitted this change and it was merged.

Change subject: cmake: search for all required Kerberos binaries
..


cmake: search for all required Kerberos binaries

If one or more binaries are not found, CMake will issue a warning like:

-- Kerberos binary not found: security tests will fail (missing:  kdb5_util 
krb5kdc)

Change-Id: Ic680409088f487ee9fa28cbd7e8f2a95a034b4b2
Reviewed-on: http://gerrit.cloudera.org:8080/4954
Reviewed-by: Adar Dembo 
Tested-by: Kudu Jenkins
---
M CMakeLists.txt
D cmake_modules/FindKdc.cmake
A cmake_modules/FindKerberos.cmake
3 files changed, 40 insertions(+), 38 deletions(-)

Approvals:
  Adar Dembo: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ic680409088f487ee9fa28cbd7e8f2a95a034b4b2
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [java client] Implement RPC tracing, part 1

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

Change subject: [java client] Implement RPC tracing, part 1
..


[java client] Implement RPC tracing, part 1

First part of this work is adding the tracing objects and doing the tracing. A 
second
patch will make this information available to users.

This patch is using a pretty simple method of just
shoving container objects into a list, per RPC. The traces are lightweight
and don't try anything fancy. We also introduce the concept of "parent RPC", so 
that say
a Write RPC spawns a GetTableLocations, and the latter will be added to the 
former
so that the call to the master adds traces to both RPCs.

This patch isn't adding a nice way to present the traces (like JSON) but here's 
a simple
toString example:

RpcTraceFrame{rpcMethod='Write', timestampMs=1477079973547, 
action=SEND_TO_SERVER, server=3926a6a73e994152be1336beb434154e},
RpcTraceFrame{rpcMethod='Write', timestampMs=1477079973548, 
action=RECEIVE_FROM_SERVER, server=3926a6a73e994152be1336beb434154e, 
callStatus=Network error: [Peer 3926a6a73e994152be1336beb434154e] Connection 
reset on [id: 0xc83743df]}
RpcTraceFrame{rpcMethod='Write', timestampMs=1477079973548, 
action=SLEEP_THEN_RETRY, callStatus=Network error: [Peer 
3926a6a73e994152be1336beb434154e] Connection reset on [id: 0xc83743df]},
RpcTraceFrame{rpcMethod='Write', timestampMs=1477079973574, 
action=QUERY_MASTER},
RpcTraceFrame{rpcMethod='GetTableLocations', timestampMs=1477079973574, 
action=SEND_TO_SERVER, server=c0d4588690d241c69821ee773eebd185},
RpcTraceFrame{rpcMethod='GetTableLocations', timestampMs=1477079973576, 
action=RECEIVE_FROM_SERVER, server=c0d4588690d241c69821ee773eebd185, 
callStatus=OK},
RpcTraceFrame{rpcMethod='Write', timestampMs=1477079973579, 
action=PICKED_REPLICA},
RpcTraceFrame{rpcMethod='Write', timestampMs=1477079973579, 
action=SEND_TO_SERVER, server=0353a6d97d6c49f9a727bc1ee6c3393e},

This patch also fixes up some paths where we weren't passing a timeout 
correctly to an
RPC that was created in relation to another RPC (basically paths where the 
parent RPC
had to be set).

Change-Id: I69ef56acc071b9f80b34e38c1821df4096f54907
Reviewed-on: http://gerrit.cloudera.org:8080/4781
Reviewed-by: Dan Burkert 
Tested-by: Kudu Jenkins
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java
A java/kudu-client/src/main/java/org/apache/kudu/client/RpcTraceFrame.java
M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java
A java/kudu-client/src/test/java/org/apache/kudu/client/TestRpcTraces.java
8 files changed, 387 insertions(+), 35 deletions(-)

Approvals:
  Dan Burkert: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I69ef56acc071b9f80b34e38c1821df4096f54907
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] [rpc] Close socket on non-linux platforms during RPC shutdown

Hello Mike Percy, Adar Dembo, Todd Lipcon,

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

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

to review the following change.

Change subject: [rpc] Close socket on non-linux platforms during RPC shutdown
..

[rpc] Close socket on non-linux platforms during RPC shutdown

While using a MiniTabletServer::Restart() functionality, it was observed
that tablet server was not able to bind to same socket again on OS X.
It turns out non-linux platforms can leave the stale sockets behind
when we shutdown the RPC server. While shutdown() is not available on
platforms other than linux, we should be able to close() the socket on
other platforms to make sure stale fds are not left behind.

NOTE: This change is expected to impact non-linux platforms only.

Traces from the log which originally observed the issue:

W1104 14:58:36.040030 1953316864 net_util.cc:293] Failed to bind to 
0.0.0.0:56021. Trying to use lsof to find any processes listening on the same 
port:
I1104 14:58:36.040457 1953316864 net_util.cc:296] $ lsof -n -i 'TCP:56021' 
-sTCP:LISTEN ; for pid in $(lsof -F p -n -i 'TCP:56021' -sTCP:LISTEN | cut -f 2 
-dp) ; do  pstree $pid || ps h -p $pid;done
W1104 14:58:36.147557 1953316864 net_util.cc:303] COMMANDPID   USER   FD   
TYPE DEVICE SIZE/OFF NODE NAME
rpc-test 38600 dinesh8u  IPv4 0xcf92530d59e2332d  0t0  TCP *:56021 
(LISTEN)
-+= 38600 dinesh ./bin/rpc-test 
--gtest_filter=TestRpc.TestAcceptorPoolSocketShutDown
 \-+- 38601 dinesh bash -c lsof -n -i 'TCP:56021' -sTCP:LISTEN ; for pid in 
$(lsof -F p -n -i 'TCP:56021' -sTCP:LISTEN | cut -f 2 -dp) ; do  pstree $pid || 
ps h -p $pid;done
\-+- 38608 dinesh pstree 38600
 \--- 38609 root ps -axwwo user,pid,ppid,pgid,command
 ../../src/kudu/rpc/rpc-test.cc:106: Failure
 Failed
 Bad status: Network error: error binding socket to 0.0.0.0:56021: 
Address already in use (error 48)

Change-Id: I1ed12bd5e4be5ee932196e77ce2f0ecd3810c992
---
M src/kudu/rpc/acceptor_pool.cc
M src/kudu/rpc/rpc-test.cc
2 files changed, 21 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I1ed12bd5e4be5ee932196e77ce2f0ecd3810c992
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] ssl: switch to older APIs for initializing SSL

2016-11-04 Thread Sailesh Mukil (Code Review)

Sailesh Mukil has posted comments on this change.

Change subject: ssl: switch to older APIs for initializing SSL
..


Patch Set 1:

(1 comment)

After reading the attached discussion, LGTM.

http://gerrit.cloudera.org:8080/#/c/4957/1/src/kudu/util/net/ssl_factory.cc
File src/kudu/util/net/ssl_factory.cc:

PS1, Line 89: SSL v2
nit: no space


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I08fd5c3f6f8d2c228f760604dcecd7f1439578fb
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: Sailesh Mukil 
Gerrit-HasComments: Yes

[kudu-CR] ssl: switch to older APIs for initializing SSL

Hello Dan Burkert, Sailesh Mukil, Alexey Serbin,

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

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

to review the following change.

Change subject: ssl: switch to older APIs for initializing SSL
..

ssl: switch to older APIs for initializing SSL

This enables support for OpenSSL 1.0.0 as found on RHEL 6.4.

Change-Id: I08fd5c3f6f8d2c228f760604dcecd7f1439578fb
---
M src/kudu/util/net/ssl_factory.cc
1 file changed, 11 insertions(+), 3 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I08fd5c3f6f8d2c228f760604dcecd7f1439578fb
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Sailesh Mukil

[kudu-CR] Fix broken GFlags documentation link

Todd Lipcon has posted comments on this change.

Change subject: Fix broken GFlags documentation link
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib67847245608bfe2b8bd827c34aa27ae051d97a3
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] cmake: search for all required Kerberos binaries

Adar Dembo has posted comments on this change.

Change subject: cmake: search for all required Kerberos binaries
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic680409088f487ee9fa28cbd7e8f2a95a034b4b2
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] [java client] Implement RPC tracing, part 1

Dan Burkert has posted comments on this change.

Change subject: [java client] Implement RPC tracing, part 1
..


Patch Set 9: Code-Review+2

thanks!

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I69ef56acc071b9f80b34e38c1821df4096f54907
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Avoid extra fsyncs of tombstoned tablets during startup

Todd Lipcon has posted comments on this change.

Change subject: Avoid extra fsyncs of tombstoned tablets during startup
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4915/3/src/kudu/tablet/tablet_metadata.cc
File src/kudu/tablet/tablet_metadata.cc:

Line 173:   return Status::OK();
> I wonder if we can narrow the 'skip-fsync' to tombstoned tablet bootstrap s
OK, I reorganized the code a bit in the new revision of the patch: now we check 
whether the tombstoning is a 'no-op' at a higher level, with the plus that we 
no longer log a WARN message per tombstoned tablet during startup.

Not entirely following what you mean by the bit about the superblock with no 
log -- that's only the case for a tombstoned tablet, right? What's the case 
you're talking about? We write the metadata to tombstoned before deleting the 
log/meta so I'm not sure how you could have a non-deleted metadata state with 
missing log.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I60cb184b8de2a6a381371ddcf2fb938a19757eec
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] cmake: search for all required Kerberos binaries

Hello Adar Dembo,

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

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

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

Change subject: cmake: search for all required Kerberos binaries
..

cmake: search for all required Kerberos binaries

If one or more binaries are not found, CMake will issue a warning like:

-- Kerberos binary not found: security tests will fail (missing:  kdb5_util 
krb5kdc)

Change-Id: Ic680409088f487ee9fa28cbd7e8f2a95a034b4b2
---
M CMakeLists.txt
D cmake_modules/FindKdc.cmake
A cmake_modules/FindKerberos.cmake
3 files changed, 40 insertions(+), 38 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic680409088f487ee9fa28cbd7e8f2a95a034b4b2
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [java client] Implement RPC tracing, part 1

Hello Adar Dembo, Alexey Serbin, Kudu Jenkins,

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

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

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

Change subject: [java client] Implement RPC tracing, part 1
..

[java client] Implement RPC tracing, part 1

First part of this work is adding the tracing objects and doing the tracing. A 
second
patch will make this information available to users.

This patch is using a pretty simple method of just
shoving container objects into a list, per RPC. The traces are lightweight
and don't try anything fancy. We also introduce the concept of "parent RPC", so 
that say
a Write RPC spawns a GetTableLocations, and the latter will be added to the 
former
so that the call to the master adds traces to both RPCs.

This patch isn't adding a nice way to present the traces (like JSON) but here's 
a simple
toString example:

RpcTraceFrame{rpcMethod='Write', timestampMs=1477079973547, 
action=SEND_TO_SERVER, server=3926a6a73e994152be1336beb434154e},
RpcTraceFrame{rpcMethod='Write', timestampMs=1477079973548, 
action=RECEIVE_FROM_SERVER, server=3926a6a73e994152be1336beb434154e, 
callStatus=Network error: [Peer 3926a6a73e994152be1336beb434154e] Connection 
reset on [id: 0xc83743df]}
RpcTraceFrame{rpcMethod='Write', timestampMs=1477079973548, 
action=SLEEP_THEN_RETRY, callStatus=Network error: [Peer 
3926a6a73e994152be1336beb434154e] Connection reset on [id: 0xc83743df]},
RpcTraceFrame{rpcMethod='Write', timestampMs=1477079973574, 
action=QUERY_MASTER},
RpcTraceFrame{rpcMethod='GetTableLocations', timestampMs=1477079973574, 
action=SEND_TO_SERVER, server=c0d4588690d241c69821ee773eebd185},
RpcTraceFrame{rpcMethod='GetTableLocations', timestampMs=1477079973576, 
action=RECEIVE_FROM_SERVER, server=c0d4588690d241c69821ee773eebd185, 
callStatus=OK},
RpcTraceFrame{rpcMethod='Write', timestampMs=1477079973579, 
action=PICKED_REPLICA},
RpcTraceFrame{rpcMethod='Write', timestampMs=1477079973579, 
action=SEND_TO_SERVER, server=0353a6d97d6c49f9a727bc1ee6c3393e},

This patch also fixes up some paths where we weren't passing a timeout 
correctly to an
RPC that was created in relation to another RPC (basically paths where the 
parent RPC
had to be set).

Change-Id: I69ef56acc071b9f80b34e38c1821df4096f54907
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java
A java/kudu-client/src/main/java/org/apache/kudu/client/RpcTraceFrame.java
M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java
A java/kudu-client/src/test/java/org/apache/kudu/client/TestRpcTraces.java
8 files changed, 387 insertions(+), 35 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I69ef56acc071b9f80b34e38c1821df4096f54907
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] Fix broken GFlags documentation link

2016-11-04 Thread Sailesh Mukil (Code Review)

Sailesh Mukil has uploaded a new change for review.

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

Change subject: Fix broken GFlags documentation link
..

Fix broken GFlags documentation link

Change-Id: Ib67847245608bfe2b8bd827c34aa27ae051d97a3
---
M docs/configuration.adoc
1 file changed, 1 insertion(+), 1 deletion(-)


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

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

[kudu-CR] [java client] Redirect KuduExceptions to RowError in KuduSession

Hello Adar Dembo, Alexey Serbin, Kudu Jenkins,

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

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

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

Change subject: [java client] Redirect KuduExceptions to RowError in KuduSession
..

[java client] Redirect KuduExceptions to RowError in KuduSession

Currently the only RowErrors that the users were going to see were the ones
sent from the tablet servers. Any other client-side error was sent as an
exception, even timeouts. The other problem with timeouts is that, when using
AUTO_FLUSH_BACKGROUND, background flush responses that the client isn't waiting
on would get lost since we don't have an equivalent to the error collector
for exceptions. You could find them in the log, but that's it.

This patch augments one of the errbacks in AsyncKuduSession to start creating
OperationResponses for KuduExceptions. For those cases, we return a
List which switches us from the _errback_ to the _callback_
track. Other exceptions are sent straight back.

This patch also makes some changes for an issue that Adar saw in 
TestAsyncKuduSession
that was kind of hard to debug, a stray buffer was being flushed against a 
table that
was deleted (but then the table's name was reused which made reading the logs 
harder).

The stray buffer most likely came from testBatchErrorCauseSessionStuck which 
isn't
checking for all the error conditions. This patch adds more checking but 
doesn't fix
the root cause (which is currently unknown) so it *may* make it fail more often,
but in more obvious ways.

Change-Id: Ie871cde658036d04b9c07a3efe2fdfb4a7e98273
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java
2 files changed, 92 insertions(+), 33 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie871cde658036d04b9c07a3efe2fdfb4a7e98273
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [java client] Implement RPC tracing, part 2

Adar Dembo has posted comments on this change.

Change subject: [java client] Implement RPC tracing, part 2
..


Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2c37143a587971f0e8985c59f4ab1d0c164c3723
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] cmake: search for all required Kerberos binaries

Hello Adar Dembo,

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

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

to review the following change.

Change subject: cmake: search for all required Kerberos binaries
..

cmake: search for all required Kerberos binaries

Change-Id: Ic680409088f487ee9fa28cbd7e8f2a95a034b4b2
---
M CMakeLists.txt
D cmake_modules/FindKdc.cmake
A cmake_modules/FindKerberos.cmake
3 files changed, 41 insertions(+), 38 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic680409088f487ee9fa28cbd7e8f2a95a034b4b2
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo

[kudu-CR] [java client] Implement RPC tracing, part 1

Hello Adar Dembo, Alexey Serbin, Kudu Jenkins,

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

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

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

Change subject: [java client] Implement RPC tracing, part 1
..

[java client] Implement RPC tracing, part 1

First part of this work is adding the tracing objects and doing the tracing. A 
second
patch will make this information available to users.

This patch is using a pretty simple method of just
shoving container objects into a list, per RPC. The traces are lightweight
and don't try anything fancy. We also introduce the concept of "parent RPC", so 
that say
a Write RPC spawns a GetTableLocations, and the latter will be added to the 
former
so that the call to the master adds traces to both RPCs.

This patch isn't adding a nice way to present the traces (like JSON) but here's 
a simple
toString example:

RpcTraceFrame{rpcMethod='Write', timestampMs=1477079973547, 
action=SEND_TO_SERVER, server=3926a6a73e994152be1336beb434154e},
RpcTraceFrame{rpcMethod='Write', timestampMs=1477079973548, 
action=RECEIVE_FROM_SERVER, server=3926a6a73e994152be1336beb434154e, 
callStatus=Network error: [Peer 3926a6a73e994152be1336beb434154e] Connection 
reset on [id: 0xc83743df]}
RpcTraceFrame{rpcMethod='Write', timestampMs=1477079973548, 
action=SLEEP_THEN_RETRY, callStatus=Network error: [Peer 
3926a6a73e994152be1336beb434154e] Connection reset on [id: 0xc83743df]},
RpcTraceFrame{rpcMethod='Write', timestampMs=1477079973574, 
action=QUERY_MASTER},
RpcTraceFrame{rpcMethod='GetTableLocations', timestampMs=1477079973574, 
action=SEND_TO_SERVER, server=c0d4588690d241c69821ee773eebd185},
RpcTraceFrame{rpcMethod='GetTableLocations', timestampMs=1477079973576, 
action=RECEIVE_FROM_SERVER, server=c0d4588690d241c69821ee773eebd185, 
callStatus=OK},
RpcTraceFrame{rpcMethod='Write', timestampMs=1477079973579, 
action=PICKED_REPLICA},
RpcTraceFrame{rpcMethod='Write', timestampMs=1477079973579, 
action=SEND_TO_SERVER, server=0353a6d97d6c49f9a727bc1ee6c3393e},

This patch also fixes up some paths where we weren't passing a timeout 
correctly to an
RPC that was created in relation to another RPC (basically paths where the 
parent RPC
had to be set).

Change-Id: I69ef56acc071b9f80b34e38c1821df4096f54907
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java
A java/kudu-client/src/main/java/org/apache/kudu/client/RpcTraceFrame.java
M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java
A java/kudu-client/src/test/java/org/apache/kudu/client/TestRpcTraces.java
8 files changed, 387 insertions(+), 35 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I69ef56acc071b9f80b34e38c1821df4096f54907
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] [java client] Implement RPC tracing, part 2

Hello Adar Dembo, Alexey Serbin, Kudu Jenkins,

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

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

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

Change subject: [java client] Implement RPC tracing, part 2
..

[java client] Implement RPC tracing, part 2

This patch adds pretty printing for the traces, and makes them always part
of KuduRPC.toString. We thus rely on the exceptions' messages to include
the RPC's string representation in order to propagate the traces back to
the user.

In the future we could entertain having the traces in some queryable fashion in
KuduException, either by embedding the KuduRpc or just the list of traces.

Here's a trace snippet where a tserver that had a leader tablet was killed and 
we're
spinning on finding the new leader:

[0ms] sending RPC to server 5df1a37bd7e14380b5a8fceeba6b0d07
[70ms] received from server 5df1a37bd7e14380b5a8fceeba6b0d07 response Network 
error: [Peer 5df1a37bd7e14380b5a8fceeba6b0d07] Connection reset
[70ms] delaying RPC due to Network error: [Peer 
5df1a37bd7e14380b5a8fceeba6b0d07] Connection reset
[101ms] querying master
[101ms] Sub rpc: GetTableLocations sending RPC to server 
e6e499debd804a8183b85d20a33ad560
[104ms] Sub rpc: GetTableLocations sending RPC to server 
e6e499debd804a8183b85d20a33ad560
[104ms] Sub rpc: GetTableLocations received from server 
e6e499debd804a8183b85d20a33ad560 response OK
[109ms] sending RPC to server 5df1a37bd7e14380b5a8fceeba6b0d07
[110ms] received from server 5df1a37bd7e14380b5a8fceeba6b0d07 response Network 
error: [Peer 5df1a37bd7e14380b5a8fceeba6b0d07] Connection reset

Change-Id: I2c37143a587971f0e8985c59f4ab1d0c164c3723
---
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RpcTraceFrame.java
2 files changed, 68 insertions(+), 6 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2c37143a587971f0e8985c59f4ab1d0c164c3723
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [java client] Implement RPC tracing, part 1

Dan Burkert has posted comments on this change.

Change subject: [java client] Implement RPC tracing, part 1
..


Patch Set 5:

(1 comment)

sorz

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

PS7, Line 27:  Action {
Could you rename this to RpcTraceFrame or something more specific than Object?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I69ef56acc071b9f80b34e38c1821df4096f54907
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Add KuduTable.getFormattedRangePartitions method

Jean-Daniel Cryans has posted comments on this change.

Change subject: Add KuduTable.getFormattedRangePartitions method
..


Patch Set 2:

(8 comments)

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

PS2, Line 268: public
Does this need to be public?


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

PS2, Line 208: the tables
nit: this table's


Line 223:   if (!Iterators.all(partition.getHashBuckets().iterator(), 
Predicates.equalTo(0))) continue;
nit: I personally prefer if statements to not be on a single line, especially 
if sandwiched like this, for better readability.


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

PS2, Line 579: appendCellValueDebugString
nit: might as well put that in the enum itself.


PS2, Line 785: Returns {@code true} if the cell values for the given column are 
equal in the two rows.
nit: that text should be in @return. Write something here that's like "Checks 
if the specified cell is equal in both rows" or something similar.


PS2, Line 802: switch (type) {
Guess you could move that into the enum as well.


PS2, Line 839: Returns
Same comment as above.


Line 858: switch (type) {
Same comment as above.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9b263d2444314d46533191918833840e75b7ba7
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1735. Fix crash when aborting a skipped config change round

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1735. Fix crash when aborting a skipped config change round
..

KUDU-1735. Fix crash when aborting a skipped config change round

This fixes a crash seen in a test cluster when deleting a tablet
in which there is a pending (uncommitted) REPLICATE message for a
skipped CONFIG_CHANGE operation. The CONFIG_CHANGE was skipped because
the tablet already had a higher indexed configuration written to disk.

This can happen in a couple cases: one, exercised by a test included
here, is if a tablet server crashes just after committing a config
change to disk but before writing the COMMIT message to the WAL, then
upon restart that config change will be a pending operation despite being
committed. If the table is then deleted before the operation is
committed, it would crash on abort.

The approach taken to fix this is as follows:

When aborting a config change operation, we only clear the 'pending'
config if we see that its index matches the index of the operation being
aborted. Otherwise, we ignore the abort (whereas we used to crash).

In order to achieve this, we have to remember the index of the pending
config, which previously wasn't set until after the commit. So, this
assignment is moved out of the commit path and into
AddPendingOperationUnlocked(). Changing the assumption of where the
index was set involved making a few corresponding changes to DCHECKs
elsewhere in the code.

There is a slight wire/upgrade compatibility issue here: previously the
leader would replicate config change messages with no opid set in the
'new_config'. Now, it does. I think this would cause DCHECKs to fire in
a mixed-version cluster, though no CHECKs. Additionally, we aren't
purporting to fully support rolling upgrade yet, so I don't think it's a
big deal. I was able to upgrade the test cluster which experienced this
issue in-place and it came up fine.

This patch removes raft_consensus_state-test, which had some various
assertions against setting pending configuration changes with opids
set. After looking at this test, I realized that it's purporting to test
ReplicaState but in fact all of the methods that it tests are just
pass-throughs to ConsensusMeta and thus are already covered by
consensus_meta-test. So, I figured there's no sense spending time
updating this redundant test code.

I ran raft_consensus-itest 1000 times[1]and the new test alone another
1000 times each in DEBUG[2] and TSAN[3] to test.

[1] http://dist-test.cloudera.org/job?job_id=todd.1478141668.26073
[2] http://dist-test.cloudera.org/job?job_id=todd.1478291990.1887
[3] http://dist-test.cloudera.org/job?job_id=todd.1478292124.2771

Change-Id: I2b4e4eb1d60ef66527edf33357fabc22f4b5b32f
---
M src/kudu/consensus/CMakeLists.txt
M src/kudu/consensus/consensus_meta-test.cc
M src/kudu/consensus/quorum_util.cc
M src/kudu/consensus/raft_consensus.cc
D src/kudu/consensus/raft_consensus_state-test.cc
M src/kudu/consensus/raft_consensus_state.cc
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
8 files changed, 121 insertions(+), 175 deletions(-)


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

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

[kudu-CR] [java client] Implement RPC tracing, part 2

Hello Adar Dembo, Alexey Serbin, Kudu Jenkins,

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

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

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

Change subject: [java client] Implement RPC tracing, part 2
..

[java client] Implement RPC tracing, part 2

This patch adds pretty printing for the traces, and makes them always part
of KuduRPC.toString. We thus rely on the exceptions' messages to include
the RPC's string representation in order to propagate the traces back to
the user.

In the future we could entertain having the traces in some queryable fashion in
KuduException, either by embedding the KuduRpc or just the list of traces.

Here's a trace snippet where a tserver that had a leader tablet was killed and 
we're
spinning on finding the new leader:

[0ms] sending RPC to server 5df1a37bd7e14380b5a8fceeba6b0d07
[70ms] received from server 5df1a37bd7e14380b5a8fceeba6b0d07 response Network 
error: [Peer 5df1a37bd7e14380b5a8fceeba6b0d07] Connection reset
[70ms] delaying RPC due to Network error: [Peer 
5df1a37bd7e14380b5a8fceeba6b0d07] Connection reset
[101ms] querying master
[101ms] Sub rpc: GetTableLocations sending RPC to server 
e6e499debd804a8183b85d20a33ad560
[104ms] Sub rpc: GetTableLocations sending RPC to server 
e6e499debd804a8183b85d20a33ad560
[104ms] Sub rpc: GetTableLocations received from server 
e6e499debd804a8183b85d20a33ad560 response OK
[109ms] sending RPC to server 5df1a37bd7e14380b5a8fceeba6b0d07
[110ms] received from server 5df1a37bd7e14380b5a8fceeba6b0d07 response Network 
error: [Peer 5df1a37bd7e14380b5a8fceeba6b0d07] Connection reset

Change-Id: I2c37143a587971f0e8985c59f4ab1d0c164c3723
---
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RpcTraceObject.java
2 files changed, 68 insertions(+), 6 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2c37143a587971f0e8985c59f4ab1d0c164c3723
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [java client] Redirect KuduExceptions to RowError in KuduSession

Hello Adar Dembo, Alexey Serbin, Kudu Jenkins,

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

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

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

Change subject: [java client] Redirect KuduExceptions to RowError in KuduSession
..

[java client] Redirect KuduExceptions to RowError in KuduSession

Currently the only RowErrors that the users were going to see were the ones
sent from the tablet servers. Any other client-side error was sent as an
exception, even timeouts. The other problem with timeouts is that, when using
AUTO_FLUSH_BACKGROUND, background flush responses that the client isn't waiting
on would get lost since we don't have an equivalent to the error collector
for exceptions. You could find them in the log, but that's it.

This patch augments one of the errbacks in AsyncKuduSession to start creating
OperationResponses for KuduExceptions. For those cases, we return a
List which switches us from the _errback_ to the _callback_
track. Other exceptions are sent straight back.

This patch also makes some changes for an issue that Adar saw in 
TestAsyncKuduSession
that was kind of hard to debug, a stray buffer was being flushed against a 
table that
was deleted (but then the table's name was reused which made reading the logs 
harder).

The stray buffer most likely came from testBatchErrorCauseSessionStuck which 
isn't
checking for all the error conditions. This patch adds more checking but 
doesn't fix
the root cause (which is currently unknown) so it *may* make it more often,
but in more obvious ways.

Change-Id: Ie871cde658036d04b9c07a3efe2fdfb4a7e98273
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java
2 files changed, 92 insertions(+), 33 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie871cde658036d04b9c07a3efe2fdfb4a7e98273
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [java client] Redirect KuduExceptions to RowError in KuduSession

Jean-Daniel Cryans has posted comments on this change.

Change subject: [java client] Redirect KuduExceptions to RowError in KuduSession
..

Patch Set 1:

(4 comments)

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

PS1, Line 13: recipient
> What does "recipient" mean in this context? If you mean a destination tserv
A recipient for the flush's response. I'll reword.

Line 18: List Nit: missing a '>' here.
Done

PS1, Line 26: fixing
> So this patch fixes the test? Or makes it fail more often? I don't see how
Done

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

PS1, Line 664: // We send the callback second so that the error
collector's count becomes visible first,
: // since calling callback will wake up whoever is
waiting right away and having the error
: // missing in the collector could be confusing.
> Nit: makes sense, but can we reword for terseness? Maybe:
Done

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie871cde658036d04b9c07a3efe2fdfb4a7e98273
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] [tools] Manual recovery tools (part 1)

Dinesh Bhat has posted comments on this change.

Change subject: [tools] Manual recovery tools (part 1)
..


Patch Set 10:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4834/6/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

Line 1149
> Are you saying that committing this patch will break the macOS build? Seems
It doesn't fail the build, it fails the test on OS x only. Since my follow-up 
patch fixes it, I wouldn't see os x fix as a blocker to this, but I am fine 
submitting the os x fix first.


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

PS1, Line 276:   return Status::OK();
 : }
> I am a bit torn on this one.
Hmmm current notion is that we use 'local_replica delete' when we couldn't 
bring up tablet server due to one or more bad tablets. It makes sense to keep 
the bad tablet around with --archive option for debugging purposes later. At 
high level, I am still trying to think of a situation where we want to 
tombstone a bad tablet... today we bootstrap tombstoned tablet when we bring up 
the server right ? and chances are that we may still crash ? So, I am not sure 
how tombstoning would help here.

I think one important piece as Todd pointed out in recovery doc is to be able 
to exactly identify which tablet is causing the crash. Otherwise, we wouldn't 
know what tablet this tool is supposed to act on.

Having a tombstone option for a remote_replica (i.e, when destination tserver 
is running) may be useful. I will go down the path of last_logged id with 
'remote_replica tombstone' may-be ?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I113a25e9b6c14f7c3814140917b61e35030b58d0
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [java client] Implement RPC tracing, part 2

Adar Dembo has posted comments on this change.

Change subject: [java client] Implement RPC tracing, part 2
..


Patch Set 1:

(4 comments)

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

PS1, Line 10: We thus rely on having the exceptions' messages to have
: the RPC's string representation in them in order to have the 
traces make their
: way back to the user.
Nit: a little clunky ("have" or "having" three times), reword?


PS1, Line 15: be embedding the KuduRpc or just the list of traces.
Nit: "by embedding the KuduRpc or just its list of traces"


Line 23: [101ms] querying master, [101ms] Sub rpc: GetTableLocations sending 
RPC to server e6e499debd804a8183b85d20a33ad560
Why were these two entries ("querying master" and "Sub rpc: ...") on the same 
line?


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

Line 104: case SEND_TO_SERVER:
You can encapsulate these in AppendToStringBuilder(RpcTraceObject, 
StringBuilder) methods, one for each Action enum.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2c37143a587971f0e8985c59f4ab1d0c164c3723
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] [java client] Redirect KuduExceptions to RowError in KuduSession

Adar Dembo has posted comments on this change.

Change subject: [java client] Redirect KuduExceptions to RowError in KuduSession
..

Patch Set 1:

(5 comments)

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

PS1, Line 13: recipient
What does "recipient" mean in this context? If you mean a destination tserver,
every flush has one (or more) of those, so you must mean something else, right?

Line 18: List' here.

PS1, Line 26: make it
Nit: make it fail?

PS1, Line 26: fixing
So this patch fixes the test? Or makes it fail more often? I don't see how it'd
be possible to do both.

Maybe reword this paragraph to clarify.

PS1, Line 664: // We send the callback second so that the error
collector's count becomes visible first,
: // since calling callback will wake up whoever is
waiting right away and having the error
: // missing in the collector could be confusing.
Nit: makes sense, but can we reword for terseness? Maybe:

// Fire the callback after collecting the error so that the error is visible
should the callback interrogate the error collector.

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

[kudu-CR] [java client] Implement RPC tracing, part 1

Adar Dembo has posted comments on this change.

Change subject: [java client] Implement RPC tracing, part 1
..


Patch Set 6: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I69ef56acc071b9f80b34e38c1821df4096f54907
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] installation.adoc: add openssl requirement for macOS

Dan Burkert has posted comments on this change.

Change subject: installation.adoc: add openssl requirement for macOS
..


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/4952/1/docs/installation.adoc
File docs/installation.adoc:

Line 512: [[osx_from_source]]
> This should be updated, or removed entirely if there's no link to it elsewh
I'm going to leave this, in case there are outstanding links.


Line 579: .OSX Build Script
> macOS Build Script?
Done


PS1, Line 581: OSX
> macOS
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6f68aeecd0194da645269bb232cc0b96d6bb5cc7
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] installation.adoc: add openssl requirement for macOS

Hello Kudu Jenkins,

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

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

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

Change subject: installation.adoc: add openssl requirement for macOS
..

installation.adoc: add openssl requirement for macOS

Change-Id: I6f68aeecd0194da645269bb232cc0b96d6bb5cc7
---
M docs/installation.adoc
1 file changed, 18 insertions(+), 13 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6f68aeecd0194da645269bb232cc0b96d6bb5cc7
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] block manager: better preallocation in log block manager

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

Change subject: block_manager: better preallocation in log block manager
..


block_manager: better preallocation in log block manager

As has been discussed to death, the LBM's preallocation strategy isn't great
because it yields one filesystem extent per created block. While multiple
Append() calls will write to the same extent, small blocks (i.e. those with
just a single Append()) suffer. We can do much better, and in this patch, we
do: a rolling preallocation window is maintained for each container.

Implementation-wise, I did what I could to restrict this knowledge to the
containers. But it did mean a small semantic change to UpdateBytesWritten()
to accomodate for the fact that Append() now updates total_bytes_written_.

As a quick test, I ran block_manager_stress-test before and after, using
filefrag to count the number of extents. The number of containers was the
same in both cases (64). Before, I counted 446 extents totaling 2.1Gb and
after I counted 72 extents totaling 1.4Gb.

Change-Id: Ie7dcba01aff125a68817ca91b3c306977c7104e6
Reviewed-on: http://gerrit.cloudera.org:8080/4848
Tested-by: Adar Dembo 
Reviewed-by: Jean-Daniel Cryans 
---
M src/kudu/fs/block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
2 files changed, 124 insertions(+), 63 deletions(-)

Approvals:
  Jean-Daniel Cryans: Looks good to me, approved
  Adar Dembo: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ie7dcba01aff125a68817ca91b3c306977c7104e6
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] block manager: better preallocation in log block manager

Jean-Daniel Cryans has posted comments on this change.

Change subject: block_manager: better preallocation in log block manager
..


Patch Set 4: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7dcba01aff125a68817ca91b3c306977c7104e6
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] [java client] Cleanup "Connection reset" message in TabletClient

Adar Dembo has submitted this change and it was merged.

Change subject: [java client] Cleanup "Connection reset" message in TabletClient
..


[java client] Cleanup "Connection reset" message in TabletClient

It was adding a Channel.toString() which has never been useful, removing.

Change-Id: I4c47df9dbd7183a328fe15cabc2e7a137c6d2184
Reviewed-on: http://gerrit.cloudera.org:8080/4947
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo 
---
M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java
1 file changed, 2 insertions(+), 2 deletions(-)

Approvals:
  Adar Dembo: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I4c47df9dbd7183a328fe15cabc2e7a137c6d2184
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [java client] Cleanup "Connection reset" message in TabletClient

Adar Dembo has posted comments on this change.

Change subject: [java client] Cleanup "Connection reset" message in TabletClient
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4c47df9dbd7183a328fe15cabc2e7a137c6d2184
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] installation.adoc: add openssl requirement for macOS

Adar Dembo has posted comments on this change.

Change subject: installation.adoc: add openssl requirement for macOS
..


Patch Set 1:

(3 comments)

We also need to doc the openssl requirement on Linux. Do you want to do that 
here, or later?

http://gerrit.cloudera.org:8080/#/c/4952/1/docs/installation.adoc
File docs/installation.adoc:

Line 512: [[osx_from_source]]
This should be updated, or removed entirely if there's no link to it elsewhere.


Line 579: .OSX Build Script
macOS Build Script?


PS1, Line 581: OSX
macOS


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6f68aeecd0194da645269bb232cc0b96d6bb5cc7
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] installation.adoc: add openssl requirement for macOS

Hello Adar Dembo, Alexey Serbin,

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

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

to review the following change.

Change subject: installation.adoc: add openssl requirement for macOS
..

installation.adoc: add openssl requirement for macOS

Change-Id: I6f68aeecd0194da645269bb232cc0b96d6bb5cc7
---
M docs/installation.adoc
1 file changed, 16 insertions(+), 11 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I6f68aeecd0194da645269bb232cc0b96d6bb5cc7
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin

[kudu-CR] [tools] Manual recovery tools (part 1)

Mike Percy has posted comments on this change.

Change subject: [tools] Manual recovery tools (part 1)
..


Patch Set 1:

(1 comment)

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

PS1, Line 276:   RETURN_NOT_OK(TSTabletManager::DeleteTabletData(
 :   meta, TabletDataState::TABLET_DATA_DELETED, boost::none));
> Yeah sure, my first approach was to provide last_logged_opid, but I think t
I am a bit torn on this one.

On the one hand, I think tombstoning the replica by default would be much 
better, while also providing an option to delete it. Deletion is not generally 
safe and should only happen in an emergency scenario or when the table itself 
is being deleted.

However, in order to properly tombstone a tablet, we should pass in the 
last_logged_opid. That comes from the WAL. I'm not sure how easy that is to do 
that without starting up lots of other components.

I think it's worth attempting to get the last logged opid by spinning up a 
LogReader, and if it's actually not that difficult then we should do it. If 
it's very complicated then I suppose we can just rely on the remote replica 
tool for tombstoning and just use this one for local deletion.


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

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

[kudu-CR] [tools] Manual recovery tools (part 1)

Mike Percy has posted comments on this change.

Change subject: [tools] Manual recovery tools (part 1)
..


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4834/6/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

Line 1149: }
> Yeah, I root-caused it to be a missing piece in socket teardown path for ma
Are you saying that committing this patch will break the macOS build? Seems 
like we need to wait for that other patch to happen first if that is the case.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I113a25e9b6c14f7c3814140917b61e35030b58d0
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [tools] Manual recovery tools (part 1)

Mike Percy has posted comments on this change.

Change subject: [tools] Manual recovery tools (part 1)
..


Patch Set 10:

(2 comments)

> Would you mind also adding a test that makes sure that attempting
> to copy over an existing (running) tablet, even in --force mode,
> fails?

Forget this comment, after looking at the code it would not fail, it would 
actually tombstone the existing one and then overwrite it.

http://gerrit.cloudera.org:8080/#/c/4834/10/src/kudu/tools/tool_action_remote_replica.cc
File src/kudu/tools/tool_action_remote_replica.cc:

Line 52: DEFINE_bool(force_copy, false,
Hmm, can we just make this --force instead of --force_copy ? --force seems a 
lot easier to remember.


Line 320:   }
I think by default we should pass in the source server's term. This can be 
obtained via the consensus_proxy->GetConsensusState() call.

If we don't do this, people will have to just use --force all of the time which 
seems silly.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I113a25e9b6c14f7c3814140917b61e35030b58d0
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [tools] Manual recovery tools (part 1)

Dinesh Bhat has posted comments on this change.

Change subject: [tools] Manual recovery tools (part 1)
..


Patch Set 10:

(2 comments)

> (2 comments)
 > 
 > Would you mind also adding a test that makes sure that attempting
 > to copy over an existing (running) tablet, even in --force mode,
 > fails?
 
BTW, this doesn't fail if I am copying over an existing healthy replica with 
--force, because we set the caller id to INT_MAX from the tool.

 > Also, I wonder what the value of the --force flag is, since this is
 > a manual tool maybe we should always force. This seems like a tool
 > that you would only typically use --force with, so why bother
 > requiring people to specify it.
Precisely the above case, we don't want user to accidentally copy on an 
existing healthy replica, but if he wants to, we can override using force 
option.

http://gerrit.cloudera.org:8080/#/c/4834/6/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

Line 1149
> It's bad that they have different semantics, because they implement the sam
Yeah, I root-caused it to be a missing piece in socket teardown path for macOS, 
but I am gonna fix that in a different patch.


http://gerrit.cloudera.org:8080/#/c/4834/9/src/kudu/tools/tool_action_remote_replica.cc
File src/kudu/tools/tool_action_remote_replica.cc:

Line 331: strings::Substitute("Remote server returned error code $0",
> it's generated and placed in build/latest/src/kudu/tserver/tserver.pb.cc
Yeah, I was more curious about what makes this Enum special ? It turns out the 
protobuf always generate one such routines for Enum types, that's cool :)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I113a25e9b6c14f7c3814140917b61e35030b58d0
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Reject CREATE TABLE ops with even replication factor

2016-11-04 Thread Hao Hao (Code Review)

Hao Hao has posted comments on this change.

Change subject: Reject CREATE TABLE ops with even replication factor
..


Patch Set 2:

Working on the fix..

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1674cc59cdfc2955d42fc5e4d8c0d962d9cc8e8e
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] [java client] Implement RPC tracing, part 2

Hello Adar Dembo, Alexey Serbin,

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

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

to review the following change.

Change subject: [java client] Implement RPC tracing, part 2
..

[java client] Implement RPC tracing, part 2

This patch adds pretty printing for the traces, and makes them always part
of KuduRPC.toString. We thus rely on having the exceptions' messages to have
the RPC's string representation in them in order to have the traces make their
way back to the user.

In the future we could entertain having the traces in some queryable fashion in
KuduException, either be embedding the KuduRpc or just the list of traces.

Here's a trace snippet where a tserver that had a leader tablet was killed and 
we're
spinning on finding the new leader:

[0ms] sending RPC to server 5df1a37bd7e14380b5a8fceeba6b0d07
[70ms] received from server 5df1a37bd7e14380b5a8fceeba6b0d07 response Network 
error: [Peer 5df1a37bd7e14380b5a8fceeba6b0d07] Connection reset
[70ms] delaying RPC due to Network error: [Peer 
5df1a37bd7e14380b5a8fceeba6b0d07] Connection reset
[101ms] querying master, [101ms] Sub rpc: GetTableLocations sending RPC to 
server e6e499debd804a8183b85d20a33ad560
[104ms] Sub rpc: GetTableLocations sending RPC to server 
e6e499debd804a8183b85d20a33ad560
[104ms] Sub rpc: GetTableLocations received from server 
e6e499debd804a8183b85d20a33ad560 response OK
[109ms] sending RPC to server 5df1a37bd7e14380b5a8fceeba6b0d07
[110ms] received from server 5df1a37bd7e14380b5a8fceeba6b0d07 response Network 
error: [Peer 5df1a37bd7e14380b5a8fceeba6b0d07] Connection reset

Change-Id: I2c37143a587971f0e8985c59f4ab1d0c164c3723
---
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RpcTraceObject.java
2 files changed, 59 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I2c37143a587971f0e8985c59f4ab1d0c164c3723
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin

[kudu-CR] [java client] Redirect KuduExceptions to RowError in KuduSession

Hello Adar Dembo, Alexey Serbin,

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

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

to review the following change.

Change subject: [java client] Redirect KuduExceptions to RowError in KuduSession
..

[java client] Redirect KuduExceptions to RowError in KuduSession

Currently the only RowErrors that the users were going to see were the ones
sent from the tablet servers. Any other client-side error was sent as an
exception, even timeouts. The other problem with timeouts is that, when using
AUTO_FLUSH_BACKGROUND, background flushes that the client isn't waiting on
would get lost since they don't have a recipient. You could find them in the
log, but that's it.

This patch augments one of the errbacks in AsyncKuduSession to start creating
OperationResponses for KuduExceptions. For those cases, we return a
Listhttp://gerrit.cloudera.org:8080/4949
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie871cde658036d04b9c07a3efe2fdfb4a7e98273
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin

[kudu-CR] [java client] Cleanup "Connection reset" message in TabletClient

Hello Adar Dembo, Alexey Serbin,

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

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

to review the following change.

Change subject: [java client] Cleanup "Connection reset" message in TabletClient
..

[java client] Cleanup "Connection reset" message in TabletClient

It was adding a Channel.toString() which has never been useful, removing.

Change-Id: I4c47df9dbd7183a328fe15cabc2e7a137c6d2184
---
M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java
1 file changed, 2 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I4c47df9dbd7183a328fe15cabc2e7a137c6d2184
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin

[kudu-CR] [tools] Manual recovery tools (part 1)

Mike Percy has posted comments on this change.

Change subject: [tools] Manual recovery tools (part 1)
..


Patch Set 10:

> maybe we should always force

Actually, what I think we should do is attempt to send the actual 
committed_opid_index with the default request... let me figure out how we can 
do that.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I113a25e9b6c14f7c3814140917b61e35030b58d0
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Reject CREATE TABLE ops with even replication factor

Jean-Daniel Cryans has posted comments on this change.

Change subject: Reject CREATE TABLE ops with even replication factor
..


Patch Set 2:

> Build Failed
 > 
 > http://104.196.14.100/job/kudu-gerrit/4280/ : FAILURE

Looks like there's a few unit tests to fix :)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1674cc59cdfc2955d42fc5e4d8c0d962d9cc8e8e
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] block manager: disk space checking everywhere

Mike Percy has posted comments on this change.

Change subject: block_manager: disk space checking everywhere
..


Patch Set 5:

Apparently needs a rebase, please feel free to carry my +2 through

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

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

[kudu-CR] block manager: various changes to disk space reservation checking

Mike Percy has submitted this change and it was merged.

Change subject: block_manager: various changes to disk space reservation 
checking
..


block_manager: various changes to disk space reservation checking

I was trying to move log block container preallocation out of CreateBlock(),
but it was very difficult to reason about its behavior due to how wrapped up
it was with disk space checking. So here's some more yak shaving.

First, disk space checking is now done in DataDirManager. I like this for
several reasons:
- DataDir already models a data directory; disk space checking just adds a
  couple fields.
- It feels natural for DataDirManager to incorporate the "ignore full data
  directories" behavior into GetNextDataDir().
- It makes disk space checking available to all block managers.

Second, its semantics are now slightly different. We no longer check if
we're about to exceed the reserved space; instead, we just check if we've
already exceeded it. This is simpler but it preserves the "soft" nature of
the reservation behavior. The main advantage is that we can now run the
check from anywhere instead of just before a file grows, which makes it
easier to reason about and reduces the coupling on preallocation.

Third, the published metric is now a function of data directories, not
containers. I don't think a container-based metric is particularly useful
here because technically all containers hosted on a data directory are
"unavailable" if the underlying filesystem is full; it's more relevant to
count the data directory itself.

Change-Id: Ica2bb710a246ef09890554d1d6ff6da528145a6e
Reviewed-on: http://gerrit.cloudera.org:8080/4831
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy 
---
M src/kudu/fs/block_manager-test.cc
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/file_block_manager.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/fs/log_block_manager.h
M src/kudu/integration-tests/disk_reservation-itest.cc
M src/kudu/util/metrics.h
8 files changed, 268 insertions(+), 238 deletions(-)

Approvals:
  Mike Percy: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ica2bb710a246ef09890554d1d6ff6da528145a6e
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] block manager: cosmetic changes to the LBM

Mike Percy has posted comments on this change.

Change subject: block_manager: cosmetic changes to the LBM
..


Patch Set 4: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib5990ac1946cb2aab331a9b618ba4710ccfdf3e3
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] block manager: various changes to disk space reservation checking