[kudu-CR] KUDU-2305: Limit sidecars to INT MAX and fortify socket code

[Joe McDonnell (Code Review)]

Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9601 )

Change subject: KUDU-2305: Limit sidecars to INT_MAX and fortify socket code
..


Patch Set 1:

There are probably some comments that need to be updated. 
src/kudu/rpc/rpc_sidecar.h might need to note the limit on sidecar size.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d041e214b15d9c22b810588643798e2b3bc5c24
Gerrit-Change-Number: 9601
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 13 Mar 2018 05:58:33 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2305: Limit sidecars to INT MAX and fortify socket code

[Joe McDonnell (Code Review)]

Joe McDonnell has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9601


Change subject: KUDU-2305: Limit sidecars to INT_MAX and fortify socket code
..

KUDU-2305: Limit sidecars to INT_MAX and fortify socket code

Inspection of the code revealed some other local variables
that could overflow with large messages. This patch takes
two approaches to eliminate the issues.

First, it limits the total size of the messages by limiting
the total size of the sidecars to INT_MAX. The total size
of the protobuf and header components of the message
should be considerably smaller, so limiting the sidecars
to INT_MAX eliminates messages that are larger than UINT_MAX.
This also means that the sidecar offsets, which are unsigned
32-bit integers, are also safe. Given that the
rpc_max_message_size is 32-bit integer, the receiver would
reject any message with sidecars this large anyway. This also
helps with the networking codepath, as any given sidecar will
have a size less than INT_MAX, so all Slices that interact
with Writev() are each less than INT_MAX.

Second, even with sidecars limited to INT_MAX, the headers
and protobuf parts of the messages mean that certain messages
could still exceed INT_MAX. This patch changes some of the sockets
codepath to tolerate iovec's that reference more than INT_MAX
bytes total. Specifically, it changes Writev()'s nwritten bytes
to an int64_t for both TlsSocket and Socket. TlsSocket works
because it is sending each Slice individually. The first change
limited any given Slice to INT_MAX, so each individual Write()
should not be impacted. For Socket, Writev() uses sendmsg(). It
should do partial network sends to handle this case. Any Write()
call specifies its size with a 32-bit integer, and that will
not be impacted by this patch.

Testing:
 - Modified TestRpcSidecarLimits() to verify that sidecars are
   limited to INT_MAX.

Change-Id: I2d041e214b15d9c22b810588643798e2b3bc5c24
---
M src/kudu/rpc/inbound_call.cc
M src/kudu/rpc/inbound_call.h
M src/kudu/rpc/outbound_call.cc
M src/kudu/rpc/rpc-test.cc
M src/kudu/rpc/rpc_controller.cc
M src/kudu/rpc/rpc_controller.h
M src/kudu/rpc/serialization.cc
M src/kudu/rpc/transfer.cc
M src/kudu/rpc/transfer.h
M src/kudu/security/tls_socket-test.cc
M src/kudu/security/tls_socket.cc
M src/kudu/security/tls_socket.h
M src/kudu/util/net/socket.cc
M src/kudu/util/net/socket.h
14 files changed, 63 insertions(+), 15 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2d041e214b15d9c22b810588643798e2b3bc5c24
Gerrit-Change-Number: 9601
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell 

[kudu-CR] KUDU-2335. Work around rare consensus health bug for 1.7 release

[Mike Percy (Code Review)]

Mike Percy has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9597 )

Change subject: KUDU-2335. Work around rare consensus health bug for 1.7 release
..

KUDU-2335. Work around rare consensus health bug for 1.7 release

In very rare circumstances we have hit a DHCECK in quorum_util.cc in
pre-commit builds stating that the leader should always have a HEALTHY
health status. We have traced this to points in the replica lifecycle
when the health status could be UNKNOWN.

Since we want to release 1.7.0 soon, let's work around this issue for
now. We'll follow up with a "real" fix and a decent test later.

Change-Id: Iad67c7943a5b619ef2fa3a67c92cc033e207e197
Reviewed-on: http://gerrit.cloudera.org:8080/9597
Reviewed-by: Alexey Serbin 
Tested-by: Mike Percy 
---
M src/kudu/consensus/quorum_util.cc
1 file changed, 13 insertions(+), 3 deletions(-)

Approvals:
  Alexey Serbin: Looks good to me, approved
  Mike Percy: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Iad67c7943a5b619ef2fa3a67c92cc033e207e197
Gerrit-Change-Number: 9597
Gerrit-PatchSet: 4
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 

[kudu-CR] KUDU-2335. Work around rare consensus health bug for 1.7 release

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9597 )

Change subject: KUDU-2335. Work around rare consensus health bug for 1.7 release
..


Patch Set 3: Verified+1

test failed due to KUDU-2059, overriding Jenkins


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iad67c7943a5b619ef2fa3a67c92cc033e207e197
Gerrit-Change-Number: 9597
Gerrit-PatchSet: 3
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Tue, 13 Mar 2018 05:42:28 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2335. Work around rare consensus health bug for 1.7 release

[Mike Percy (Code Review)]

Mike Percy has removed a vote on this change.

Change subject: KUDU-2335. Work around rare consensus health bug for 1.7 release
..


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Iad67c7943a5b619ef2fa3a67c92cc033e207e197
Gerrit-Change-Number: 9597
Gerrit-PatchSet: 3
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 

[kudu-CR] [kudu-jepsen] clarify on how binaries get to DB nodes

[Alexey Serbin (Code Review)]

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

Change subject: [kudu-jepsen] clarify on how binaries get to DB nodes
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9419/2/java/kudu-jepsen/README.adoc
File java/kudu-jepsen/README.adoc:

http://gerrit.cloudera.org:8080/#/c/9419/2/java/kudu-jepsen/README.adoc@90
PS2, Line 90: By default, the local binaries mode is used.
> Can we document how to switch to the second mode?
It turned out the second (package-like mode) does not have all the necessary 
knobs available at the maven level.  It seems there was some progress on that: 
https://gerrit.cloudera.org/#/c/5769/

I will need to add that, and it would make sense to do so in a separate 
changelist.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8b56f1be6e232d954e374ed1720977ee2dd646f
Gerrit-Change-Number: 9419
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 13 Mar 2018 05:36:39 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2322: don't spew logs about behind-log-GC follower

[Alexey Serbin (Code Review)]

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

Change subject: KUDU-2322: don't spew logs about behind-log-GC follower
..


Patch Set 2: Verified+1

Unrelated flakes in:

  org.apache.kudu.client.TestAsyncKuduSession
  org.apache.kudu.client.TestSecurity.testImportExportAuthenticationCredentials


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4438e63aef0be26c149c57f9f58cb54327e41a33
Gerrit-Change-Number: 9573
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 13 Mar 2018 05:33:10 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2322: don't spew logs about behind-log-GC follower

[Alexey Serbin (Code Review)]

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

Change subject: KUDU-2322: don't spew logs about behind-log-GC follower
..


Removed reviewer Kudu Jenkins with the following votes:

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: I4438e63aef0be26c149c57f9f58cb54327e41a33
Gerrit-Change-Number: 9573
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [release notes] replica management scheme notes

[Alexey Serbin (Code Review)]

Hello Will Berkeley, Mike Percy, Dan Burkert, Jean-Daniel Cryans, Kudu Jenkins, 
Grant Henke, Todd Lipcon,

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

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

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

Change subject: [release_notes] replica management scheme notes
..

[release_notes] replica management scheme notes

Added relevant notes on the new replica management scheme used
in Kudu 1.7 by default:
  * the new replica management scheme is incompatible with old one
  * rolling upgrade 1.6 -> 1.7 is not possible

Change-Id: I49f1f1e17cdaee272592d598431a33dbfe55123f
---
M docs/release_notes.adoc
1 file changed, 38 insertions(+), 2 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I49f1f1e17cdaee272592d598431a33dbfe55123f
Gerrit-Change-Number: 9571
Gerrit-PatchSet: 6
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] KUDU-2334: Fix OutboundTransfer::TransferStarted() to work with SSL write()

[Sailesh Mukil (Code Review)]

Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9587 )

Change subject: KUDU-2334: Fix OutboundTransfer::TransferStarted() to work with 
SSL_write()
..


Patch Set 2: Code-Review+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id7ebdcbc1ef2a3e0c5e7162f03214c232755b683
Gerrit-Change-Number: 9587
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 13 Mar 2018 04:34:13 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2334: Fix OutboundTransfer::TransferStarted() to work with SSL write()

[Sailesh Mukil (Code Review)]

Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9587 )

Change subject: KUDU-2334: Fix OutboundTransfer::TransferStarted() to work with 
SSL_write()
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9587/2/src/kudu/rpc/rpc-test.cc
File src/kudu/rpc/rpc-test.cc:

http://gerrit.cloudera.org:8080/#/c/9587/2/src/kudu/rpc/rpc-test.cc@1225
PS2, Line 1225: // This serves as a helper function for 
TestCancellationMultiThreads() to exercise
  : // cancellation when there are concurrent RPCs.
nit: Better to not explain what a user of this function would do, since it's 
already explained in TestCancellationMultiThreads().



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id7ebdcbc1ef2a3e0c5e7162f03214c232755b683
Gerrit-Change-Number: 9587
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 13 Mar 2018 04:33:57 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2322: don't spew logs about behind-log-GC follower

[Alexey Serbin (Code Review)]

Hello Mike Percy, Kudu Jenkins, Todd Lipcon,

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

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

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

Change subject: KUDU-2322: don't spew logs about behind-log-GC follower
..

KUDU-2322: don't spew logs about behind-log-GC follower

This patch addresses the issue of a leader spewing a lot of INFO
log messages about missing logs segments for a follower that falls
behind log GC.  In the 3-4-3 replica management scheme, such followers
linger around until corresponding non-voter replica catches up with
the leader.  That might take long time.

With this patch, the leader throttles messages about missing log
segments for a behind-log-GC follower, outputting 1 message per minute.

Change-Id: I4438e63aef0be26c149c57f9f58cb54327e41a33
---
M src/kudu/consensus/consensus-test-util.h
M src/kudu/consensus/consensus_peers.cc
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/consensus_queue.h
4 files changed, 37 insertions(+), 25 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4438e63aef0be26c149c57f9f58cb54327e41a33
Gerrit-Change-Number: 9573
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2338: [Java] Coerce BigDecimal values in KuduPredicate

[Grant Henke (Code Review)]

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

Change subject: KUDU-2338: [Java] Coerce BigDecimal values in KuduPredicate
..

KUDU-2338: [Java] Coerce BigDecimal values in KuduPredicate

We need to coerce the BigDecimal values to the expected
scale to ensure they can be correctly decoded server side.

This was missed in the KuduPredicate implementation though
we did it everywhere else.

I added the coercion and a test that will fail if it is ever removed.

Change-Id: Ia7e1e397524dfc2725b33b9683ebbdc38fe8cfc8
Reviewed-on: http://gerrit.cloudera.org:8080/9598
Reviewed-by: Mike Percy 
Tested-by: Kudu Jenkins
---
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduPredicate.java
2 files changed, 20 insertions(+), 0 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia7e1e397524dfc2725b33b9683ebbdc38fe8cfc8
Gerrit-Change-Number: 9598
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 

[kudu-CR] [release notes] replica management scheme notes

[Grant Henke (Code Review)]

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

Change subject: [release_notes] replica management scheme notes
..


Patch Set 5: Code-Review+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I49f1f1e17cdaee272592d598431a33dbfe55123f
Gerrit-Change-Number: 9571
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Tue, 13 Mar 2018 03:27:01 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2335. Work around rare consensus health bug for 1.7 release

[Alexey Serbin (Code Review)]

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

Change subject: KUDU-2335. Work around rare consensus health bug for 1.7 release
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iad67c7943a5b619ef2fa3a67c92cc033e207e197
Gerrit-Change-Number: 9597
Gerrit-PatchSet: 3
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 13 Mar 2018 03:12:16 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2338: [Java] Coerce BigDecimal values in KuduPredicate

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9598 )

Change subject: KUDU-2338: [Java] Coerce BigDecimal values in KuduPredicate
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7e1e397524dfc2725b33b9683ebbdc38fe8cfc8
Gerrit-Change-Number: 9598
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Tue, 13 Mar 2018 03:10:53 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2338: [Java] Coerce BigDecimal values in KuduPredicate

[Grant Henke (Code Review)]

Hello Mike Percy, Dan Burkert, Kudu Jenkins,

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

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

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

Change subject: KUDU-2338: [Java] Coerce BigDecimal values in KuduPredicate
..

KUDU-2338: [Java] Coerce BigDecimal values in KuduPredicate

We need to coerce the BigDecimal values to the expected
scale to ensure they can be correctly decoded server side.

This was missed in the KuduPredicate implementation though
we did it everywhere else.

I added the coercion and a test that will fail if it is ever removed.

Change-Id: Ia7e1e397524dfc2725b33b9683ebbdc38fe8cfc8
---
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduPredicate.java
2 files changed, 20 insertions(+), 0 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia7e1e397524dfc2725b33b9683ebbdc38fe8cfc8
Gerrit-Change-Number: 9598
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 

[kudu-CR] KUDU-2338: [Java] Coerce BigDecimal values in KuduPredicate

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9598 )

Change subject: KUDU-2338: [Java] Coerce BigDecimal values in KuduPredicate
..


Patch Set 1: Code-Review+2

(1 comment)

lgtm

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

http://gerrit.cloudera.org:8080/#/c/9598/1//COMMIT_MSG@12
PS1, Line 12: implimentation
implementation



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7e1e397524dfc2725b33b9683ebbdc38fe8cfc8
Gerrit-Change-Number: 9598
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Tue, 13 Mar 2018 03:08:50 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2322: don't spew logs about behind-log-GC follower

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9573 )

Change subject: KUDU-2322: don't spew logs about behind-log-GC follower
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9573/1/src/kudu/consensus/consensus_queue.cc@652
PS1, Line 652: KLOG_EVERY_N_SECS_THROTTLER(INFO, 60, 
peer_copy.status_log_throttler, "logs_gced")
> Right -- that was my concern as well.  However, I saw this construct is use
Alexey and I discussed this offline and we think a shared_ptr for the 
LogThrottler member would solve the problem with the copies.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4438e63aef0be26c149c57f9f58cb54327e41a33
Gerrit-Change-Number: 9573
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 13 Mar 2018 02:50:13 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2335. Work around rare consensus health bug for 1.7 release

[Mike Percy (Code Review)]

Hello Alexey Serbin, Kudu Jenkins,

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

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

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

Change subject: KUDU-2335. Work around rare consensus health bug for 1.7 release
..

KUDU-2335. Work around rare consensus health bug for 1.7 release

In very rare circumstances we have hit a DHCECK in quorum_util.cc in
pre-commit builds stating that the leader should always have a HEALTHY
health status. We have traced this to points in the replica lifecycle
when the health status could be UNKNOWN.

Since we want to release 1.7.0 soon, let's work around this issue for
now. We'll follow up with a "real" fix and a decent test later.

Change-Id: Iad67c7943a5b619ef2fa3a67c92cc033e207e197
---
M src/kudu/consensus/quorum_util.cc
1 file changed, 13 insertions(+), 3 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iad67c7943a5b619ef2fa3a67c92cc033e207e197
Gerrit-Change-Number: 9597
Gerrit-PatchSet: 3
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [doc] Document the new decimal column type

[Grant Henke (Code Review)]

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

Change subject: [doc] Document the new decimal column type
..

[doc] Document the new decimal column type

Change-Id: I9489613d35daad708648ea04d49e472d3149b33d
Reviewed-on: http://gerrit.cloudera.org:8080/9432
Reviewed-by: Grant Henke 
Tested-by: Grant Henke 
---
M docs/developing.adoc
M docs/known_issues.adoc
M docs/kudu_impala_integration.adoc
M docs/release_notes.adoc
M docs/schema_design.adoc
5 files changed, 65 insertions(+), 8 deletions(-)

Approvals:
  Grant Henke: Looks good to me, approved; Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I9489613d35daad708648ea04d49e472d3149b33d
Gerrit-Change-Number: 9432
Gerrit-PatchSet: 6
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [doc] Document the new decimal column type

[Grant Henke (Code Review)]

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

Change subject: [doc] Document the new decimal column type
..


Patch Set 5: Verified+1 Code-Review+2

Adding back Todd's +2 after the simple rebase.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9489613d35daad708648ea04d49e472d3149b33d
Gerrit-Change-Number: 9432
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 13 Mar 2018 02:19:41 +
Gerrit-HasComments: No

[kudu-CR] [release notes]: RYW read mode

[Hao Hao (Code Review)]

Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9594 )

Change subject: [release notes]: RYW read mode
..


Patch Set 1: Verified+1

Unrelated test failure.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9906398b0796307c26150f78e0e256e285ded516
Gerrit-Change-Number: 9594
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 13 Mar 2018 02:18:52 +
Gerrit-HasComments: No

[kudu-CR] [release notes]: RYW read mode

[Hao Hao (Code Review)]

Hao Hao has removed a vote on this change.

Change subject: [release notes]: RYW read mode
..


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I9906398b0796307c26150f78e0e256e285ded516
Gerrit-Change-Number: 9594
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [doc] Document the new decimal column type

[Grant Henke (Code Review)]

Hello Alex Rodoni, Dan Burkert, Kudu Jenkins, Todd Lipcon,

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

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

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

Change subject: [doc] Document the new decimal column type
..

[doc] Document the new decimal column type

Change-Id: I9489613d35daad708648ea04d49e472d3149b33d
---
M docs/developing.adoc
M docs/known_issues.adoc
M docs/kudu_impala_integration.adoc
M docs/release_notes.adoc
M docs/schema_design.adoc
5 files changed, 65 insertions(+), 8 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9489613d35daad708648ea04d49e472d3149b33d
Gerrit-Change-Number: 9432
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] WIP: KUDU-2335. Do not collect tablet report when queue is closed

[Mike Percy (Code Review)]

Hello Alexey Serbin,

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

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

to review the following change.


Change subject: WIP: KUDU-2335. Do not collect tablet report when queue is 
closed
..

WIP: KUDU-2335. Do not collect tablet report when queue is closed

Needs tests.

Change-Id: I93cccbc7c76c5920485e85c68f131e141146005e
---
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/consensus_queue.h
M src/kudu/consensus/raft_consensus.cc
3 files changed, 16 insertions(+), 4 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I93cccbc7c76c5920485e85c68f131e141146005e
Gerrit-Change-Number: 9599
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 

[kudu-CR] KUDU-2338: [Java] Coerce BigDecimal values in KuduPredicate

[Grant Henke (Code Review)]

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


Change subject: KUDU-2338: [Java] Coerce BigDecimal values in KuduPredicate
..

KUDU-2338: [Java] Coerce BigDecimal values in KuduPredicate

We need to coerce the BigDecimal values to the expected
scale to ensure they can be correctly decoded server side.

This was missed in the KuduPredicate implimentation though
we did it everywhere else.

I added the coercion and a test that will fail if it is ever removed.

Change-Id: Ia7e1e397524dfc2725b33b9683ebbdc38fe8cfc8
---
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduPredicate.java
2 files changed, 20 insertions(+), 0 deletions(-)



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

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

[kudu-CR] KUDU-2335. Work around rare consensus health bug for 1.7 release

[Alexey Serbin (Code Review)]

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

Change subject: KUDU-2335. Work around rare consensus health bug for 1.7 release
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iad67c7943a5b619ef2fa3a67c92cc033e207e197
Gerrit-Change-Number: 9597
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 13 Mar 2018 02:04:55 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2335. Work around rare consensus health bug for 1.7 release

[Mike Percy (Code Review)]

Hello Alexey Serbin,

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

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

to review the following change.


Change subject: KUDU-2335. Work around rare consensus health bug for 1.7 release
..

KUDU-2335. Work around rare consensus health bug for 1.7 release

In very rare circumstances we have hit a DHCECK in quorum_util.cc
stating that the leader should never have a "failed" health status.
If we hit this in production we should log an error and decline to evict
any nodes from that configuration.

Since this is just a workaround for a very rare bug, we should also
implement a fix once we get to the bottom of the issue.

Change-Id: Iad67c7943a5b619ef2fa3a67c92cc033e207e197
---
M src/kudu/consensus/quorum_util.cc
1 file changed, 8 insertions(+), 3 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iad67c7943a5b619ef2fa3a67c92cc033e207e197
Gerrit-Change-Number: 9597
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 

[kudu-CR] KUDU-2334: Fix OutboundTransfer::TransferStarted() to work with SSL write()

[Michael Ho (Code Review)]

Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9587 )

Change subject: KUDU-2334: Fix OutboundTransfer::TransferStarted() to work with 
SSL_write()
..


Patch Set 2:

Hit with flaky tests again in TSAN build:

[  FAILED  ] AllTypesItest/3.TestAllKeyTypes, where TypeParam = 
kudu::client::IntKeysTestSetup >
[  FAILED  ] AllTypesItest/7.TestAllKeyTypes, where TypeParam = 
kudu::client::IntKeysTestSetup 
>

Both failed with the following backtraces:

F0313 01:15:33.904728 14346 master_main.cc:74] Check failed: _s.ok() Bad 
status: Service unavailable: Cannot initialize clock: Error reading clock. 
Clock considered unsynchronized
*** Check failure stack trace: ***
@ 0x7f9641ff03b9  google::LogMessage::Flush() at ??:0
@ 0x7f9641ff446e  google::LogMessageFatal::~LogMessageFatal() at ??:0
@   0x4b5391  kudu::master::MasterMain() at 
/home/jenkins-slave/workspace/kudu-master/3/thirdparty/src/llvm-4.0.0.src/projects/compiler-rt/lib/tsan/../sanitizer_common/sanitizer_common_interceptors.inc:6199
@   0x4b4ddf  main at 
/home/jenkins-slave/workspace/kudu-master/3/thirdparty/src/llvm-4.0.0.src/projects/compiler-rt/lib/tsan/../sanitizer_common/sanitizer_common_interceptors.inc:6189
@ 0x7f963f379f45  __libc_start_main at ??:0
@   0x41e092  (unknown) at ??:0
/home/jenkins-slave/workspace/kudu-master/3/src/kudu/integration-tests/all_types-itest.cc:518:
 Failure


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id7ebdcbc1ef2a3e0c5e7162f03214c232755b683
Gerrit-Change-Number: 9587
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 13 Mar 2018 01:47:34 +
Gerrit-HasComments: No

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

[Todd Lipcon (Code Review)]

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

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


Patch Set 2:

> Patch Set 2:
>
> (1 comment)

oh indeed, good catch


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I092cab794fd20a39fcfbbbe52f4c325fac2d149c
Gerrit-Change-Number: 9554
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 13 Mar 2018 01:39:13 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2336. diagnostics log: fix an assertion failure for null stack frames

[Alexey Serbin (Code Review)]

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

Change subject: KUDU-2336. diagnostics_log: fix an assertion failure for null 
stack frames
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I73ada54bef056b665c03ca142a3995ae6ad59230
Gerrit-Change-Number: 9591
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 13 Mar 2018 01:39:10 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2336. diagnostics log: fix an assertion failure for null stack frames

[Todd Lipcon (Code Review)]

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

Change subject: KUDU-2336. diagnostics_log: fix an assertion failure for null 
stack frames
..

KUDU-2336. diagnostics_log: fix an assertion failure for null stack frames

On my Ubuntu 16 box, in ASAN builds, it seems I sometimes get a stack
frame with a nullptr address. This caused SymbolSet::Add(...) to fail an
assertion since it uses google::dense_hash_set<> which requires that
there be one key reserved as a non-insertable "empty" marker. We were
using nullptr for that but not checking before potentially inserting it.

This adds a simple workaround. Without the patch, if I started a tserver
with --diagnostics-log-stack-traces-interval-ms=10, it would crash about
10% of the time with the following assertion failure:

kudu-master: 
../../thirdparty/installed/common/include/sparsehash/internal/densehashtable.h:969:
  std::pair google::dense_hashtable<...>...:
  Assertion `!equals(std::forward(key), key_info.empty_key) && "Inserting 
the empty key"' failed.

Change-Id: I73ada54bef056b665c03ca142a3995ae6ad59230
Reviewed-on: http://gerrit.cloudera.org:8080/9591
Tested-by: Todd Lipcon 
Reviewed-by: Alexey Serbin 
---
M src/kudu/server/diagnostics_log.cc
1 file changed, 4 insertions(+), 1 deletion(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I73ada54bef056b665c03ca142a3995ae6ad59230
Gerrit-Change-Number: 9591
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2336. diagnostics log: fix an assertion failure for null stack frames

[Todd Lipcon (Code Review)]

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

Change subject: KUDU-2336. diagnostics_log: fix an assertion failure for null 
stack frames
..


Patch Set 2: Verified+1

Precommit failed due to clock sync again


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I73ada54bef056b665c03ca142a3995ae6ad59230
Gerrit-Change-Number: 9591
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 13 Mar 2018 01:37:14 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2336. diagnostics log: fix an assertion failure for null stack frames

[Todd Lipcon (Code Review)]

Todd Lipcon has removed a vote on this change.

Change subject: KUDU-2336. diagnostics_log: fix an assertion failure for null 
stack frames
..


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I73ada54bef056b665c03ca142a3995ae6ad59230
Gerrit-Change-Number: 9591
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Remove tcmalloc spinlock contention metric

[Todd Lipcon (Code Review)]

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

Change subject: Remove tcmalloc_spinlock_contention metric
..


Patch Set 1: Verified+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2c29b81a2c692d5d876d2dd5381ba2c295684324
Gerrit-Change-Number: 9595
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 13 Mar 2018 01:34:56 +
Gerrit-HasComments: No

[kudu-CR] Remove tcmalloc spinlock contention metric

[Todd Lipcon (Code Review)]

Todd Lipcon has removed Kudu Jenkins from this change.  ( 
http://gerrit.cloudera.org:8080/9595 )

Change subject: Remove tcmalloc_spinlock_contention metric
..


Removed reviewer Kudu Jenkins with the following votes:

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: I2c29b81a2c692d5d876d2dd5381ba2c295684324
Gerrit-Change-Number: 9595
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 

[kudu-CR] KUDU-2153. Servers should not delete tmp files until after locking directories

[Todd Lipcon (Code Review)]

Hello Andrew Wong,

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

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

to review the following change.


Change subject: KUDU-2153. Servers should not delete tmp files until after 
locking directories
..

KUDU-2153. Servers should not delete tmp files until after locking directories

This changes the order of FsManager startup to not try to clean tmp
files until after successfully locking the data directories. This
prevents potential issues such as:

- a tserver is already running on some host, and in the middle of
  consensus voting. Thus it has created a tmp file.
- someone accidentally attempts to start a tserver with the same set of
  data dirs. Prior to this patch, it would delete the tmp file before
  realizing that it could not successfully lock its data dirs and
  aborting.
- the original tserver would crash or otherwise get very confused
  because the tmp file it just wrote would be gone.

This patch relies on the locking on the block manager instance files to
provide exclusive access to some non-block-manager-related storage such
as the consensus meta, etc. That means that it's still possible for
someone to hit the above issue if they were to start servers with
disjoint sets of data dirs but with the same meta dir. However, the
patch is still a net improvement because the most likely scenario is
that the two servers are started with identical configurations.

This patch also removes the block_manager_lock_dirs flag which was
apparently unused. It was always marked as 'unsafe' so it's not a
compatibility issue to remove it without a deprecation period.

Change-Id: I3a3471c8ce00e77fa1712ea518f6ab281864a08d
---
M src/kudu/fs/block_manager.cc
M src/kudu/fs/file_block_manager.cc
M src/kudu/fs/fs_manager-test.cc
M src/kudu/fs/fs_manager.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/util/env_posix.cc
6 files changed, 46 insertions(+), 20 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3a3471c8ce00e77fa1712ea518f6ab281864a08d
Gerrit-Change-Number: 9596
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Andrew Wong 

[kudu-CR] KUDU-2334: Fix OutboundTransfer::TransferStarted() to work with SSL write()

[Michael Ho (Code Review)]

Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9587 )

Change subject: KUDU-2334: Fix OutboundTransfer::TransferStarted() to work with 
SSL_write()
..


Patch Set 1:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/9587/1/src/kudu/rpc/rpc-test-base.h
File src/kudu/rpc/rpc-test-base.h:

http://gerrit.cloudera.org:8080/#/c/9587/1/src/kudu/rpc/rpc-test-base.h@441
PS1, Line 441:   FLAGS_rpc_encrypt_loopback_connections = true;
> ah, does this mean we were not successfully testing encrypted RPCs in the p
I believe so. I confirmed that the TlsSocket path is now being exercised by 
manually adding a CHECK() in that TlsSocket::Write().


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

http://gerrit.cloudera.org:8080/#/c/9587/1/src/kudu/rpc/rpc-test.cc@740
PS1, Line 740:// Linux, SSL enabled
> hmm, is this fix related to the patch in question? I'm surprised we haven't
Yes, as we weren't using TlsSocket before so we weren't triggering this error 
message.


http://gerrit.cloudera.org:8080/#/c/9587/1/src/kudu/rpc/rpc-test.cc@1223
PS1, Line 1223: // Helper function for TestCancellationMultiThreads.
> better to document briefly what it does, rather than what it is.
Done


http://gerrit.cloudera.org:8080/#/c/9587/1/src/kudu/rpc/rpc-test.cc@1224
PS1, Line 1224: SendRpcAndCancel
> this should probably be named SendAndCancelRpcs or something, since it send
Done


http://gerrit.cloudera.org:8080/#/c/9587/1/src/kudu/rpc/rpc-test.cc@1251
PS1, Line 1251: ASSERT_TRUE
> have to use CHECK here since this isn't the main thread and gtest isn't thr
Done


http://gerrit.cloudera.org:8080/#/c/9587/1/src/kudu/rpc/rpc-test.cc@1256
PS1, Line 1256: #define BUF_SIZE (16 * 1024 * 1024)
> can you use a const int defined below within the function instead?  We pref
Done


http://gerrit.cloudera.org:8080/#/c/9587/1/src/kudu/rpc/rpc-test.cc@1258
PS1, Line 1258: Regression
> did this test successfully repro the bug w/o the fix?
Unfortunately, no even with multiple tries in different build types. That said, 
I think this is still a good test to include. Any suggestion to make this test 
more effective in triggering the bug is welcomed.

I confirmed the fix with Impala stress test running on a 130 node cluster which 
hit this bug quite consistently without the fix. Updated the commit message 
with this piece of information.


http://gerrit.cloudera.org:8080/#/c/9587/1/src/kudu/rpc/rpc-test.cc@1274
PS1, Line 1274:   unique_ptr buf(new uint8_t[BUF_SIZE]);
  :   memset(buf.get(), 'a', BUF_SIZE);
  :   Slice slice(buf.get(), BUF_SIZE);
> you could probably get away with just:
Done


http://gerrit.cloudera.org:8080/#/c/9587/1/src/kudu/rpc/transfer.h
File src/kudu/rpc/transfer.h:

http://gerrit.cloudera.org:8080/#/c/9587/1/src/kudu/rpc/transfer.h@186
PS1, Line 186: True if we have tried sending anything in 'payload_slices_'
> Actually isn't it more like "True if SendBuffer() is called at least once."
Done


http://gerrit.cloudera.org:8080/#/c/9587/1/src/kudu/rpc/transfer.h@187
PS1, Line 187:   // no bytes were sent successfully. This is needed as 
SSL_write() is stateful.
> nit: Add a reference to this JIRA, else it's hard to understand why we adde
Done


http://gerrit.cloudera.org:8080/#/c/9587/1/src/kudu/rpc/transfer.cc
File src/kudu/rpc/transfer.cc:

http://gerrit.cloudera.org:8080/#/c/9587/1/src/kudu/rpc/transfer.cc@190
PS1, Line 190:   started_ = true;
> Do we know of any cases where setting started_ = true this early can backfi
As far as I can tell, this should be fine. Both SendBuffer() and 
OutboundTransfer::TransferStarted() are called from Connection::WriteHandler() 
only. So, if we get to the point of SendBuffer(), we should have taken the path 
of if (!transfer->TransferStarted()) once.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id7ebdcbc1ef2a3e0c5e7162f03214c232755b683
Gerrit-Change-Number: 9587
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 13 Mar 2018 00:54:24 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2334: Fix OutboundTransfer::TransferStarted() to work with SSL write()

[Michael Ho (Code Review)]

Hello Kudu Jenkins, Sailesh Mukil, Todd Lipcon,

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

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

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

Change subject: KUDU-2334: Fix OutboundTransfer::TransferStarted() to work with 
SSL_write()
..

KUDU-2334: Fix OutboundTransfer::TransferStarted() to work with SSL_write()

Previously, OutboundTransfer::TransferStarted() returns true iff
non-zero bytes have been successfully sent via Writev(). As it turns
out, this doesn't work well with SSL_write(). When SSL_write() returns -1
with errno EAGAIN or ETRYAGAIN, we need to retry the call with exactly
the same buffer pointer next time even if 0 bytes have been written.

The following sequence becomes problematic with the previous implementation
of OutboundTransfer::TransferStarted():

- WriteHandler() calls SendBuffer() on an OutboundTransfer.
- SendBuffer() calls TlsSocket::Writev() which hits the EAGAIN error above.
  Since 0 bytes were written, cur_slice_idx_ and cur_offset_in_slice_ remain 0
  and OutboundTransfer::TransferStarted() still returns false.
- OutboundTransfer is cancelled or timed out. car->call is set to NULL.
- WirteHandler() is called again and as it notices that the OutboundTransfer
  hasn't really started yet and "car->call" is NULL due to cancellation, it
  removes it from the outbound transfer queue and moves on to the next entry
  in the queue.
- WriteHandler() calls SendBuffer() with the next entry in the queue and
  eventually calls SSL_write() with a different buffer than expected by
  SSL_write(), leading to "SSL3_WRITE_PENDING:bad write retry" error.

This change fixes the problem above by adding a boolean flag 'started_'
which is set to true if OutboundTransfer::SendBuffer() has been called
at least once. Also added some tests to exercise cancellation paths with
multiple concurrent RPCs.

Confirmed the problem above is fixed by running stress test in a 130 node
cluster with Impala. The problem happened consistently without the fix.

Change-Id: Id7ebdcbc1ef2a3e0c5e7162f03214c232755b683
---
M src/kudu/rpc/rpc-test-base.h
M src/kudu/rpc/rpc-test.cc
M src/kudu/rpc/transfer.cc
M src/kudu/rpc/transfer.h
4 files changed, 84 insertions(+), 1 deletion(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id7ebdcbc1ef2a3e0c5e7162f03214c232755b683
Gerrit-Change-Number: 9587
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 

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

[Alexey Serbin (Code Review)]

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

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


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9554/2/build-support/iwyu/iwyu_tool.py
File build-support/iwyu/iwyu_tool.py:

http://gerrit.cloudera.org:8080/#/c/9554/2/build-support/iwyu/iwyu_tool.py@190
PS2, Line 190: '(-isystem |-I\s*)([^/]\S+)'
> but that's not a relative path, so it shouldn't match, right?
Maybe I'm missing something, but I don't see how it's guaranteed that the 
command doesn't include absolute paths for '-I' directives.  And if so, then I 
think the result is not what we want:

>>> import os
>>> import re
>>>
>>> def subber(m):
... return m.group(1) + os.path.abspath(os.path.join('/tmp') + 
m.group(2))
...

>>> re.sub(r'(-isystem |-I\s*)([^/]\S+)', subber, '-I /usr/include')
'-I/tmp /usr/include'
>>>

So, that's not what we want, right?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I092cab794fd20a39fcfbbbe52f4c325fac2d149c
Gerrit-Change-Number: 9554
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 13 Mar 2018 00:42:33 +
Gerrit-HasComments: Yes

[kudu-CR] Remove tcmalloc spinlock contention metric

[Todd Lipcon (Code Review)]

Hello Alexey Serbin,

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

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

to review the following change.


Change subject: Remove tcmalloc_spinlock_contention metric
..

Remove tcmalloc_spinlock_contention metric

Spinlock contention was removed from tcmalloc upstream, so this metric
has been set to 0 since we upgraded to tcmalloc 2.6. I looked briefly at
reverting its removal upstream, but the revert would be fairly large, so
I think it's best to just accept the loss of this instrumentation.

Change-Id: I2c29b81a2c692d5d876d2dd5381ba2c295684324
---
M docs/release_notes.adoc
M src/kudu/scripts/parse_metrics_log.py
M src/kudu/util/spinlock_profiling-test.cc
M src/kudu/util/spinlock_profiling.cc
M src/kudu/util/spinlock_profiling.h
M src/kudu/util/trace.h
M src/kudu/util/trace_metrics.h
7 files changed, 7 insertions(+), 103 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2c29b81a2c692d5d876d2dd5381ba2c295684324
Gerrit-Change-Number: 9595
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 

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

[Todd Lipcon (Code Review)]

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

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


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9554/2/build-support/iwyu/iwyu_tool.py
File build-support/iwyu/iwyu_tool.py:

http://gerrit.cloudera.org:8080/#/c/9554/2/build-support/iwyu/iwyu_tool.py@190
PS2, Line 190: '(-isystem |-I\s*)([^/]\S+)'
> I don't think this pattern works properly with include path specs like '-I
but that's not a relative path, so it shouldn't match, right?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I092cab794fd20a39fcfbbbe52f4c325fac2d149c
Gerrit-Change-Number: 9554
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 13 Mar 2018 00:29:06 +
Gerrit-HasComments: Yes

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

[Alexey Serbin (Code Review)]

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

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


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9554/2/build-support/iwyu/iwyu_tool.py
File build-support/iwyu/iwyu_tool.py:

http://gerrit.cloudera.org:8080/#/c/9554/2/build-support/iwyu/iwyu_tool.py@190
PS2, Line 190: '(-isystem |-I\s*)([^/]\S+)'
I don't think this pattern works properly with include path specs like '-I 
/usr/include'.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I092cab794fd20a39fcfbbbe52f4c325fac2d149c
Gerrit-Change-Number: 9554
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 13 Mar 2018 00:28:06 +
Gerrit-HasComments: Yes

[kudu-CR] release notes: changes to ksck reporting updates

[Andrew Wong (Code Review)]

Andrew Wong has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9593 )

Change subject: release notes: changes to ksck reporting updates
..

release notes: changes to ksck reporting updates

Covers:
https://github.com/apache/kudu/commit/d869b8033ac0026d585ab1d5bc69a1ba205357fa

Change-Id: Ia7bae8e1d298f4222bd1697f4409863b5880
Reviewed-on: http://gerrit.cloudera.org:8080/9593
Reviewed-by: Todd Lipcon 
Tested-by: Kudu Jenkins
---
M docs/release_notes.adoc
1 file changed, 4 insertions(+), 1 deletion(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia7bae8e1d298f4222bd1697f4409863b5880
Gerrit-Change-Number: 9593
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] [release notes]: RYW read mode

[Hao Hao (Code Review)]

Hao Hao has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9594


Change subject: [release notes]: RYW read mode
..

[release notes]: RYW read mode

Change-Id: I9906398b0796307c26150f78e0e256e285ded516
---
M docs/release_notes.adoc
1 file changed, 8 insertions(+), 1 deletion(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9906398b0796307c26150f78e0e256e285ded516
Gerrit-Change-Number: 9594
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao 

[kudu-CR] KUDU-2322: don't spew logs about behind-log-GC follower

[Alexey Serbin (Code Review)]

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

Change subject: KUDU-2322: don't spew logs about behind-log-GC follower
..


Patch Set 1:

(3 comments)

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

http://gerrit.cloudera.org:8080/#/c/9573/1/src/kudu/consensus/consensus_peers.cc@230
PS1, Line 230: VLOG_WITH_PREFIX_UNLOCKED(1) << s.ToString();
> Why is this demoted to VLOG instead of throttled? I've found this message u
All error messages from RequestForPeer() are printed (with throttle or not) 
within the RequestForPeer() method itself except for pure information message 
that a peer's queue is no longer in a leader mode, and I thought there is not 
much the reader can get out of that message anyway.


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

http://gerrit.cloudera.org:8080/#/c/9573/1/src/kudu/consensus/consensus_queue.cc@652
PS1, Line 652: KLOG_EVERY_N_SECS_THROTTLER(INFO, 60, 
peer_copy.status_log_throttler, "logs_gced")
> Also, how does this work given that the throttler here is a member of the p
Right -- that was my concern as well.  However, I saw this construct is used 
elsewhere in the code (like at line 697), so I assumed it worked somehow.  All 
right, I'll take a second look at that.


http://gerrit.cloudera.org:8080/#/c/9573/1/src/kudu/consensus/consensus_queue.cc@652
PS1, Line 652: KLOG_EVERY_N_SECS_THROTTLER(INFO, 60, 
peer_copy.status_log_throttler, "logs_gced")
> I'm a little bit concerned about not printing this message for every peer t
I thought the peer_copy.status_log_throttler takes care of the  per-peer part, 
but maybe I was wrong.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4438e63aef0be26c149c57f9f58cb54327e41a33
Gerrit-Change-Number: 9573
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 13 Mar 2018 00:14:06 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2295 fix nullptr dereference in Tablet

[Alexey Serbin (Code Review)]

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

Change subject: KUDU-2295 fix nullptr dereference in Tablet
..

KUDU-2295 fix nullptr dereference in Tablet

Prior to this patch, in the case of concurrent events of shutting
down a tablet and running a snapshot scan on it, sometimes the code
ended up trying to access already freed members of a Tablet object,
resulting in stack traces like below.

The traces were captured from modified raft_consensus_stress-itest
(added a single read thread for TestWorkload) runs using dist_test.

(after RYW changes):
PC: @ 0x7f563b5e31cb std::atomic_bool::load()
*** SIGSEGV (@0x1f8) received by PID 19893 (TID 0x7f561ce82700) from PID 504; 
stack trace: ***
@ 0x7f5638713330 (unknown) at ??:0
@ 0x7f563b5e31cb std::atomic_bool::load() at ??:0
@ 0x7f563b609c31 kudu::tablet::MvccManager::is_open() at ??:0
@ 0x7f563b6085f3 kudu::tablet::MvccManager::CheckOpen() at ??:0
@ 0x7f563b607fc5 kudu::tablet::MvccManager::WaitUntil() at ??:0
@ 0x7f563b608938 
kudu::tablet::MvccManager::WaitForSnapshotWithAllCommitted() at ??:0
@ 0x7f563ca61b55 
kudu::tserver::TabletServiceImpl::HandleScanAtSnapshot() at ??:0
@ 0x7f563ca5c0e2 
kudu::tserver::TabletServiceImpl::HandleNewScanRequest() at ??:0
@ 0x7f563ca59793 kudu::tserver::TabletServiceImpl::Scan() at ??:0
@ 0x7f5637324e4d 
kudu::tserver::TabletServerServiceIf::TabletServerServiceIf()::$_5::operator()()
 at ??:0
@ 0x7f5637324c92 std::_Function_handler<>::_M_invoke() at ??:0
@ 0x7f563648992b std::function<>::operator()() at ??:0
@ 0x7f56364891ed kudu::rpc::GeneratedServiceIf::Handle() at ??:0
@ 0x7f563648b5e6 kudu::rpc::ServicePool::RunThread() at ??:0
@ 0x7f563648dc29 boost::_mfi::mf0<>::operator()() at ??:0
@ 0x7f563648db90 boost::_bi::list1<>::operator()<>() at ??:0
@ 0x7f563648db3a boost::_bi::bind_t<>::operator()() at ??:0
@ 0x7f563648d91d 
boost::detail::function::void_function_obj_invoker0<>::invoke() at ??:0
@ 0x7f5636430078 boost::function0<>::operator()() at ??:0
@ 0x7f563472c08d kudu::Thread::SuperviseThread() at ??:0
@ 0x7f563870b184 start_thread at ??:0
@ 0x7f5630a2affd clone at ??:0

(before RYW changes):
PC: @ 0x7f1e02025790 scoped_refptr<>::operator->()
*** SIGSEGV (@0x160) received by PID 8782 (TID 0x7f1de3c7e700) from PID 352; 
stack trace: ***
@ 0x7f1dfdcfc330 (unknown) at ??:0
@ 0x7f1e02025790 scoped_refptr<>::operator->() at ??:0
@ 0x7f1e00ae62e7 kudu::tablet::Tablet::GetTabletAncientHistoryMark() at 
??:0
@ 0x7f1e00ae627d kudu::tablet::Tablet::GetHistoryGcOpts() at ??:0
@ 0x7f1e02012c53 kudu::tserver::(anonymous 
namespace)::VerifyNotAncientHistory() at ??:0
@ 0x7f1e0201223b 
kudu::tserver::TabletServiceImpl::HandleScanAtSnapshot() at ??:0
@ 0x7f1e0200c6dd 
kudu::tserver::TabletServiceImpl::HandleNewScanRequest() at ??:0
@ 0x7f1e02009d33 kudu::tserver::TabletServiceImpl::Scan() at ??:0
@ 0x7f1dfc90de4d 
kudu::tserver::TabletServerServiceIf::TabletServerServiceIf()::$_5::operator()()
 at ??:0
@ 0x7f1dfc90dc92 std::_Function_handler<>::_M_invoke() at ??:0
@ 0x7f1dfba728ab std::function<>::operator()() at ??:0
@ 0x7f1dfba7216d kudu::rpc::GeneratedServiceIf::Handle() at ??:0
@ 0x7f1dfba74526 kudu::rpc::ServicePool::RunThread() at ??:0
@ 0x7f1dfba76ad9 boost::_mfi::mf0<>::operator()() at ??:0
@ 0x7f1dfba76a40 boost::_bi::list1<>::operator()<>() at ??:0
@ 0x7f1dfba769ea boost::_bi::bind_t<>::operator()() at ??:0
@ 0x7f1dfba767cd 
boost::detail::function::void_function_obj_invoker0<>::invoke() at ??:0
@ 0x7f1dfba190f8 boost::function0<>::operator()() at ??:0
@ 0x7f1df9d1788d kudu::Thread::SuperviseThread() at ??:0
@ 0x7f1dfdcf4184 start_thread at ??:0
@ 0x7f1df6023ffd clone at ??:0
@0x0 (unknown)

Change-Id: Ib4b8cc2a307e5a0bd6019bd155f33c1565fca513
Reviewed-on: http://gerrit.cloudera.org:8080/9574
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo 
Reviewed-by: Mike Percy 
---
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tablet_service.h
2 files changed, 15 insertions(+), 14 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Adar Dembo: Looks good to me, but someone else must approve
  Mike Percy: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib4b8cc2a307e5a0bd6019bd155f33c1565fca513
Gerrit-Change-Number: 9574
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 

[kudu-CR] KUDU-2295 fix nullptr dereference in Tablet

[Alexey Serbin (Code Review)]

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

Change subject: KUDU-2295 fix nullptr dereference in Tablet
..


Patch Set 2:

> I agree with Todd that it would be good to have a test for this.
 > But this patch looks good to me.

Yep, sure -- I added https://issues.apache.org/jira/browse/KUDU-2337 to track 
this.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4b8cc2a307e5a0bd6019bd155f33c1565fca513
Gerrit-Change-Number: 9574
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 13 Mar 2018 00:00:57 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2322: don't spew logs about behind-log-GC follower

[Todd Lipcon (Code Review)]

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

Change subject: KUDU-2322: don't spew logs about behind-log-GC follower
..


Patch Set 1:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/9573/1/src/kudu/consensus/consensus_peers.cc@230
PS1, Line 230: VLOG_WITH_PREFIX_UNLOCKED(1) << s.ToString();
Why is this demoted to VLOG instead of throttled? I've found this message 
useful in the past to understand when a peer has fallen behind GC


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

http://gerrit.cloudera.org:8080/#/c/9573/1/src/kudu/consensus/consensus_queue.cc@652
PS1, Line 652: KLOG_EVERY_N_SECS_THROTTLER(INFO, 60, 
peer_copy.status_log_throttler, "logs_gced")
> I'm a little bit concerned about not printing this message for every peer t
Also, how does this work given that the throttler here is a member of the peer 
_copy_ and not the original peer itself? Isn't that a new instance each time?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4438e63aef0be26c149c57f9f58cb54327e41a33
Gerrit-Change-Number: 9573
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 12 Mar 2018 23:55:24 +
Gerrit-HasComments: Yes

[kudu-CR] release notes: changes to ksck reporting updates

[Todd Lipcon (Code Review)]

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

Change subject: release notes: changes to ksck reporting updates
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7bae8e1d298f4222bd1697f4409863b5880
Gerrit-Change-Number: 9593
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Mon, 12 Mar 2018 23:53:20 +
Gerrit-HasComments: No

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

[Todd Lipcon (Code Review)]

Hello Alexey Serbin, Kudu Jenkins,

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

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

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

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

iwyu: workaround missing IWYU results when using Ninja

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

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

Change-Id: I092cab794fd20a39fcfbbbe52f4c325fac2d149c
---
M build-support/iwyu/iwyu_tool.py
1 file changed, 27 insertions(+), 0 deletions(-)


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

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

[kudu-CR] release notes: changes to ksck reporting updates

[Andrew Wong (Code Review)]

Andrew Wong has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9593


Change subject: release notes: changes to ksck reporting updates
..

release notes: changes to ksck reporting updates

Covers:
https://github.com/apache/kudu/commit/d869b8033ac0026d585ab1d5bc69a1ba205357fa

Change-Id: Ia7bae8e1d298f4222bd1697f4409863b5880
---
M docs/release_notes.adoc
1 file changed, 4 insertions(+), 1 deletion(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia7bae8e1d298f4222bd1697f4409863b5880
Gerrit-Change-Number: 9593
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong 

[kudu-CR] KUDU-2295 fix nullptr dereference in Tablet

[Alexey Serbin (Code Review)]

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

Change subject: KUDU-2295 fix nullptr dereference in Tablet
..


Patch Set 2:

> > Would be nice to have a test for this in the codebase, since it
 > > sounds like you had to make some local modifications to
 > > raft_consensus-stress-itest to cause it. Perhaps in
 > > tablet_server-stress-test or delete_tablet-itest we could add
 > > something more specific to this issue?
 >
 > If I enable those modifications, the test fails because of the
 > assert in TestWorkload.  There is specific TODO in
 > raft_consensus_stress-itest.  Once the issue with that addressed
 > (via retries in TestWorkload or via leader replicas), I'm going to
 > enable that additional reader thread.

'leader replicas' --> 'leader leases'


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4b8cc2a307e5a0bd6019bd155f33c1565fca513
Gerrit-Change-Number: 9574
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 12 Mar 2018 23:49:29 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2322: don't spew logs about behind-log-GC follower

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9573 )

Change subject: KUDU-2322: don't spew logs about behind-log-GC follower
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9573/1/src/kudu/consensus/consensus_queue.cc@652
PS1, Line 652: KLOG_EVERY_N_SECS_THROTTLER(INFO, 60, 
peer_copy.status_log_throttler, "logs_gced")
I'm a little bit concerned about not printing this message for every peer that 
hits this, at least the first time. Do you know of a way we can throttle this 
message on a unique peer basis?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4438e63aef0be26c149c57f9f58cb54327e41a33
Gerrit-Change-Number: 9573
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 12 Mar 2018 23:48:53 +
Gerrit-HasComments: Yes

[kudu-CR] [doc] Document the new decimal column type

[Todd Lipcon (Code Review)]

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

Change subject: [doc] Document the new decimal column type
..


Patch Set 4:

Looks like this needs a rebase


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9489613d35daad708648ea04d49e472d3149b33d
Gerrit-Change-Number: 9432
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 12 Mar 2018 23:47:51 +
Gerrit-HasComments: No

[kudu-CR] condition variable: rework timed waiting mechanisms and associated cleanup

[Todd Lipcon (Code Review)]

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

Change subject: condition_variable: rework timed waiting mechanisms and 
associated cleanup
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2d3d0d9d84c3b1a76f3efc8ae706ddcaa455630
Gerrit-Change-Number: 9572
Gerrit-PatchSet: 3
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 12 Mar 2018 23:47:05 +
Gerrit-HasComments: No

[kudu-CR] condition variable: rework timed waiting mechanisms and associated cleanup

[Todd Lipcon (Code Review)]

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

Change subject: condition_variable: rework timed waiting mechanisms and 
associated cleanup
..

condition_variable: rework timed waiting mechanisms and associated cleanup

This patch reworks ConditionVariable::TimedWait into two new methods:
1. WaitFor(delta), equivalent to TimedWait.
2. WaitUntil(deadline), waits for a deadline to elapse.

Besides the improved ergonomics of using WaitUntil() when all one has on
hand is a deadline, WaitUntil() also yields more precise timeouts when
dealing with spurious wakeups while waiting in a loop; the WaitFor methods
in CountDownLatch and ThreadPool stand to benefit from this.

I also cleaned up some dead portability code in condition_variable.cc.

Change-Id: Ie2d3d0d9d84c3b1a76f3efc8ae706ddcaa455630
Reviewed-on: http://gerrit.cloudera.org:8080/9572
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M src/kudu/master/catalog_manager.cc
M src/kudu/server/diagnostics_log.cc
M src/kudu/tserver/heartbeater.cc
M src/kudu/tserver/scanners.cc
M src/kudu/util/async_logger.cc
M src/kudu/util/blocking_queue.h
M src/kudu/util/condition_variable.cc
M src/kudu/util/condition_variable.h
M src/kudu/util/countdown_latch.h
M src/kudu/util/maintenance_manager.cc
M src/kudu/util/monotime.cc
M src/kudu/util/monotime.h
M src/kudu/util/pstack_watcher.cc
M src/kudu/util/threadpool.cc
14 files changed, 90 insertions(+), 93 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie2d3d0d9d84c3b1a76f3efc8ae706ddcaa455630
Gerrit-Change-Number: 9572
Gerrit-PatchSet: 4
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] release notes: update dirs and fs metadata dir

[Andrew Wong (Code Review)]

Andrew Wong has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9581 )

Change subject: release notes: update_dirs and fs_metadata_dir
..

release notes: update_dirs and fs_metadata_dir

This patch adds release notes for support of data dir removal via the
`kudu fs update_dirs` tool and for the configuration of the metadata
directory.

Note: These blurbs are taken from the configuration docs.

Change-Id: I87f0857c3e7d9d98865fbb50e5e69b78e03c6fe2
Reviewed-on: http://gerrit.cloudera.org:8080/9581
Reviewed-by: Adar Dembo 
Reviewed-by: Alexey Serbin 
Tested-by: Andrew Wong 
---
M docs/release_notes.adoc
1 file changed, 14 insertions(+), 0 deletions(-)

Approvals:
  Adar Dembo: Looks good to me, approved
  Alexey Serbin: Looks good to me, but someone else must approve
  Andrew Wong: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I87f0857c3e7d9d98865fbb50e5e69b78e03c6fe2
Gerrit-Change-Number: 9581
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] release notes: update dirs and fs metadata dir

[Andrew Wong (Code Review)]

Andrew Wong has removed a vote on this change.

Change subject: release notes: update_dirs and fs_metadata_dir
..


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I87f0857c3e7d9d98865fbb50e5e69b78e03c6fe2
Gerrit-Change-Number: 9581
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] release notes: update dirs and fs metadata dir

[Andrew Wong (Code Review)]

Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9581 )

Change subject: release notes: update_dirs and fs_metadata_dir
..


Patch Set 3: Verified+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I87f0857c3e7d9d98865fbb50e5e69b78e03c6fe2
Gerrit-Change-Number: 9581
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 12 Mar 2018 23:40:22 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2295 fix nullptr dereference in Tablet

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9574 )

Change subject: KUDU-2295 fix nullptr dereference in Tablet
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4b8cc2a307e5a0bd6019bd155f33c1565fca513
Gerrit-Change-Number: 9574
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 12 Mar 2018 23:38:35 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2295 fix nullptr dereference in Tablet

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9574 )

Change subject: KUDU-2295 fix nullptr dereference in Tablet
..


Patch Set 2:

I agree with Todd that it would be good to have a test for this. But this patch 
looks good to me.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4b8cc2a307e5a0bd6019bd155f33c1565fca513
Gerrit-Change-Number: 9574
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 12 Mar 2018 23:38:50 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2295 fix nullptr dereference in Tablet

[Alexey Serbin (Code Review)]

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

Change subject: KUDU-2295 fix nullptr dereference in Tablet
..


Patch Set 2:

> Would be nice to have a test for this in the codebase, since it
 > sounds like you had to make some local modifications to
 > raft_consensus-stress-itest to cause it. Perhaps in
 > tablet_server-stress-test or delete_tablet-itest we could add
 > something more specific to this issue?

If I enable those modifications, the test fails because of the assert in 
TestWorkload.  There is specific TODO in raft_consensus_stress-itest.  Once the 
issue with that addressed (via retries in TestWorkload or via leader replicas), 
I'm going to enable that additional reader thread.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4b8cc2a307e5a0bd6019bd155f33c1565fca513
Gerrit-Change-Number: 9574
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 12 Mar 2018 23:37:50 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2334: Fix OutboundTransfer::TransferStarted() to work with SSL write()

[Sailesh Mukil (Code Review)]

Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9587 )

Change subject: KUDU-2334: Fix OutboundTransfer::TransferStarted() to work with 
SSL_write()
..


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/9587/1/src/kudu/rpc/transfer.h
File src/kudu/rpc/transfer.h:

http://gerrit.cloudera.org:8080/#/c/9587/1/src/kudu/rpc/transfer.h@186
PS1, Line 186: True if we have tried sending anything in 'payload_slices_'
Actually isn't it more like "True if SendBuffer() is called at least once." ?

The Writev() function can return without attempting to send anything in some 
cases.


http://gerrit.cloudera.org:8080/#/c/9587/1/src/kudu/rpc/transfer.h@187
PS1, Line 187:   // no bytes were sent successfully. This is needed as 
SSL_write() is stateful.
nit: Add a reference to this JIRA, else it's hard to understand why we added it.


http://gerrit.cloudera.org:8080/#/c/9587/1/src/kudu/rpc/transfer.cc
File src/kudu/rpc/transfer.cc:

http://gerrit.cloudera.org:8080/#/c/9587/1/src/kudu/rpc/transfer.cc@190
PS1, Line 190:   started_ = true;
Do we know of any cases where setting started_ = true this early can backfire? 
I looked at the code and it seems fine to me, but I just want to make sure.

In some cases, we won't send anything at all:
https://github.com/apache/kudu/blob/master/src/kudu/security/tls_socket.cc#L50-L55

Or we could hit errors before actually calling the SSL_write() or send() 
functions.

So we should make sure that having started_ = true for these cases doesn't 
cause a problem elsewhere in the code.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id7ebdcbc1ef2a3e0c5e7162f03214c232755b683
Gerrit-Change-Number: 9587
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 12 Mar 2018 23:36:34 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2295 fix nullptr dereference in Tablet

[Todd Lipcon (Code Review)]

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

Change subject: KUDU-2295 fix nullptr dereference in Tablet
..


Patch Set 2:

Would be nice to have a test for this in the codebase, since it sounds like you 
had to make some local modifications to raft_consensus-stress-itest to cause 
it. Perhaps in tablet_server-stress-test or delete_tablet-itest we could add 
something more specific to this issue?


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4b8cc2a307e5a0bd6019bd155f33c1565fca513
Gerrit-Change-Number: 9574
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 12 Mar 2018 23:30:30 +
Gerrit-HasComments: No

[kudu-CR] Fix tests leaving behind diagnostics logs

[Todd Lipcon (Code Review)]

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

Change subject: Fix tests leaving behind diagnostics logs
..

Fix tests leaving behind diagnostics logs

Prior to this patch, if tests are run with TEST_TMPDIR set, tests which
use an internal minicluster could leave behind a diagnostics log file.
The issue was the following:

- GLog has some code which defaults FLAGS_log_dir to the the
  $TEST_TMPDIR environment variable.
- Since we enabled the diagnosics log by default, this meant that
  minicluster servers would start logging into $TEST_TMPDIR directly
- Our test harness only takes care of cleaning up the test-case-specific
  directory rather than the top-level $TEST_TMPDIR

The fix here is to make our own test harness override FLAGS_log_dir to
point to the test-specific directory.

Tested by setting TEST_TMPDIR and running tablet_server-test. Before the
patch, it left some files behind. With the patch, it did not. I couldn't
write an easy automated test since the initialization of FLAGS_log_dir
happens during glog's static constructors.

Change-Id: Ic68826f652c7aefce1314d9e3481b11666f2699f
Reviewed-on: http://gerrit.cloudera.org:8080/9592
Reviewed-by: Adar Dembo 
Tested-by: Todd Lipcon 
---
M src/kudu/master/master-test.cc
M src/kudu/util/test_util.cc
2 files changed, 10 insertions(+), 1 deletion(-)

Approvals:
  Adar Dembo: Looks good to me, approved
  Todd Lipcon: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic68826f652c7aefce1314d9e3481b11666f2699f
Gerrit-Change-Number: 9592
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Fix tests leaving behind diagnostics logs

[Todd Lipcon (Code Review)]

Todd Lipcon has removed a vote on this change.

Change subject: Fix tests leaving behind diagnostics logs
..


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ic68826f652c7aefce1314d9e3481b11666f2699f
Gerrit-Change-Number: 9592
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] Fix tests leaving behind diagnostics logs

[Todd Lipcon (Code Review)]

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

Change subject: Fix tests leaving behind diagnostics logs
..


Patch Set 1: Verified+1

Unrelated tablet_copy flake


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic68826f652c7aefce1314d9e3481b11666f2699f
Gerrit-Change-Number: 9592
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 12 Mar 2018 23:28:10 +
Gerrit-HasComments: No

[kudu-CR] release notes: update dirs and fs metadata dir

[Alexey Serbin (Code Review)]

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

Change subject: release notes: update_dirs and fs_metadata_dir
..


Patch Set 3: Code-Review+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I87f0857c3e7d9d98865fbb50e5e69b78e03c6fe2
Gerrit-Change-Number: 9581
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 12 Mar 2018 23:24:30 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2336. diagnostics log: fix an assertion failure for null stack frames

[Todd Lipcon (Code Review)]

Hello Kudu Jenkins, Adar Dembo,

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

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

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

Change subject: KUDU-2336. diagnostics_log: fix an assertion failure for null 
stack frames
..

KUDU-2336. diagnostics_log: fix an assertion failure for null stack frames

On my Ubuntu 16 box, in ASAN builds, it seems I sometimes get a stack
frame with a nullptr address. This caused SymbolSet::Add(...) to fail an
assertion since it uses google::dense_hash_set<> which requires that
there be one key reserved as a non-insertable "empty" marker. We were
using nullptr for that but not checking before potentially inserting it.

This adds a simple workaround. Without the patch, if I started a tserver
with --diagnostics-log-stack-traces-interval-ms=10, it would crash about
10% of the time with the following assertion failure:

kudu-master: 
../../thirdparty/installed/common/include/sparsehash/internal/densehashtable.h:969:
  std::pair google::dense_hashtable<...>...:
  Assertion `!equals(std::forward(key), key_info.empty_key) && "Inserting 
the empty key"' failed.

Change-Id: I73ada54bef056b665c03ca142a3995ae6ad59230
---
M src/kudu/server/diagnostics_log.cc
1 file changed, 4 insertions(+), 1 deletion(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I73ada54bef056b665c03ca142a3995ae6ad59230
Gerrit-Change-Number: 9591
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] diagnostics log: fix an assertion failure for null stack frames

[Todd Lipcon (Code Review)]

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

Change subject: diagnostics_log: fix an assertion failure for null stack frames
..


Patch Set 1:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/9591/1//COMMIT_MSG@7
PS1, Line 7: diagnostics_log: fix an assertion failure for null stack frames
> You can tag KUDU-2336 here.
ah I missed that this was filed. Done.


http://gerrit.cloudera.org:8080/#/c/9591/1//COMMIT_MSG@20
PS1, Line 20: kudu-master: 
../../thirdparty/installed/common/include/sparsehash/internal/densehashtable.h:969:
 std::pair google::dense_hashtable, google::dense_hash_set, 
std::equal_to, google::libc_allocator_with_realloc >::Identity, 
google::dense_hash_set, std::equal_to, 
google::libc_allocator_with_realloc >::SetKey, std::equal_to, 
google::libc_allocator_with_realloc >::insert_noresize(K &&, Args 
&&...) [Value = void *, Key = void *, HashFcn = std::hash, ExtractKey = 
google::dense_hash_set, std::equal_to, 
google::libc_allocator_with_realloc >::Identity, SetKey = 
google::dense_hash_set, std::equal_to, 
google::libc_allocator_with_realloc >::SetKey, EqualKey = 
std::equal_to, Alloc = google::libc_allocator_with_realloc, K = 
void *const &, Args = ]: Assertion 
`!equals(std::forward(key), key_info.empty_key) && "Inserting the empty 
key"' failed.
> Nit: could you move this be to about Change-Id? And maybe reformat/elide so
ah, git hook gone awry



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I73ada54bef056b665c03ca142a3995ae6ad59230
Gerrit-Change-Number: 9591
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 12 Mar 2018 23:22:33 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2230: java: sync client exception stack traces should make sense

[Todd Lipcon (Code Review)]

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

Change subject: KUDU-2230: java: sync client exception stack traces should make 
sense
..


Patch Set 2:

> Patch Set 2:
>
> > I was thinking that "suppressed" more accurately conveys that it's probably 
> > irrelevant to what the user should look at, vs a normal "cause". Happy to 
> > use cause if y'all think it is less strange.
>
> With "cause", do the stack traces become totally unwieldy?

Actually I just remembered why I didn't switch the cause. The cause of an 
exception can only be set once using 'initCause' and not replaced later. So, to 
use 'cause', we'd need to instantiate a new KuduException in the 
transformException function.

In order to do so, we'd have two options:

a) use some reflection to figure out which KuduException subclass we had and 
try to re-instantiate the same one. That seems ugly.
b) use clone() and make sure that all KuduException subclasses are Cloneable. 
Also kind of a pain, no?


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I874d47b5239bcc8493c15ef763ecfe0fd878d795
Gerrit-Change-Number: 9489
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
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-Comment-Date: Mon, 12 Mar 2018 23:20:46 +
Gerrit-HasComments: No

[kudu-CR] release notes: update dirs and fs metadata dir

[Adar Dembo (Code Review)]

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

Change subject: release notes: update_dirs and fs_metadata_dir
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I87f0857c3e7d9d98865fbb50e5e69b78e03c6fe2
Gerrit-Change-Number: 9581
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 12 Mar 2018 23:10:16 +
Gerrit-HasComments: No

[kudu-CR] [tools] KUDU-2331: Allow move tool to work when uninvolved tserver is down

[Adar Dembo (Code Review)]

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

Change subject: [tools] KUDU-2331: Allow move tool to work when uninvolved 
tserver is down
..


Patch Set 5:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/9510/5/src/kudu/tools/kudu-admin-test.cc@278
PS5, Line 278:   // To test that the move fails if any peer is down, when 
downTS is
 :   // 'TabletPeer', shut down a server besides 'add' that hosts a 
replica.
 :   if (downTS == DownTS::TabletPeer) {
 : 
NO_FATALS(cluster_->tablet_server_by_uuid(active_tservers.back())->Shutdown());
 :   }
 :   // Regression case for KUDU-2331, where move_replica would 
fail if any tablet
 :   // server is down, even if that tablet server was not involved 
in the move.
 :   if (downTS == DownTS::UninvolvedTS) {
 : 
NO_FATALS(cluster_->tablet_server_by_uuid(inactive_tservers.back())->Shutdown());
 :   }
Nit: can be else-if, or a switch.


http://gerrit.cloudera.org:8080/#/c/9510/5/src/kudu/tools/kudu-admin-test.cc@309
PS5, Line 309:   SUCCEED();
What does this do? What happens if you just returned?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id1fc60233f4f50f478da7ccb104e37df3067400c
Gerrit-Change-Number: 9510
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Mon, 12 Mar 2018 23:09:19 +
Gerrit-HasComments: Yes

[kudu-CR] release notes: update dirs and fs metadata dir

[Andrew Wong (Code Review)]

Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, Grant Henke,

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

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

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

Change subject: release notes: update_dirs and fs_metadata_dir
..

release notes: update_dirs and fs_metadata_dir

This patch adds release notes for support of data dir removal via the
`kudu fs update_dirs` tool and for the configuration of the metadata
directory.

Note: These blurbs are taken from the configuration docs.

Change-Id: I87f0857c3e7d9d98865fbb50e5e69b78e03c6fe2
---
M docs/release_notes.adoc
1 file changed, 14 insertions(+), 0 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I87f0857c3e7d9d98865fbb50e5e69b78e03c6fe2
Gerrit-Change-Number: 9581
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] release notes: update dirs and fs metadata dir

[Andrew Wong (Code Review)]

Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9581 )

Change subject: release notes: update_dirs and fs_metadata_dir
..


Patch Set 3:

(4 comments)

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

http://gerrit.cloudera.org:8080/#/c/9581/2//COMMIT_MSG@14
PS2, Line 14:
> Nit: Change-Id is usually last.
Done


http://gerrit.cloudera.org:8080/#/c/9581/2/docs/release_notes.adoc
File docs/release_notes.adoc:

http://gerrit.cloudera.org:8080/#/c/9581/2/docs/release_notes.adoc@55
PS2, Line 55: tablet
> tablet replicas
Done


http://gerrit.cloudera.org:8080/#/c/9581/2/docs/release_notes.adoc@56
PS2, Line 56: ail upo
> starting
Done


http://gerrit.cloudera.org:8080/#/c/9581/2/docs/release_notes.adoc@59
PS2, Line 59: Users can use the new `--fs_metadata_dir` to specify the 
directory in which
:   to place tablet-specific metadata.
> What happens when --fs_metadata_dir is specified for 1.7 tserver, where alr
We don't try moving metadata at all. If a 1.7 server with the default metadata 
configuration sees metadata in the first data dir, it will prefer using that. 
If it doesn't see metadata in the first data dir, it will check the wal dir. In 
that way, we keep backwards compatibility with pre-1.7 servers that don't have 
this flag.

And if the user tries to specify a custom metadata dir, outside of either of 
these, when we are expecting there to be existing metadata, Kudu will fail to 
start up.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I87f0857c3e7d9d98865fbb50e5e69b78e03c6fe2
Gerrit-Change-Number: 9581
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 12 Mar 2018 23:07:57 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2230: java: sync client exception stack traces should make sense

[Adar Dembo (Code Review)]

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

Change subject: KUDU-2230: java: sync client exception stack traces should make 
sense
..


Patch Set 2:

> I was thinking that "suppressed" more accurately conveys that it's probably 
> irrelevant to what the user should look at, vs a normal "cause". Happy to use 
> cause if y'all think it is less strange.

With "cause", do the stack traces become totally unwieldy?


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I874d47b5239bcc8493c15ef763ecfe0fd878d795
Gerrit-Change-Number: 9489
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
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-Comment-Date: Mon, 12 Mar 2018 23:05:00 +
Gerrit-HasComments: No

[kudu-CR] Fix tests leaving behind diagnostics logs

[Adar Dembo (Code Review)]

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

Change subject: Fix tests leaving behind diagnostics logs
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic68826f652c7aefce1314d9e3481b11666f2699f
Gerrit-Change-Number: 9592
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 12 Mar 2018 23:03:51 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2230: java: sync client exception stack traces should make sense

[Todd Lipcon (Code Review)]

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

Change subject: KUDU-2230: java: sync client exception stack traces should make 
sense
..


Patch Set 2:

> Patch Set 2:
>
> Why did you choose to model the original exception as suppressed rather than 
> as the cause of the new exception? From my reading of suppressed exceptions, 
> it seems like it's used when there's no causal link between two exceptions 
> (e.g. a second exception thrown while cleaning up in a try-with-resources 
> block). But in this case there is a causal link.
>
> Dan was also curious about this (he asked the same thing on Slack).

I was thinking that "suppressed" more accurately conveys that it's probably 
irrelevant to what the user should look at, vs a normal "cause". Happy to use 
cause if y'all think it is less strange.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I874d47b5239bcc8493c15ef763ecfe0fd878d795
Gerrit-Change-Number: 9489
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
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-Comment-Date: Mon, 12 Mar 2018 23:02:44 +
Gerrit-HasComments: No

[kudu-CR] diagnostics log: fix an assertion failure for null stack frames

[Adar Dembo (Code Review)]

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

Change subject: diagnostics_log: fix an assertion failure for null stack frames
..


Patch Set 1:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/9591/1//COMMIT_MSG@7
PS1, Line 7: diagnostics_log: fix an assertion failure for null stack frames
You can tag KUDU-2336 here.


http://gerrit.cloudera.org:8080/#/c/9591/1//COMMIT_MSG@20
PS1, Line 20: kudu-master: 
../../thirdparty/installed/common/include/sparsehash/internal/densehashtable.h:969:
 std::pair google::dense_hashtable, google::dense_hash_set, 
std::equal_to, google::libc_allocator_with_realloc >::Identity, 
google::dense_hash_set, std::equal_to, 
google::libc_allocator_with_realloc >::SetKey, std::equal_to, 
google::libc_allocator_with_realloc >::insert_noresize(K &&, Args 
&&...) [Value = void *, Key = void *, HashFcn = std::hash, ExtractKey = 
google::dense_hash_set, std::equal_to, 
google::libc_allocator_with_realloc >::Identity, SetKey = 
google::dense_hash_set, std::equal_to, 
google::libc_allocator_with_realloc >::SetKey, EqualKey = 
std::equal_to, Alloc = google::libc_allocator_with_realloc, K = 
void *const &, Args = ]: Assertion 
`!equals(std::forward(key), key_info.empty_key) && "Inserting the empty 
key"' failed.
Nit: could you move this be to about Change-Id? And maybe reformat/elide some 
of the templating to make the error more clear?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I73ada54bef056b665c03ca142a3995ae6ad59230
Gerrit-Change-Number: 9591
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 12 Mar 2018 23:00:02 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2334: Fix OutboundTransfer::TransferStarted() to work with SSL write()

[Todd Lipcon (Code Review)]

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

Change subject: KUDU-2334: Fix OutboundTransfer::TransferStarted() to work with 
SSL_write()
..


Patch Set 1:

(8 comments)

> Patch Set 1:
>
> the test failure seems to be infrastructure issue ?
>
> tablet-decoder-eval-test.0dist-test-slave-dist-test-slave-p2pc-2  
> Nonefailed to download task files: WARNING 100 
> isolateserver(1484): Adding unknown file 9a0053f2a34

yea that's odd, agree it's unrelated.

http://gerrit.cloudera.org:8080/#/c/9587/1/src/kudu/rpc/rpc-test-base.h
File src/kudu/rpc/rpc-test-base.h:

http://gerrit.cloudera.org:8080/#/c/9587/1/src/kudu/rpc/rpc-test-base.h@441
PS1, Line 441:   FLAGS_rpc_encrypt_loopback_connections = true;
ah, does this mean we were not successfully testing encrypted RPCs in the past 
with this test? :(


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

http://gerrit.cloudera.org:8080/#/c/9587/1/src/kudu/rpc/rpc-test.cc@740
PS1, Line 740:// Linux, SSL enabled
hmm, is this fix related to the patch in question? I'm surprised we haven't 
seen this failure on our flaky test dashboard


http://gerrit.cloudera.org:8080/#/c/9587/1/src/kudu/rpc/rpc-test.cc@1223
PS1, Line 1223: // Helper function for TestCancellationMultiThreads.
better to document briefly what it does, rather than what it is.


http://gerrit.cloudera.org:8080/#/c/9587/1/src/kudu/rpc/rpc-test.cc@1224
PS1, Line 1224: SendRpcAndCancel
this should probably be named SendAndCancelRpcs or something, since it sends 
and cancels more than one of them


http://gerrit.cloudera.org:8080/#/c/9587/1/src/kudu/rpc/rpc-test.cc@1251
PS1, Line 1251: ASSERT_TRUE
have to use CHECK here since this isn't the main thread and gtest isn't 
thread-safe


http://gerrit.cloudera.org:8080/#/c/9587/1/src/kudu/rpc/rpc-test.cc@1256
PS1, Line 1256: #define BUF_SIZE (16 * 1024 * 1024)
can you use a const int defined below within the function instead?  We prefer 
'const int kBufferSize' vs a #define


http://gerrit.cloudera.org:8080/#/c/9587/1/src/kudu/rpc/rpc-test.cc@1258
PS1, Line 1258: Regression
did this test successfully repro the bug w/o the fix?


http://gerrit.cloudera.org:8080/#/c/9587/1/src/kudu/rpc/rpc-test.cc@1274
PS1, Line 1274:   unique_ptr buf(new uint8_t[BUF_SIZE]);
  :   memset(buf.get(), 'a', BUF_SIZE);
  :   Slice slice(buf.get(), BUF_SIZE);
you could probably get away with just:

  string buf('a', kBufferSize);

and then down below pass Slice(buf)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id7ebdcbc1ef2a3e0c5e7162f03214c232755b683
Gerrit-Change-Number: 9587
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 12 Mar 2018 22:59:10 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2230: java: sync client exception stack traces should make sense

[Adar Dembo (Code Review)]

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

Change subject: KUDU-2230: java: sync client exception stack traces should make 
sense
..


Patch Set 2:

Why did you choose to model the original exception as suppressed rather than as 
the cause of the new exception? From my reading of suppressed exceptions, it 
seems like it's used when there's no causal link between two exceptions (e.g. a 
second exception thrown while cleaning up in a try-with-resources block). But 
in this case there is a causal link.

Dan was also curious about this (he asked the same thing on Slack).


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I874d47b5239bcc8493c15ef763ecfe0fd878d795
Gerrit-Change-Number: 9489
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
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-Comment-Date: Mon, 12 Mar 2018 22:55:51 +
Gerrit-HasComments: No

[kudu-CR] diagnostics log: fix an assertion failure for null stack frames

[Todd Lipcon (Code Review)]

Hello Adar Dembo,

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

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

to review the following change.


Change subject: diagnostics_log: fix an assertion failure for null stack frames
..

diagnostics_log: fix an assertion failure for null stack frames

On my Ubuntu 16 box, in ASAN builds, it seems I sometimes get a stack
frame with a nullptr address. This caused SymbolSet::Add(...) to fail an
assertion since it uses google::dense_hash_set<> which requires that
there be one key reserved as a non-insertable "empty" marker. We were
using nullptr for that but not checking before potentially inserting it.

This adds a simple workaround. Without the patch, if I started a tserver
with --diagnostics-log-stack-traces-interval-ms=10, it would crash about
10% of the time with the following assertion failure:

Change-Id: I73ada54bef056b665c03ca142a3995ae6ad59230
kudu-master: 
../../thirdparty/installed/common/include/sparsehash/internal/densehashtable.h:969:
 std::pair google::dense_hashtable, google::dense_hash_set, 
std::equal_to, google::libc_allocator_with_realloc >::Identity, 
google::dense_hash_set, std::equal_to, 
google::libc_allocator_with_realloc >::SetKey, std::equal_to, 
google::libc_allocator_with_realloc >::insert_noresize(K &&, Args 
&&...) [Value = void *, Key = void *, HashFcn = std::hash, ExtractKey = 
google::dense_hash_set, std::equal_to, 
google::libc_allocator_with_realloc >::Identity, SetKey = 
google::dense_hash_set, std::equal_to, 
google::libc_allocator_with_realloc >::SetKey, EqualKey = 
std::equal_to, Alloc = google::libc_allocator_with_realloc, K = 
void *const &, Args = ]: Assertion 
`!equals(std::forward(key), key_info.empty_key) && "Inserting the empty 
key"' failed.
---
M src/kudu/server/diagnostics_log.cc
1 file changed, 4 insertions(+), 1 deletion(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I73ada54bef056b665c03ca142a3995ae6ad59230
Gerrit-Change-Number: 9591
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 

[kudu-CR] Fix tests leaving behind diagnostics logs

[Todd Lipcon (Code Review)]

Hello Adar Dembo,

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

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

to review the following change.


Change subject: Fix tests leaving behind diagnostics logs
..

Fix tests leaving behind diagnostics logs

Prior to this patch, if tests are run with TEST_TMPDIR set, tests which
use an internal minicluster could leave behind a diagnostics log file.
The issue was the following:

- GLog has some code which defaults FLAGS_log_dir to the the
  $TEST_TMPDIR environment variable.
- Since we enabled the diagnosics log by default, this meant that
  minicluster servers would start logging into $TEST_TMPDIR directly
- Our test harness only takes care of cleaning up the test-case-specific
  directory rather than the top-level $TEST_TMPDIR

The fix here is to make our own test harness override FLAGS_log_dir to
point to the test-specific directory.

Tested by setting TEST_TMPDIR and running tablet_server-test. Before the
patch, it left some files behind. With the patch, it did not. I couldn't
write an easy automated test since the initialization of FLAGS_log_dir
happens during glog's static constructors.

Change-Id: Ic68826f652c7aefce1314d9e3481b11666f2699f
---
M src/kudu/master/master-test.cc
M src/kudu/util/test_util.cc
2 files changed, 10 insertions(+), 1 deletion(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic68826f652c7aefce1314d9e3481b11666f2699f
Gerrit-Change-Number: 9592
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 

[kudu-CR] release notes: update dirs and fs metadata dir

[Alexey Serbin (Code Review)]

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

Change subject: release notes: update_dirs and fs_metadata_dir
..


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/9581/2/docs/release_notes.adoc
File docs/release_notes.adoc:

http://gerrit.cloudera.org:8080/#/c/9581/2/docs/release_notes.adoc@55
PS2, Line 55: tablets
tablet replicas

BTW, a non-replicated tablet is not going to be re-replicated elsewhere, right?


http://gerrit.cloudera.org:8080/#/c/9581/2/docs/release_notes.adoc@56
PS2, Line 56: startin
starting


http://gerrit.cloudera.org:8080/#/c/9581/2/docs/release_notes.adoc@59
PS2, Line 59: Users can use the new `--fs_metadata_dir` to specify the 
directory in which
:   to place tablet-specific metadata.
What happens when --fs_metadata_dir is specified for 1.7 tserver, where already 
existing metadata for tablets are stored under --fs_wal_dir?  Would the old 
metadata be copied over to the new location and the old metadata removed?  If 
not the latter, what would happen if after some time, running with 1.7 tserver, 
user will try to run 1.6 tserver again?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I87f0857c3e7d9d98865fbb50e5e69b78e03c6fe2
Gerrit-Change-Number: 9581
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 12 Mar 2018 22:33:09 +
Gerrit-HasComments: Yes

[kudu-CR] release notes: update dirs and fs metadata dir

[Adar Dembo (Code Review)]

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

Change subject: release notes: update_dirs and fs_metadata_dir
..


Patch Set 2: Code-Review+2

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9581/2//COMMIT_MSG@14
PS2, Line 14: Note: These blurbs are taken from the configuration docs.
Nit: Change-Id is usually last.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I87f0857c3e7d9d98865fbb50e5e69b78e03c6fe2
Gerrit-Change-Number: 9581
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 12 Mar 2018 22:28:46 +
Gerrit-HasComments: Yes

[kudu-CR] build: don't name unsharded tests with a shard suffix

[Adar Dembo (Code Review)]

Adar Dembo has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9524 )

Change subject: build: don't name unsharded tests with a shard suffix
..

build: don't name unsharded tests with a shard suffix

Change-Id: I33611f9766381d367c7a5ac7b09a00f01c48fe74
Reviewed-on: http://gerrit.cloudera.org:8080/9524
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo 
---
M CMakeLists.txt
M build-support/run-test.sh
2 files changed, 12 insertions(+), 3 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I33611f9766381d367c7a5ac7b09a00f01c48fe74
Gerrit-Change-Number: 9524
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] build: don't name unsharded tests with a shard suffix

[Adar Dembo (Code Review)]

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

Change subject: build: don't name unsharded tests with a shard suffix
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I33611f9766381d367c7a5ac7b09a00f01c48fe74
Gerrit-Change-Number: 9524
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 12 Mar 2018 22:26:30 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2295 fix nullptr dereference in Tablet

[Adar Dembo (Code Review)]

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

Change subject: KUDU-2295 fix nullptr dereference in Tablet
..


Patch Set 2: Code-Review+1

Looks OK to me but would prefer another review since I'm not as familiar with 
this.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4b8cc2a307e5a0bd6019bd155f33c1565fca513
Gerrit-Change-Number: 9574
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 12 Mar 2018 22:25:53 +
Gerrit-HasComments: No

[kudu-CR] condition variable: rework timed waiting mechanisms and associated cleanup

[Adar Dembo (Code Review)]

Hello Kudu Jenkins, Todd Lipcon,

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

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

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

Change subject: condition_variable: rework timed waiting mechanisms and 
associated cleanup
..

condition_variable: rework timed waiting mechanisms and associated cleanup

This patch reworks ConditionVariable::TimedWait into two new methods:
1. WaitFor(delta), equivalent to TimedWait.
2. WaitUntil(deadline), waits for a deadline to elapse.

Besides the improved ergonomics of using WaitUntil() when all one has on
hand is a deadline, WaitUntil() also yields more precise timeouts when
dealing with spurious wakeups while waiting in a loop; the WaitFor methods
in CountDownLatch and ThreadPool stand to benefit from this.

I also cleaned up some dead portability code in condition_variable.cc.

Change-Id: Ie2d3d0d9d84c3b1a76f3efc8ae706ddcaa455630
---
M src/kudu/master/catalog_manager.cc
M src/kudu/server/diagnostics_log.cc
M src/kudu/tserver/heartbeater.cc
M src/kudu/tserver/scanners.cc
M src/kudu/util/async_logger.cc
M src/kudu/util/blocking_queue.h
M src/kudu/util/condition_variable.cc
M src/kudu/util/condition_variable.h
M src/kudu/util/countdown_latch.h
M src/kudu/util/maintenance_manager.cc
M src/kudu/util/monotime.cc
M src/kudu/util/monotime.h
M src/kudu/util/pstack_watcher.cc
M src/kudu/util/threadpool.cc
14 files changed, 90 insertions(+), 93 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie2d3d0d9d84c3b1a76f3efc8ae706ddcaa455630
Gerrit-Change-Number: 9572
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2334: Fix OutboundTransfer::TransferStarted() to work with SSL write()

[Michael Ho (Code Review)]

Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9587 )

Change subject: KUDU-2334: Fix OutboundTransfer::TransferStarted() to work with 
SSL_write()
..


Patch Set 1:

the test failure seems to be infrastructure issue ?

tablet-decoder-eval-test.0  dist-test-slave-dist-test-slave-p2pc-2  
Nonefailed to download task files: WARNING 100 isolateserver(1484): 
Adding unknown file 9a0053f2a34


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id7ebdcbc1ef2a3e0c5e7162f03214c232755b683
Gerrit-Change-Number: 9587
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 12 Mar 2018 21:23:21 +
Gerrit-HasComments: No

[kudu-CR] [tools] KUDU-2331: Allow move tool to work when uninvolved tserver is down

[Alexey Serbin (Code Review)]

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

Change subject: [tools] KUDU-2331: Allow move tool to work when uninvolved 
tserver is down
..


Patch Set 5:

(3 comments)

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

http://gerrit.cloudera.org:8080/#/c/9510/5/src/kudu/tools/kudu-admin-test.cc@246
PS5, Line 246: ts_flags = master_flags = { 
"--raft_prepare_replacement_before_eviction=true" };
It seems it's not a direct change of this patch, but while you are at it.

To make this piece independent on what replica scheme is used by default, 
consider replacing this conditional piece with

..._flags = { Substitute("--raft_prepare_replacement_before_eviction=$0", 
(enable_kudu_1097 == Kudu1097::Enable)) };


http://gerrit.cloudera.org:8080/#/c/9510/5/src/kudu/tools/kudu-admin-test.cc@291
PS5, Line 291: AllowSlowTests() && downTS == DownTS::None
nit: maybe, add parenthesis for better readability?


http://gerrit.cloudera.org:8080/#/c/9510/5/src/kudu/tools/kudu-admin-test.cc@307
PS5, Line 307: ASSERT_TRUE(s.IsRuntimeError());
Does it make sense to narrow this down and check for some specific error 
message?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id1fc60233f4f50f478da7ccb104e37df3067400c
Gerrit-Change-Number: 9510
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Mon, 12 Mar 2018 21:21:16 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode

[Hao Hao (Code Review)]

Hao Hao has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/8847 )

Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode
..

KUDU-1704: add java client support for READ_YOUR_WRITES mode

Change-Id: I6239521c022147257859e399f55c6f3f945af465
Reviewed-on: http://gerrit.cloudera.org:8080/8847
Tested-by: Kudu Jenkins
Reviewed-by: David Ribeiro Alves 
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/TestScannerMultiTablet.java
4 files changed, 236 insertions(+), 7 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465
Gerrit-Change-Number: 8847
Gerrit-PatchSet: 11
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [release notes] replica management scheme notes

[Alexey Serbin (Code Review)]

Hello Will Berkeley, Mike Percy, Dan Burkert, Jean-Daniel Cryans, Kudu Jenkins, 
Grant Henke, Todd Lipcon,

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

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

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

Change subject: [release_notes] replica management scheme notes
..

[release_notes] replica management scheme notes

Added relevant notes on the new replica management scheme used
in Kudu 1.7 by default:
  * the new replica management scheme is incompatible with old one
  * rolling upgrade 1.6 -> 1.7 is not possible

Change-Id: I49f1f1e17cdaee272592d598431a33dbfe55123f
---
M docs/release_notes.adoc
1 file changed, 37 insertions(+), 2 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I49f1f1e17cdaee272592d598431a33dbfe55123f
Gerrit-Change-Number: 9571
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8847 )

Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode
..


Patch Set 10:

as discussed offline, ok with merging this as long as we make sure that we're 
going to finish the jepsen part of the tests


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465
Gerrit-Change-Number: 8847
Gerrit-PatchSet: 10
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 12 Mar 2018 21:07:00 +
Gerrit-HasComments: No

[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8847 )

Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode
..


Patch Set 10: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465
Gerrit-Change-Number: 8847
Gerrit-PatchSet: 10
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 12 Mar 2018 21:06:25 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2334: Fix OutboundTransfer::TransferStarted() to work with SSL write()

[Michael Ho (Code Review)]

Michael Ho has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9587


Change subject: KUDU-2334: Fix OutboundTransfer::TransferStarted() to work with 
SSL_write()
..

KUDU-2334: Fix OutboundTransfer::TransferStarted() to work with SSL_write()

Previously, OutboundTransfer::TransferStarted() returns true iff
non-zero bytes have been successfully sent via Writev(). As it turns
out, this doesn't work well with SSL_write(). When SSL_write() returns -1
with errno EAGAIN or ETRYAGAIN, we need to retry the call with exactly
the same buffer pointer next time even if 0 bytes have been written.

The following sequence becomes problematic with the previous implementation
of OutboundTransfer::TransferStarted():

- WriteHandler() calls SendBuffer() on an OutboundTransfer.
- SendBuffer() calls TlsSocket::Writev() which hits the EAGAIN error above.
  Since 0 bytes were written, cur_slice_idx_ and cur_offset_in_slice_ remain 0
  and OutboundTransfer::TransferStarted() still returns false.
- OutboundTransfer is cancelled or timed out. car->call is set to NULL.
- WirteHandler() is called again and as it notices that the OutboundTransfer
  hasn't really started yet and "car->call" is NULL due to cancellation, it
  removes it from the outbound transfer queue and moves on to the next entry
  in the queue.
- WriteHandler() calls SendBuffer() with the next entry in the queue and
  eventually calls SSL_write() with a different buffer than expected by
  SSL_write(), leading to "SSL3_WRITE_PENDING:bad write retry" error.

This change fixes the problem above by adding a boolean flag 'started_'
which is set to true if OutboundTransfer::SendBuffer() has been called
at least once. Also added some tests to exercise cancellation paths with
multiple concurrent RPCs.

Change-Id: Id7ebdcbc1ef2a3e0c5e7162f03214c232755b683
---
M src/kudu/rpc/rpc-test-base.h
M src/kudu/rpc/rpc-test.cc
M src/kudu/rpc/transfer.cc
M src/kudu/rpc/transfer.h
4 files changed, 83 insertions(+), 1 deletion(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id7ebdcbc1ef2a3e0c5e7162f03214c232755b683
Gerrit-Change-Number: 9587
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho 

[kudu-CR] [release notes] notes on the replica management scheme

[Alexey Serbin (Code Review)]

Hello Will Berkeley, Mike Percy, Dan Burkert, Jean-Daniel Cryans, Kudu Jenkins, 
Grant Henke, Todd Lipcon,

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

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

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

Change subject: [release_notes] notes on the replica management scheme
..

[release_notes] notes on the replica management scheme

Added relevant notes on the new replica management scheme used
in Kudu 1.7 by defaul:
  * the new replica management scheme is incompatible with old one
  * rolling upgrade 1.6 -> 1.7 is not possible

Change-Id: I49f1f1e17cdaee272592d598431a33dbfe55123f
---
M docs/release_notes.adoc
1 file changed, 38 insertions(+), 2 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I49f1f1e17cdaee272592d598431a33dbfe55123f
Gerrit-Change-Number: 9571
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] KUDU-2230: java: sync client exception stack traces should make sense

[Alexey Serbin (Code Review)]

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

Change subject: KUDU-2230: java: sync client exception stack traces should make 
sense
..


Patch Set 2: Code-Review+2

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9489/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java@110
PS2, Line 110:   StackTraceElement[] stack = new 
Exception().getStackTrace();
> actually given that this is a recursive method that's probably not robust.
Yeah, but since it's assumed exceptions happen not so often, probably that's 
not so crucial this to be robust.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I874d47b5239bcc8493c15ef763ecfe0fd878d795
Gerrit-Change-Number: 9489
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
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-Comment-Date: Mon, 12 Mar 2018 20:08:15 +
Gerrit-HasComments: Yes