[kudu-CR] De-flake heavy-update-compaction-itest

[David Ribeiro Alves (Code Review)]

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

Change subject: De-flake heavy-update-compaction-itest
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2be0b12c010e991ee05ffbbb0c433c6ca1a8d8e7
Gerrit-Change-Number: 9067
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 19 Jan 2018 02:42:03 +
Gerrit-HasComments: No

[kudu-CR] java: improve error messages when tokens are not used

[Todd Lipcon (Code Review)]

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

Change subject: java: improve error messages when tokens are not used
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java@337
PS1, Line 337: chosenAuthnType = chooseAuthenticationType(response);
> Yes, I think it makes sense to expose that information as well  in case of
in the case that the server doesn't accept token, we'll end up in the SASL path 
down below and hit the error messages that I've added in this patch. ie these 
error messages I've added are _only_ in the case that it's SASL, so adding it 
would always result in saying "SASL was chosen".



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
Gerrit-Change-Number: 9070
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 19 Jan 2018 01:42:08 +
Gerrit-HasComments: Yes

[kudu-CR] java: improve error messages when tokens are not used

[Dan Burkert (Code Review)]

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

Change subject: java: improve error messages when tokens are not used
..


Patch Set 1:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java@135
PS1, Line 135: was
s/was/is to keep the tense consistent


http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java@439
PS1, Line 439:   message = "server requires SASL authentication, but " +
I don't think adding SASL makes it more correct here, the server may also 
accept token or cert authn.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
Gerrit-Change-Number: 9070
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 19 Jan 2018 01:39:01 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2126: Add conditional check to prevent unnecessary fsyncs

[Jeffrey F. Lukman (Code Review)]

Hello Mike Percy, Kudu Jenkins, Todd Lipcon,

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

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

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

Change subject: KUDU-2126: Add conditional check to prevent unnecessary fsyncs
..

KUDU-2126: Add conditional check to prevent unnecessary fsyncs

This patch includes unit test to detect fsyncs that happened
when a DeleteTablet RPC tries to tombstone an already tombstoned tablet.
In order to fix the issue, this patch add additional checking
before executing another DeleteTabletData that leads to another fsyncs.

Change-Id: Idde8b413374f43ca2ef09339f0f412208f03685e
---
M src/kudu/integration-tests/delete_tablet-itest.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tablet/tablet_metadata.h
M src/kudu/tserver/ts_tablet_manager.cc
4 files changed, 70 insertions(+), 0 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idde8b413374f43ca2ef09339f0f412208f03685e
Gerrit-Change-Number: 9069
Gerrit-PatchSet: 2
Gerrit-Owner: Jeffrey F. Lukman 
Gerrit-Reviewer: Jeffrey F. Lukman 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Kudu-2126: Add conditional check to prevent unnecessary fsyncs

[Jeffrey F. Lukman (Code Review)]

Jeffrey F. Lukman has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9069 )

Change subject: Kudu-2126: Add conditional check to prevent unnecessary fsyncs
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9069/1/src/kudu/tablet/metadata.proto
File src/kudu/tablet/metadata.proto:

http://gerrit.cloudera.org:8080/#/c/9069/1/src/kudu/tablet/metadata.proto@66
PS1, Line 66:   // A tablet state is set to TABLET_DATA_DELETED when a tablet 
is permanently deleted
:   // from all tablet servers and this state is a terminal state.
:   // Therefore, the tablet should not be allowed to transition 
back to any other state.
:
> I think the bit about it being a "roll-forward" state is useful. Particular
I will remove this change for now.
If this change is still needed, Mike will submit a separate change later.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idde8b413374f43ca2ef09339f0f412208f03685e
Gerrit-Change-Number: 9069
Gerrit-PatchSet: 1
Gerrit-Owner: Jeffrey F. Lukman 
Gerrit-Reviewer: Jeffrey F. Lukman 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 19 Jan 2018 01:30:21 +
Gerrit-HasComments: Yes

[kudu-CR] docs: update docs for metadata dir

[Alex Rodoni (Code Review)]

Alex Rodoni has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9068 )

Change subject: docs: update docs for metadata dir
..


Patch Set 2: Code-Review+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I333d32080106cd83b92ad2060f3239b9c44d201b
Gerrit-Change-Number: 9068
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Fri, 19 Jan 2018 01:29:21 +
Gerrit-HasComments: No

[kudu-CR] java: improve error messages when tokens are not used

[Hao Hao (Code Review)]

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

Change subject: java: improve error messages when tokens are not used
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java:

http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java@57
PS1, Line 57: Assert.assertThat(e.getMessage(), 
CoreMatchers.containsString(
> I wasn't sure how to engineer those situations without having to do a bunch
Agree, maybe a simple verification after initiation of Negotiator object is 
enough? Though that may not work for NOT_CHOSEN_BY_SERVER case.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
Gerrit-Change-Number: 9070
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 19 Jan 2018 01:25:21 +
Gerrit-HasComments: Yes

[kudu-CR] docs: update docs for metadata dir

[Andrew Wong (Code Review)]

Andrew Wong has removed a vote on this change.

Change subject: docs: update docs for metadata dir
..


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I333d32080106cd83b92ad2060f3239b9c44d201b
Gerrit-Change-Number: 9068
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] docs: update docs for metadata dir

[Andrew Wong (Code Review)]

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

Change subject: docs: update docs for metadata dir
..


Patch Set 2: Verified+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I333d32080106cd83b92ad2060f3239b9c44d201b
Gerrit-Change-Number: 9068
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Fri, 19 Jan 2018 01:24:48 +
Gerrit-HasComments: No

[kudu-CR] java: improve error messages when tokens are not used

[Alexey Serbin (Code Review)]

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

Change subject: java: improve error messages when tokens are not used
..


Patch Set 1:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java@337
PS1, Line 337: chosenAuthnType = chooseAuthenticationType(response);
> Isn't there only the choice of sasl and token? where are you suggesting we
Yes, I think it makes sense to expose that information as well  in case of 
negotiation error.  It could be useful if there is some bug on the server side 
so the server doesn't accept token even if the client is ready to send it in.


http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java:

http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java@57
PS1, Line 57: Assert.assertThat(e.getMessage(), 
CoreMatchers.containsString(
> I wasn't sure how to engineer those situations without having to do a bunch
Well, by my understanding this particular place is just for the specific case.  
Not sure we want to verify other cases right here.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
Gerrit-Change-Number: 9070
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 19 Jan 2018 01:24:15 +
Gerrit-HasComments: Yes

[kudu-CR] Kudu-2126: Add conditional check to prevent unnecessary fsyncs

[Jeffrey F. Lukman (Code Review)]

Jeffrey F. Lukman has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9069 )

Change subject: Kudu-2126: Add conditional check to prevent unnecessary fsyncs
..


Patch Set 1:

(5 comments)

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

http://gerrit.cloudera.org:8080/#/c/9069/1//COMMIT_MSG@7
PS1, Line 7: Kudu-2126: Add conditional check to prevent unnecessary fsyncs
> nit: we always capitalize jira names like KUDU-2126
Done


http://gerrit.cloudera.org:8080/#/c/9069/1/src/kudu/integration-tests/delete_tablet-itest.cc
File src/kudu/integration-tests/delete_tablet-itest.cc:

http://gerrit.cloudera.org:8080/#/c/9069/1/src/kudu/integration-tests/delete_tablet-itest.cc@168
PS1, Line 168: // Kudu-2126 : Ensure that DeleteTablet RPCs does not do double 
fsyncs.
> nit: same thing about capitalization
Done


http://gerrit.cloudera.org:8080/#/c/9069/1/src/kudu/integration-tests/delete_tablet-itest.cc@183
PS1, Line 183: table
> tablets
Done


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

http://gerrit.cloudera.org:8080/#/c/9069/1/src/kudu/tablet/tablet_metadata.cc@528
PS1, Line 528:   flush_count_for_tests_++;
> seems we should only increment this down below where we actually do the flu
Done


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

http://gerrit.cloudera.org:8080/#/c/9069/1/src/kudu/tserver/ts_tablet_manager.cc@797
PS1, Line 797:   // Kudu-2126 : if tablet is already tombstoned, then a request 
to tombstone
> nit: same formatting
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idde8b413374f43ca2ef09339f0f412208f03685e
Gerrit-Change-Number: 9069
Gerrit-PatchSet: 1
Gerrit-Owner: Jeffrey F. Lukman 
Gerrit-Reviewer: Jeffrey F. Lukman 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 19 Jan 2018 01:18:25 +
Gerrit-HasComments: Yes

[kudu-CR] java: improve error messages when tokens are not used

[Todd Lipcon (Code Review)]

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

Change subject: java: improve error messages when tokens are not used
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java:

http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java@57
PS1, Line 57: Assert.assertThat(e.getMessage(), 
CoreMatchers.containsString(
> Do we also want to add test cases to verify the log message for other cases
I wasn't sure how to engineer those situations without having to do a bunch of 
gymnastics with the server. Any ideas?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
Gerrit-Change-Number: 9070
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 19 Jan 2018 01:16:22 +
Gerrit-HasComments: Yes

[kudu-CR] java: improve error messages when tokens are not used

[Hao Hao (Code Review)]

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

Change subject: java: improve error messages when tokens are not used
..


Patch Set 1:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java@337
PS1, Line 337: chosenAuthnType = chooseAuthenticationType(response);
> I think it makes sense to report on the authentication type sent back by th
+1


http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java:

http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java@57
PS1, Line 57: Assert.assertThat(e.getMessage(), 
CoreMatchers.containsString(
Do we also want to add test cases to verify the log message for other cases 
authn token is not used?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
Gerrit-Change-Number: 9070
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 19 Jan 2018 01:12:47 +
Gerrit-HasComments: Yes

[kudu-CR] java: improve error messages when tokens are not used

[Todd Lipcon (Code Review)]

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

Change subject: java: improve error messages when tokens are not used
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java@337
PS1, Line 337: chosenAuthnType = chooseAuthenticationType(response);
> I think it makes sense to report on the authentication type sent back by th
Isn't there only the choice of sasl and token? where are you suggesting we add 
this?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
Gerrit-Change-Number: 9070
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 19 Jan 2018 01:12:02 +
Gerrit-HasComments: Yes

[kudu-CR] java: improve error messages when tokens are not used

[Alexey Serbin (Code Review)]

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

Change subject: java: improve error messages when tokens are not used
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java@337
PS1, Line 337: chosenAuthnType = chooseAuthenticationType(response);
I think it makes sense to report on the authentication type sent back by the 
server if connection negotiation failed.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
Gerrit-Change-Number: 9070
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Fri, 19 Jan 2018 01:03:50 +
Gerrit-HasComments: Yes

[kudu-CR] Kudu-2126: Add conditional check to prevent unnecessary fsyncs

[Todd Lipcon (Code Review)]

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

Change subject: Kudu-2126: Add conditional check to prevent unnecessary fsyncs
..


Patch Set 1:

(6 comments)

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

http://gerrit.cloudera.org:8080/#/c/9069/1//COMMIT_MSG@7
PS1, Line 7: Kudu-2126: Add conditional check to prevent unnecessary fsyncs
nit: we always capitalize jira names like KUDU-2126


http://gerrit.cloudera.org:8080/#/c/9069/1/src/kudu/integration-tests/delete_tablet-itest.cc
File src/kudu/integration-tests/delete_tablet-itest.cc:

http://gerrit.cloudera.org:8080/#/c/9069/1/src/kudu/integration-tests/delete_tablet-itest.cc@168
PS1, Line 168: // Kudu-2126 : Ensure that DeleteTablet RPCs does not do double 
fsyncs.
nit: same thing about capitalization


http://gerrit.cloudera.org:8080/#/c/9069/1/src/kudu/integration-tests/delete_tablet-itest.cc@183
PS1, Line 183: table
tablets


http://gerrit.cloudera.org:8080/#/c/9069/1/src/kudu/tablet/metadata.proto
File src/kudu/tablet/metadata.proto:

http://gerrit.cloudera.org:8080/#/c/9069/1/src/kudu/tablet/metadata.proto@66
PS1, Line 66:   // A tablet state is set to TABLET_DATA_DELETED when a tablet 
is permanently deleted
:   // from all tablet servers and this state is a terminal state.
:   // Therefore, the tablet should not be allowed to transition 
back to any other state.
:
I think the bit about it being a "roll-forward" state is useful. Particularly, 
I think we do flush it to disk as 'deleted' while we are doing the deletion of 
blocks, right?


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

http://gerrit.cloudera.org:8080/#/c/9069/1/src/kudu/tablet/tablet_metadata.cc@528
PS1, Line 528:   flush_count_for_tests_++;
seems we should only increment this down below where we actually do the flush. 
That way it's protected by the lock l_flush and also it ensures that we don't 
overcount the case when the metadata has been pinned


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

http://gerrit.cloudera.org:8080/#/c/9069/1/src/kudu/tserver/ts_tablet_manager.cc@797
PS1, Line 797:   // Kudu-2126 : if tablet is already tombstoned, then a request 
to tombstone
nit: same formatting



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idde8b413374f43ca2ef09339f0f412208f03685e
Gerrit-Change-Number: 9069
Gerrit-PatchSet: 1
Gerrit-Owner: Jeffrey F. Lukman 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 19 Jan 2018 01:03:01 +
Gerrit-HasComments: Yes

[kudu-CR] WIP: KUDU-2264. Automatically attempt to re-acquire Kerberos credentials before expiration

[Todd Lipcon (Code Review)]

Todd Lipcon has removed Anonymous Coward #314 from this change.  ( 
http://gerrit.cloudera.org:8080/9050 )

Change subject: WIP: KUDU-2264. Automatically attempt to re-acquire Kerberos 
credentials before expiration
..


Removed reviewer null.
--
To view, visit http://gerrit.cloudera.org:8080/9050
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: I514253e0a7f067dbc8ffe4eaf5a7a2c32900b539
Gerrit-Change-Number: 9050
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] java: improve error messages when tokens are not used

[Todd Lipcon (Code Review)]

Todd Lipcon has removed Anonymous Coward #314 from this change.  ( 
http://gerrit.cloudera.org:8080/9070 )

Change subject: java: improve error messages when tokens are not used
..


Removed reviewer null.
--
To view, visit http://gerrit.cloudera.org:8080/9070
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
Gerrit-Change-Number: 9070
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [java] add test to reproduce KUDU-2267

[Hao Hao (Code Review)]

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


Change subject: [java] add test to reproduce KUDU-2267
..

[java] add test to reproduce KUDU-2267

Currently, if a master has never been a leader from the very start of
the cluster, it has just self-signed cert. In that case if a client
does not have any valid Kerberos credential but only authenticated
token, then the client may see negotiaition failure error and is not
able to connect that masters. This patch add a test to reproduce this
issue in TestSecurity.

Change-Id: I4879749988dc884fe81cf36838819379db91ae72
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java
3 files changed, 59 insertions(+), 5 deletions(-)



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

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

[kudu-CR] docs: update docs for metadata dir

[Andrew Wong (Code Review)]

Hello Alex Rodoni, Kudu Jenkins, Adar Dembo,

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

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

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

Change subject: docs: update docs for metadata dir
..

docs: update docs for metadata dir

Commit 64926335fe263b43f1493c03a91ea999759dfc71 introduced the
fs_metadata_dir flag. Given the significance of this flag, the docs have
been updated accordingly.

I didn't document the behavior from Kudu 1.6 and below, as the important
details are already noted in the flag definition.

Rendered versions here:
https://github.com/andrwng/kudu/blob/metadata-docs/docs/administration.adoc#disk_failure_recovery
https://github.com/andrwng/kudu/blob/metadata-docs/docs/configuration.adoc#directory_configuration

Change-Id: I333d32080106cd83b92ad2060f3239b9c44d201b
---
M docs/administration.adoc
M docs/configuration.adoc
2 files changed, 24 insertions(+), 14 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I333d32080106cd83b92ad2060f3239b9c44d201b
Gerrit-Change-Number: 9068
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] docs: update docs for metadata dir

[Andrew Wong (Code Review)]

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

Change subject: docs: update docs for metadata dir
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9068/1/docs/configuration.adoc
File docs/configuration.adoc:

http://gerrit.cloudera.org:8080/#/c/9068/1/docs/configuration.adoc@66
PS1, Line 66: drives with high bandwidth and low
: latency, e.g. solid-state drives. If `--f
> drives with high bandwidth and low latency, e.g. solid-state drives.
Done


http://gerrit.cloudera.org:8080/#/c/9068/1/docs/configuration.adoc@72
PS1, Line 72:  not spe
> multiple values
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I333d32080106cd83b92ad2060f3239b9c44d201b
Gerrit-Change-Number: 9068
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Fri, 19 Jan 2018 00:39:09 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2238. DMS not flush under memory pressure.

[Todd Lipcon (Code Review)]

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

Change subject: KUDU-2238. DMS not flush under memory pressure.
..

KUDU-2238. DMS not flush under memory pressure.

When we choose DMS to flush, now we always pick the DMS with
highest log retention. However, as KUDU-2238 shows, in some cases
DMS with highest log retention may only consume little memory, and
other DMSs may consume much more memory, but get no chance to be
flushed, even under memory pressure.

This patch gives the ability to take memory consumption into
consideration when we choose DMS.

Change-Id: I0c04d76aa0e3888352dc56eeb493a5437ef47e42
Reviewed-on: http://gerrit.cloudera.org:8080/8904
Reviewed-by: Todd Lipcon 
Tested-by: Kudu Jenkins
---
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_replica_mm_ops.cc
3 files changed, 18 insertions(+), 8 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I0c04d76aa0e3888352dc56eeb493a5437ef47e42
Gerrit-Change-Number: 8904
Gerrit-PatchSet: 5
Gerrit-Owner: zhen.zhang 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: zhen.zhang 

[kudu-CR] KUDU-2254: Detect and warn about misusage of KuduContext

[Todd Lipcon (Code Review)]

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

Change subject: KUDU-2254: Detect and warn about misusage of KuduContext
..


Abandoned

Per above discussion, this no longer seems relevant
--
To view, visit http://gerrit.cloudera.org:8080/9004
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I65a7cd11a14fa8a668079b0d1fcf6ed3a34fb652
Gerrit-Change-Number: 9004
Gerrit-PatchSet: 2
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2261: The order of the responses after flush should match the order we call apply

[Todd Lipcon (Code Review)]

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

Change subject: KUDU-2261: The order of the responses after flush should match 
the order we call apply
..

KUDU-2261: The order of the responses after flush should match the order we 
call apply

The response list of flush() should have the same order of we apply operations,
so it's easier to know which operation failed and which succeeded.

Change-Id: Ib37c9e85ad03731bb7d5b83be77d40fcd95e803a
Reviewed-on: http://gerrit.cloudera.org:8080/9029
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java
M java/kudu-client/src/main/java/org/apache/kudu/client/BatchResponse.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduSession.java
4 files changed, 71 insertions(+), 10 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib37c9e85ad03731bb7d5b83be77d40fcd95e803a
Gerrit-Change-Number: 9029
Gerrit-PatchSet: 3
Gerrit-Owner: zhen.zhang 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: zhen.zhang 

[kudu-CR] KUDU-2261: The order of the responses after flush should match the order we call apply

[Todd Lipcon (Code Review)]

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

Change subject: KUDU-2261: The order of the responses after flush should match 
the order we call apply
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib37c9e85ad03731bb7d5b83be77d40fcd95e803a
Gerrit-Change-Number: 9029
Gerrit-PatchSet: 2
Gerrit-Owner: zhen.zhang 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: zhen.zhang 
Gerrit-Comment-Date: Fri, 19 Jan 2018 00:24:41 +
Gerrit-HasComments: No

[kudu-CR] docs: update docs for metadata dir

[Adar Dembo (Code Review)]

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

Change subject: docs: update docs for metadata dir
..


Patch Set 1: Code-Review+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I333d32080106cd83b92ad2060f3239b9c44d201b
Gerrit-Change-Number: 9068
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Fri, 19 Jan 2018 00:20:58 +
Gerrit-HasComments: No

[kudu-CR] WIP: KUDU-2264. Automatically attempt to re-acquire Kerberos credentials before expiration

[Todd Lipcon (Code Review)]

Hello Alexey Serbin, Dan Burkert, Kudu Jenkins, Anonymous Coward #314, Adar 
Dembo,

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

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

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

Change subject: WIP: KUDU-2264. Automatically attempt to re-acquire Kerberos 
credentials before expiration
..

WIP: KUDU-2264. Automatically attempt to re-acquire Kerberos credentials before 
expiration

WIP because it needs a bit more testing and a lot of documentation of
the new behavior and best practices

Change-Id: I514253e0a7f067dbc8ffe4eaf5a7a2c32900b539
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java
M java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java
M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiator.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool.proto
M src/kudu/tools/tool_action_test.cc
11 files changed, 560 insertions(+), 42 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I514253e0a7f067dbc8ffe4eaf5a7a2c32900b539
Gerrit-Change-Number: 9050
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Anonymous Coward #314
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] java: improve error messages when tokens are not used

[Todd Lipcon (Code Review)]

Hello Alexey Serbin, Anonymous Coward #314,

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

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

to review the following change.


Change subject: java: improve error messages when tokens are not used
..

java: improve error messages when tokens are not used

We recently had some trouble debugging Java client connection failures
where we expected it to authenticate by token but it did not. Previously
it just complained that it couldn't authenticate by SASL and didn't give
any clue as to why it didn't use tokens.

This patch makes the error more clear by explaining why a token was not
used to authenticate.

Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
---
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java
2 files changed, 42 insertions(+), 9 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
Gerrit-Change-Number: 9070
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Anonymous Coward #314

[kudu-CR] docs: update docs for metadata dir

[Alex Rodoni (Code Review)]

Alex Rodoni has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9068 )

Change subject: docs: update docs for metadata dir
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9068/1/docs/configuration.adoc
File docs/configuration.adoc:

http://gerrit.cloudera.org:8080/#/c/9068/1/docs/configuration.adoc@66
PS1, Line 66: drives (i.e. with high bandwidth
: and low latency, e.g. solid-state drives)
drives with high bandwidth and low latency, e.g. solid-state drives.


http://gerrit.cloudera.org:8080/#/c/9068/1/docs/configuration.adoc@72
PS1, Line 72: multiple
multiple values



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I333d32080106cd83b92ad2060f3239b9c44d201b
Gerrit-Change-Number: 9068
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 18 Jan 2018 23:54:57 +
Gerrit-HasComments: Yes

[kudu-CR] WIP: KUDU-2264. Automatically attempt to re-acquire Kerberos credentials before expiration

[Todd Lipcon (Code Review)]

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

Change subject: WIP: KUDU-2264. Automatically attempt to re-acquire Kerberos 
credentials before expiration
..


Patch Set 1:

(8 comments)

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

http://gerrit.cloudera.org:8080/#/c/9050/1/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java@90
PS1, Line 90:   private Object subjectLock = new Object();
> add 'final'
Done


http://gerrit.cloudera.org:8080/#/c/9050/1/java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java
File java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java:

http://gerrit.cloudera.org:8080/#/c/9050/1/java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java@78
PS1, Line 78: logins
> 'a new subject is logged in'
Done


http://gerrit.cloudera.org:8080/#/c/9050/1/java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java@186
PS1, Line 186: if (principal.getName().equals("krbtgt/" + 
principal.getRealm() +
> simplify this to
Done


http://gerrit.cloudera.org:8080/#/c/9050/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java:

http://gerrit.cloudera.org:8080/#/c/9050/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java@107
PS1, Line 107:   Thread.sleep(5000);
> Could you add an assert which guarantees the if branch is actually taken? R
changed the loop time to ensure that it always runs for twice the renewable 
lifetime, rather than 15 loops


http://gerrit.cloudera.org:8080/#/c/9050/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java@128
PS1, Line 128:   // TODO(todd) add test which externally provides a subject and 
ensures that it doesn't
> I guess one downside of removing the Java MiniKdc is that this becomes much
wasn't that bad. stay tuned.


http://gerrit.cloudera.org:8080/#/c/9050/1/src/kudu/tools/tool.proto
File src/kudu/tools/tool.proto:

http://gerrit.cloudera.org:8080/#/c/9050/1/src/kudu/tools/tool.proto@157
PS1, Line 157:   // is kinitted by default when the cluster starts.
> Instead of documenting this, consider adding it as the protobuf field defau
Done


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

http://gerrit.cloudera.org:8080/#/c/9050/1/src/kudu/tools/tool_action_test.cc@283
PS1, Line 283: case ControlShellRequestPB::kKinit:
> Just passing through, but could you add a test case for this in kudu-tool-t
Done


http://gerrit.cloudera.org:8080/#/c/9050/1/src/kudu/tools/tool_action_test.cc@288
PS1, Line 288:   string username = req.kinit().has_username() ? 
req.kinit().username() : "test-admin";
> This dance becomes unnecessary with the default defined in the .proto.
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I514253e0a7f067dbc8ffe4eaf5a7a2c32900b539
Gerrit-Change-Number: 9050
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 23:42:18 +
Gerrit-HasComments: Yes

[kudu-CR] Kudu-2126: Add conditional check to prevent unnecessary fsyncs

[Jeffrey F. Lukman (Code Review)]

Jeffrey F. Lukman has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9069


Change subject: Kudu-2126: Add conditional check to prevent unnecessary fsyncs
..

Kudu-2126: Add conditional check to prevent unnecessary fsyncs

This patch includes unit test to detect fsyncs that happened
when a DeleteTablet RPC tries to tombstone an already tombstoned tablet.
In order to fix the issue, this patch add additional checking
before executing another DeleteTabletData that leads to another fsyncs.

Furthermore, this patch also refine the TABLET_DATA_DELETED comment.

Change-Id: Idde8b413374f43ca2ef09339f0f412208f03685e
---
M src/kudu/integration-tests/delete_tablet-itest.cc
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tablet/tablet_metadata.h
M src/kudu/tserver/ts_tablet_manager.cc
5 files changed, 75 insertions(+), 3 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Idde8b413374f43ca2ef09339f0f412208f03685e
Gerrit-Change-Number: 9069
Gerrit-PatchSet: 1
Gerrit-Owner: Jeffrey F. Lukman 

[kudu-CR] docs: update docs for metadata dir

[Andrew Wong (Code Review)]

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


Change subject: docs: update docs for metadata dir
..

docs: update docs for metadata dir

Commit 64926335fe263b43f1493c03a91ea999759dfc71 introduced the
fs_metadata_dir flag. Given the significance of this flag, the docs have
been updated accordingly.

I didn't document the behavior from Kudu 1.6 and below, as the important
details are already noted in the flag definition.

Rendered versions here:
https://github.com/andrwng/kudu/blob/metadata-docs/docs/administration.adoc#disk_failure_recovery
https://github.com/andrwng/kudu/blob/metadata-docs/docs/configuration.adoc#directory_configuration

Change-Id: I333d32080106cd83b92ad2060f3239b9c44d201b
---
M docs/administration.adoc
M docs/configuration.adoc
2 files changed, 25 insertions(+), 14 deletions(-)



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

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

[kudu-CR] [docs] Removed the note about KUDU-1626

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9020 )

Change subject: [docs] Removed the note about KUDU-1626
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7b5e127cd9d1e2bda60a20de62fafd95e5f93e3
Gerrit-Change-Number: 9020
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 23:34:45 +
Gerrit-HasComments: No

[kudu-CR] [docs] Removed the note about KUDU-1626

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9020 )

Change subject: [docs] Removed the note about KUDU-1626
..

[docs] Removed the note about KUDU-1626

Change-Id: Ia7b5e127cd9d1e2bda60a20de62fafd95e5f93e3
Reviewed-on: http://gerrit.cloudera.org:8080/9020
Tested-by: Kudu Jenkins
Reviewed-by: Jean-Daniel Cryans 
---
M docs/schema_design.adoc
1 file changed, 0 insertions(+), 5 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia7b5e127cd9d1e2bda60a20de62fafd95e5f93e3
Gerrit-Change-Number: 9020
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [docs] Added a section for Kudu clients connecting to secure clusters

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8953 )

Change subject: [docs] Added a section for Kudu clients connecting to secure 
clusters
..


Patch Set 8: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iabee52216b4db0e18e5f693375f0a3e0f0a8a164
Gerrit-Change-Number: 8953
Gerrit-PatchSet: 8
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 18 Jan 2018 23:34:19 +
Gerrit-HasComments: No

[kudu-CR] [docs] Added a section for Kudu clients connecting to secure clusters

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/8953 )

Change subject: [docs] Added a section for Kudu clients connecting to secure 
clusters
..

[docs] Added a section for Kudu clients connecting to secure clusters

Change-Id: Iabee52216b4db0e18e5f693375f0a3e0f0a8a164
Reviewed-on: http://gerrit.cloudera.org:8080/8953
Tested-by: Kudu Jenkins
Reviewed-by: Jean-Daniel Cryans 
---
M docs/security.adoc
1 file changed, 13 insertions(+), 0 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Iabee52216b4db0e18e5f693375f0a3e0f0a8a164
Gerrit-Change-Number: 8953
Gerrit-PatchSet: 9
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [docs] Added steps to update HMS after migrating to multiple Kudu masters

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has removed a vote on this change.

Change subject: [docs] Added steps to update HMS after migrating to multiple 
Kudu masters
..


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Iab3999c9e581ed3591b220c08491cdae867c91db
Gerrit-Change-Number: 8948
Gerrit-PatchSet: 15
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [docs] Added steps to update HMS after migrating to multiple Kudu masters

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/8948 )

Change subject: [docs] Added steps to update HMS after migrating to multiple 
Kudu masters
..

[docs] Added steps to update HMS after migrating to multiple Kudu masters

Change-Id: Iab3999c9e581ed3591b220c08491cdae867c91db
Reviewed-on: http://gerrit.cloudera.org:8080/8948
Reviewed-by: Jean-Daniel Cryans 
Tested-by: Jean-Daniel Cryans 
---
M docs/administration.adoc
1 file changed, 36 insertions(+), 0 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Iab3999c9e581ed3591b220c08491cdae867c91db
Gerrit-Change-Number: 8948
Gerrit-PatchSet: 16
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [docs] Added steps to update HMS after migrating to multiple Kudu masters

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8948 )

Change subject: [docs] Added steps to update HMS after migrating to multiple 
Kudu masters
..


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


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iab3999c9e581ed3591b220c08491cdae867c91db
Gerrit-Change-Number: 8948
Gerrit-PatchSet: 15
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 23:33:16 +
Gerrit-HasComments: No

[kudu-CR] [docs] Added a section for Kudu clients connecting to secure clusters

[Alex Rodoni (Code Review)]

Alex Rodoni has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8953 )

Change subject: [docs] Added a section for Kudu clients connecting to secure 
clusters
..


Patch Set 8: -Code-Review

This is ready for review.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iabee52216b4db0e18e5f693375f0a3e0f0a8a164
Gerrit-Change-Number: 8953
Gerrit-PatchSet: 8
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 18 Jan 2018 23:16:55 +
Gerrit-HasComments: No

[kudu-CR] [docs] Added steps to update HMS after migrating to multiple Kudu masters

[Alex Rodoni (Code Review)]

Alex Rodoni has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8948 )

Change subject: [docs] Added steps to update HMS after migrating to multiple 
Kudu masters
..


Patch Set 15: -Code-Review

This is ready for review.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iab3999c9e581ed3591b220c08491cdae867c91db
Gerrit-Change-Number: 8948
Gerrit-PatchSet: 15
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 23:16:39 +
Gerrit-HasComments: No

[kudu-CR] [docs] Removed the note about KUDU-1626

[Alex Rodoni (Code Review)]

Alex Rodoni has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9020 )

Change subject: [docs] Removed the note about KUDU-1626
..


Patch Set 1: -Code-Review

This is ready for review.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7b5e127cd9d1e2bda60a20de62fafd95e5f93e3
Gerrit-Change-Number: 9020
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 23:16:21 +
Gerrit-HasComments: No

[kudu-CR] fs: defer failure from metadata load to bootstrap when data dir is missing

[Adar Dembo (Code Review)]

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

Change subject: fs: defer failure from metadata load to bootstrap when data dir 
is missing
..


Patch Set 8:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/8376/8/src/kudu/fs/data_dirs.cc
File src/kudu/fs/data_dirs.cc:

http://gerrit.cloudera.org:8080/#/c/8376/8/src/kudu/fs/data_dirs.cc@327
PS8, Line 327: FindOrDie(uuid_by_uuid_idx, uuid_idx)
> is this not just 'uuid' above?
Yeah, not sure what happened here.


http://gerrit.cloudera.org:8080/#/c/8376/8/src/kudu/fs/data_dirs.cc@330
PS8, Line 330:   pb->Swap();
> I think now that we are on newish protobuf we could do '*pb = std::move(gro
Done


http://gerrit.cloudera.org:8080/#/c/8376/8/src/kudu/tablet/tablet_bootstrap.cc
File src/kudu/tablet/tablet_bootstrap.cc:

http://gerrit.cloudera.org:8080/#/c/8376/8/src/kudu/tablet/tablet_bootstrap.cc@557
PS8, Line 557:   // Ensure the tablet's data dirs are present and healthy 
before it is opened.
> not 100% sure I follow why you defer it all the way until bootstrap time vs
The old location was in TSTabletManager::OpenTablet, which is run out of the 
bootstrap threadpool too. If you're suggesting I do it  in 
TabletMetadata::LoadFromSuperBlock (called synchronously for each tablet in 
TSTabletManager::Init), it'd require more plumbing.

Why did I move it at all? My preference is for TSTabletManager to hold 
tserver-specific behavior only, and while the "disk failure handling" logic 
isn't particularly useful for the master, it could be in the future, so 
locating it in TabletBootstrap is an easy way to ensure it makes it into every 
place that uses a tablet. Tablet::Open would be equally reasonable, I think.


http://gerrit.cloudera.org:8080/#/c/8376/8/src/kudu/tablet/tablet_bootstrap.cc@561
PS8, Line 561: error retrieving tablet data dir group
> will this be easy enough for an operator to understand? perhaps we should s
Done


http://gerrit.cloudera.org:8080/#/c/8376/8/src/kudu/tablet/tablet_bootstrap.cc@563
PS8, Line 563: return Status::IOError("tablet data is in a failed 
directory");
> perhaps more accurate to say that _some_ tablet data is in a failed directo
I'll clarify with 'some', but if you follow the return chain up the stack 
you'll see that TSTabletManager::OpenTablet prefixes the message with the 
tablet ID before it logs it.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e8d5697c2bb08287cce11fbdab6fb8d6e37d1ad
Gerrit-Change-Number: 8376
Gerrit-PatchSet: 8
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 22:57:43 +
Gerrit-HasComments: Yes

[kudu-CR] De-flake heavy-update-compaction-itest

[Dan Burkert (Code Review)]

Hello Kudu Jenkins, Todd Lipcon,

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

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

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

Change subject: De-flake heavy-update-compaction-itest
..

De-flake heavy-update-compaction-itest

Validated with 100 runs on dist-test with 8 stress threads.

Change-Id: I2be0b12c010e991ee05ffbbb0c433c6ca1a8d8e7
---
M src/kudu/integration-tests/heavy-update-compaction-itest.cc
1 file changed, 9 insertions(+), 0 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2be0b12c010e991ee05ffbbb0c433c6ca1a8d8e7
Gerrit-Change-Number: 9067
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2202: support for removing data directories (take two)

[Andrew Wong (Code Review)]

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

Change subject: KUDU-2202: support for removing data directories (take two)
..


Patch Set 7:

(1 comment)

With the metadata dir patch in, it's probably worth at least discussing (and 
maybe testing) the ways in which affects the remove tool's behavior. I don't 
think much changes, but something like:

- Metadata dirs in WAL dir: removing any data dir is fine. No configuration 
changes necessary.
- Metadata dirs in the first data dir: This is also fine, but further 
deployments must now specify the metadata dir, as it is no longer referenced in 
fs_data_dirs.
- Metadata dirs in any other directory: same as the WAL case.

http://gerrit.cloudera.org:8080/#/c/8978/7/src/kudu/fs/fs_manager.cc
File src/kudu/fs/fs_manager.cc:

http://gerrit.cloudera.org:8080/#/c/8978/7/src/kudu/fs/fs_manager.cc@347
PS7, Line 347:   if (!missing_roots.empty() && !opts_.read_only) {
> huh, is this an existing bug? if you use an fs_manager tool and pass an ext
missing_roots will only be non-empty if opts.update_on_disk is true (as per 
L292). This should probably be in a comment.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
Gerrit-Change-Number: 8978
Gerrit-PatchSet: 7
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 22:44:49 +
Gerrit-HasComments: Yes

[kudu-CR] De-flake heavy-update-compaction-itest

[Dan Burkert (Code Review)]

Hello Kudu Jenkins, Todd Lipcon,

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

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

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

Change subject: De-flake heavy-update-compaction-itest
..

De-flake heavy-update-compaction-itest

Validated with 100 runs on dist-test with 8 stress threads.

Change-Id: I2be0b12c010e991ee05ffbbb0c433c6ca1a8d8e7
---
M src/kudu/integration-tests/heavy-update-compaction-itest.cc
1 file changed, 9 insertions(+), 1 deletion(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2be0b12c010e991ee05ffbbb0c433c6ca1a8d8e7
Gerrit-Change-Number: 9067
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] De-flake heacy-update-compaction-itest

[Dan Burkert (Code Review)]

Hello Todd Lipcon,

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

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

to review the following change.


Change subject: De-flake heacy-update-compaction-itest
..

De-flake heacy-update-compaction-itest

Validated with 100 runs on dist-test with 8 stress threads.

Change-Id: I2be0b12c010e991ee05ffbbb0c433c6ca1a8d8e7
---
M src/kudu/integration-tests/heavy-update-compaction-itest.cc
1 file changed, 9 insertions(+), 1 deletion(-)



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

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

[kudu-CR](branch-1.5.x) rpc: allow setting --rpc tls min protocol on older RHEL versions

[Dan Burkert (Code Review)]

Dan Burkert has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9056 )

Change subject: rpc: allow setting --rpc_tls_min_protocol on older RHEL versions
..

rpc: allow setting --rpc_tls_min_protocol on older RHEL versions

Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Reviewed-on: http://gerrit.cloudera.org:8080/7821
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
Reviewed-by: Alexey Serbin 
Reviewed-by: Sailesh Mukil 
Reviewed-on: http://gerrit.cloudera.org:8080/9056
Reviewed-by: Jean-Daniel Cryans 
---
M src/kudu/security/tls_context.cc
1 file changed, 36 insertions(+), 12 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.5.x
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Gerrit-Change-Number: 9056
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Enable vptr UBSAN checker

[Dan Burkert (Code Review)]

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

Change subject: Enable vptr UBSAN checker
..


Patch Set 2:

Carrying over the +2 from Todd and Alexey.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2ce8631964292aec4d7be56577f5a6ef3b596cbd
Gerrit-Change-Number: 9051
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 21:41:33 +
Gerrit-HasComments: No

[kudu-CR] Enable vptr UBSAN checker

[Dan Burkert (Code Review)]

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

Change subject: Enable vptr UBSAN checker
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2ce8631964292aec4d7be56577f5a6ef3b596cbd
Gerrit-Change-Number: 9051
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 21:41:07 +
Gerrit-HasComments: No

[kudu-CR] Enable vptr UBSAN checker

[Dan Burkert (Code Review)]

Dan Burkert has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9051 )

Change subject: Enable vptr UBSAN checker
..

Enable vptr UBSAN checker

The issue with boost intrusive list no longer appears to be relevant. If
it reappears we can add the corresponding functions to the UBSAN
blacklist instead of disabling the check across the board.

Change-Id: I2ce8631964292aec4d7be56577f5a6ef3b596cbd
Reviewed-on: http://gerrit.cloudera.org:8080/9051
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert 
---
M CMakeLists.txt
1 file changed, 1 insertion(+), 2 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I2ce8631964292aec4d7be56577f5a6ef3b596cbd
Gerrit-Change-Number: 9051
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] WIP [master] no half-baked responses on ConnectoToMaster

[Adar Dembo (Code Review)]

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

Change subject: WIP [master] no half-baked responses on ConnectoToMaster
..


Patch Set 2:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/9052/2/src/kudu/master/master_service.cc@463
PS2, Line 463: // Do not send half-baked responses from leader master when 
its catalog
 : // manager is not in proper state yet.
Are you sure that's not by design? Todd added ConnectToMaster and I think the 
intent was for it to always "succeed" and supply enough information to explain 
the state the master was in.


http://gerrit.cloudera.org:8080/#/c/9052/2/src/kudu/master/master_service.cc@488
PS2, Line 488: // TODO(KUDU-1924): it seems there is some window when 
'role' is LEADER but
This comment seems relevant.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9c906863f5f0e1995041281b122135e1b2cd3a4
Gerrit-Change-Number: 9052
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 21:24:11 +
Gerrit-HasComments: Yes

[kudu-CR] Add unsigned-integer overflow checking to UBSAN

[Dan Burkert (Code Review)]

Dan Burkert has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/6637 )

Change subject: Add unsigned-integer overflow checking to UBSAN
..

Add unsigned-integer overflow checking to UBSAN

Background:

We recently found a very subtle bug caused by unsigned integer overflow
in tablet_peer_mm_ops.cc (KUDU-2017). This patch was able to repro that
overflow from at least two different unit tests, so the value is high.
Additionally, it lets us be a little more aggressive about using
unsigned integers. I've never been much of a fan of the style guide's
ban.

A bunch of the overflow instances are expected, like hash functions and
random number generators. A few bugs were exposed, including a missing
statement in Schema::swap, and an accounting error in the thread manager
(fixed in a preceding commit).

Change-Id: Ib0712f5307dcbab7fdffbe2364e1fdb691d3aaab
Reviewed-on: http://gerrit.cloudera.org:8080/6637
Reviewed-by: Todd Lipcon 
Tested-by: Kudu Jenkins
---
M CMakeLists.txt
A build-support/ubsan-blacklist.txt
M src/kudu/cfile/cfile-test-base.h
M src/kudu/cfile/index_btree.cc
M src/kudu/clock/hybrid_clock-test.cc
M src/kudu/clock/hybrid_clock.cc
M src/kudu/common/key_util.cc
M src/kudu/common/partition.cc
M src/kudu/common/schema.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/gutil/dynamic_annotations.h
M src/kudu/gutil/hash/city.cc
M src/kudu/gutil/hash/hash128to64.h
M src/kudu/gutil/hash/jenkins.cc
M src/kudu/gutil/hash/jenkins_lookup2.h
M src/kudu/gutil/port.h
M src/kudu/gutil/spinlock_internal.cc
M src/kudu/gutil/strings/fastmem.h
M src/kudu/gutil/strings/numbers.cc
M src/kudu/integration-tests/alter_table-randomized-test.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/tablet/mt-tablet-test.cc
M src/kudu/util/alignment.h
M src/kudu/util/bit-stream-utils.inline.h
M src/kudu/util/bitmap-test.cc
M src/kudu/util/bloom_filter.h
M src/kudu/util/debug-util.cc
M src/kudu/util/debug/trace_event_impl.cc
M src/kudu/util/hash_util.h
M src/kudu/util/memory/overwrite.cc
M src/kudu/util/random_util.cc
M src/kudu/util/rle-encoding.h
M src/kudu/util/stopwatch.h
M src/kudu/util/threadpool.cc
34 files changed, 174 insertions(+), 58 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib0712f5307dcbab7fdffbe2364e1fdb691d3aaab
Gerrit-Change-Number: 6637
Gerrit-PatchSet: 6
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR](branch-1.6.x) rpc: allow setting --rpc tls min protocol on older RHEL versions

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9055 )

Change subject: rpc: allow setting --rpc_tls_min_protocol on older RHEL versions
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.6.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Gerrit-Change-Number: 9055
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 20:45:46 +
Gerrit-HasComments: No

[kudu-CR] Fix typo in TLS support status message

[Dan Burkert (Code Review)]

Dan Burkert has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9061 )

Change subject: Fix typo in TLS support status message
..

Fix typo in TLS support status message

Introduced in recently in b88117415a02699c12a6eacbf065c4140ee0963c.

Change-Id: I36f8c5146b41d4a9df363a1315ec3baf4552282e
Reviewed-on: http://gerrit.cloudera.org:8080/9061
Reviewed-by: Alexey Serbin 
Tested-by: Kudu Jenkins
---
M src/kudu/security/tls_context.cc
1 file changed, 2 insertions(+), 2 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I36f8c5146b41d4a9df363a1315ec3baf4552282e
Gerrit-Change-Number: 9061
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR](branch-1.5.x) rpc: allow setting --rpc tls min protocol on older RHEL versions

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9056 )

Change subject: rpc: allow setting --rpc_tls_min_protocol on older RHEL versions
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.5.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Gerrit-Change-Number: 9056
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 20:45:33 +
Gerrit-HasComments: No

[kudu-CR] WIP [master] no half-baked responses on ConnectoToMaster

[Alexey Serbin (Code Review)]

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

Change subject: WIP [master] no half-baked responses on ConnectoToMaster
..


Patch Set 2: Verified+1

Unrelated flaek in 
BlockManagerType/TsRecoveryITest.TestRestartWithPendingCommitFromFailedOp/1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9c906863f5f0e1995041281b122135e1b2cd3a4
Gerrit-Change-Number: 9052
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 20:17:42 +
Gerrit-HasComments: No

[kudu-CR] WIP [master] no half-baked responses on ConnectoToMaster

[Alexey Serbin (Code Review)]

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

Change subject: WIP [master] no half-baked responses on ConnectoToMaster
..


Removed reviewer Kudu Jenkins with the following votes:

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: Ib9c906863f5f0e1995041281b122135e1b2cd3a4
Gerrit-Change-Number: 9052
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] WIP: KUDU-2264. Automatically attempt to re-acquire Kerberos credentials before expiration

[Todd Lipcon (Code Review)]

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

Change subject: WIP: KUDU-2264. Automatically attempt to re-acquire Kerberos 
credentials before expiration
..


Patch Set 1:

https://stackoverflow.com/questions/34616676/should-i-call-ugi-checktgtandreloginfromkeytab-before-every-action-on-hadoop
 is a nice read btw


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I514253e0a7f067dbc8ffe4eaf5a7a2c32900b539
Gerrit-Change-Number: 9050
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 20:16:33 +
Gerrit-HasComments: No

[kudu-CR] WIP: KUDU-2264. Automatically attempt to re-acquire Kerberos credentials before expiration

[Todd Lipcon (Code Review)]

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

Change subject: WIP: KUDU-2264. Automatically attempt to re-acquire Kerberos 
credentials before expiration
..


Patch Set 1:

I'm not certain it avoids the race. UGI's "relogin" code would end up logging 
out the current Subject (which would be used by Kudu) and then logging it back 
in. We'd have to have our own code which does a synchronized 
(UserGroupInformation.class) { ... } or something in order to avoid potentially 
accessing the Subject while it's in the process of re-login.

Alternatively we could add some code that attempts to retry authentication 
failures if we previously had kerberos tickets and then notice that they 
disappeared?


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I514253e0a7f067dbc8ffe4eaf5a7a2c32900b539
Gerrit-Change-Number: 9050
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 20:09:45 +
Gerrit-HasComments: No

[kudu-CR] KUDU-1489: allow configuration of metadata dir

[Todd Lipcon (Code Review)]

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

Change subject: KUDU-1489: allow configuration of metadata dir
..

KUDU-1489: allow configuration of metadata dir

Metadata files currently reside in the first configured data directory,
which isn't always the best choice; this first drive, if not performant,
can act as a performance bottleneck. Often times the drive containing
the WALs is a better choice, as it is recommended to be the fastest.

This patch allows users to configure their metadata directory through
the new fs_metadata_dir flag. If empty, Kudu will check if a metadata
directory exists in the first member of fs_data_dirs, and if none
exists, Kudu will use fs_wal_dir as the root directory for metadata.
Otherwise, the flag will be honored verbatim.

Aside from the update to the directory location, codepaths that
previously assumed that the first data directory _must_ be healthy have
been updated. The remaining invariant is that at least a single data
directory must be healthy.

The following test changes are included:
- fs_manager-test: unit tests for various scenarios
- data_dirs-test: a test case is updated to show that we can now open
  the directory manager with a failed first data directory (previously
  this codepath would hit D/CHECK failures)
- mini_tablet_server-test: an end-to-end test is added to manually
  go back and forth between the old and new default metadata directories
- kudu-tool-test: a small test that tools still work, but only when the
  appropriate FS flags are provided

Change-Id: I375c6b2eb283db5fa9c956135d98252c8781f5db
Reviewed-on: http://gerrit.cloudera.org:8080/9027
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo 
Reviewed-by: Todd Lipcon 
---
M src/kudu/fs/block_manager_util.cc
M src/kudu/fs/data_dirs-test.cc
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/fs_manager-test.cc
M src/kudu/fs/fs_manager.cc
M src/kudu/fs/fs_manager.h
M src/kudu/integration-tests/external_mini_cluster_fs_inspector.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/mini_tablet_server-test.cc
11 files changed, 347 insertions(+), 87 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I375c6b2eb283db5fa9c956135d98252c8781f5db
Gerrit-Change-Number: 9027
Gerrit-PatchSet: 13
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-1489: allow configuration of metadata dir

[Todd Lipcon (Code Review)]

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

Change subject: KUDU-1489: allow configuration of metadata dir
..


Patch Set 12: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I375c6b2eb283db5fa9c956135d98252c8781f5db
Gerrit-Change-Number: 9027
Gerrit-PatchSet: 12
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 20:08:06 +
Gerrit-HasComments: No

[kudu-CR] Add unsigned-integer overflow checking to UBSAN

[Todd Lipcon (Code Review)]

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

Change subject: Add unsigned-integer overflow checking to UBSAN
..


Patch Set 5: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0712f5307dcbab7fdffbe2364e1fdb691d3aaab
Gerrit-Change-Number: 6637
Gerrit-PatchSet: 5
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 20:06:40 +
Gerrit-HasComments: No

[kudu-CR] Fix typo in TLS support status message

[Alexey Serbin (Code Review)]

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

Change subject: Fix typo in TLS support status message
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I36f8c5146b41d4a9df363a1315ec3baf4552282e
Gerrit-Change-Number: 9061
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 20:04:53 +
Gerrit-HasComments: No

[kudu-CR](branch-1.6.x) rpc: allow setting --rpc tls min protocol on older RHEL versions

[Dan Burkert (Code Review)]

Hello Alexey Serbin, Jean-Daniel Cryans, Kudu Jenkins, Sailesh Mukil, Todd 
Lipcon,

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

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

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

Change subject: rpc: allow setting --rpc_tls_min_protocol on older RHEL versions
..

rpc: allow setting --rpc_tls_min_protocol on older RHEL versions

Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Reviewed-on: http://gerrit.cloudera.org:8080/7821
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
Reviewed-by: Alexey Serbin 
Reviewed-by: Sailesh Mukil 
---
M src/kudu/security/tls_context.cc
1 file changed, 36 insertions(+), 12 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.6.x
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Gerrit-Change-Number: 9055
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR](branch-1.5.x) rpc: allow setting --rpc tls min protocol on older RHEL versions

[Dan Burkert (Code Review)]

Hello Alexey Serbin, Jean-Daniel Cryans, Kudu Jenkins, Sailesh Mukil, Todd 
Lipcon,

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

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

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

Change subject: rpc: allow setting --rpc_tls_min_protocol on older RHEL versions
..

rpc: allow setting --rpc_tls_min_protocol on older RHEL versions

Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Reviewed-on: http://gerrit.cloudera.org:8080/7821
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
Reviewed-by: Alexey Serbin 
Reviewed-by: Sailesh Mukil 
---
M src/kudu/security/tls_context.cc
1 file changed, 36 insertions(+), 12 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.5.x
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Gerrit-Change-Number: 9056
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Fix typo in TLS support status message

[Dan Burkert (Code Review)]

Hello Todd Lipcon,

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

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

to review the following change.


Change subject: Fix typo in TLS support status message
..

Fix typo in TLS support status message

Introduced in recently in b88117415a02699c12a6eacbf065c4140ee0963c.

Change-Id: I36f8c5146b41d4a9df363a1315ec3baf4552282e
---
M src/kudu/security/tls_context.cc
1 file changed, 2 insertions(+), 2 deletions(-)



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

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

[kudu-CR] Add unsigned-integer overflow checking to UBSAN

[Dan Burkert (Code Review)]

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

Change subject: Add unsigned-integer overflow checking to UBSAN
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6637/4/src/kudu/gutil/port.h
File src/kudu/gutil/port.h:

http://gerrit.cloudera.org:8080/#/c/6637/4/src/kudu/gutil/port.h@425
PS4, Line 425: #if defined(__SANITIZE_ADDRESS__) || defined(ADDRESS_SANITIZER)
> hrm, so this only will trigger if ASAN is _enabled_, not _available_. So if
Ah ok I misunderstood.  I've changed it to use has_feature.  This version did 
indeed break all of the tests.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0712f5307dcbab7fdffbe2364e1fdb691d3aaab
Gerrit-Change-Number: 6637
Gerrit-PatchSet: 4
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 19:55:24 +
Gerrit-HasComments: Yes

[kudu-CR] Add unsigned-integer overflow checking to UBSAN

[Dan Burkert (Code Review)]

Hello Kudu Jenkins, Adar Dembo, Todd Lipcon,

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

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

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

Change subject: Add unsigned-integer overflow checking to UBSAN
..

Add unsigned-integer overflow checking to UBSAN

Background:

We recently found a very subtle bug caused by unsigned integer overflow
in tablet_peer_mm_ops.cc (KUDU-2017). This patch was able to repro that
overflow from at least two different unit tests, so the value is high.
Additionally, it lets us be a little more aggressive about using
unsigned integers. I've never been much of a fan of the style guide's
ban.

A bunch of the overflow instances are expected, like hash functions and
random number generators. A few bugs were exposed, including a missing
statement in Schema::swap, and an accounting error in the thread manager
(fixed in a preceding commit).

Change-Id: Ib0712f5307dcbab7fdffbe2364e1fdb691d3aaab
---
M CMakeLists.txt
A build-support/ubsan-blacklist.txt
M src/kudu/cfile/cfile-test-base.h
M src/kudu/cfile/index_btree.cc
M src/kudu/clock/hybrid_clock-test.cc
M src/kudu/clock/hybrid_clock.cc
M src/kudu/common/key_util.cc
M src/kudu/common/partition.cc
M src/kudu/common/schema.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/gutil/dynamic_annotations.h
M src/kudu/gutil/hash/city.cc
M src/kudu/gutil/hash/hash128to64.h
M src/kudu/gutil/hash/jenkins.cc
M src/kudu/gutil/hash/jenkins_lookup2.h
M src/kudu/gutil/port.h
M src/kudu/gutil/spinlock_internal.cc
M src/kudu/gutil/strings/fastmem.h
M src/kudu/gutil/strings/numbers.cc
M src/kudu/integration-tests/alter_table-randomized-test.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/tablet/mt-tablet-test.cc
M src/kudu/util/alignment.h
M src/kudu/util/bit-stream-utils.inline.h
M src/kudu/util/bitmap-test.cc
M src/kudu/util/bloom_filter.h
M src/kudu/util/debug-util.cc
M src/kudu/util/debug/trace_event_impl.cc
M src/kudu/util/hash_util.h
M src/kudu/util/memory/overwrite.cc
M src/kudu/util/random_util.cc
M src/kudu/util/rle-encoding.h
M src/kudu/util/stopwatch.h
M src/kudu/util/threadpool.cc
34 files changed, 174 insertions(+), 58 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib0712f5307dcbab7fdffbe2364e1fdb691d3aaab
Gerrit-Change-Number: 6637
Gerrit-PatchSet: 5
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] WIP: KUDU-2264. Automatically attempt to re-acquire Kerberos credentials before expiration

[Dan Burkert (Code Review)]

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

Change subject: WIP: KUDU-2264. Automatically attempt to re-acquire Kerberos 
credentials before expiration
..


Patch Set 1:

Would there be any advantage to that?  I think given that the implementation 
here is pretty straightforward and that it avoids the UGI re-acquisition race 
we discussed offline that this solution is cleaner and better.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I514253e0a7f067dbc8ffe4eaf5a7a2c32900b539
Gerrit-Change-Number: 9050
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 19:48:42 +
Gerrit-HasComments: No

[kudu-CR] WIP: KUDU-2264. Automatically attempt to re-acquire Kerberos credentials before expiration

[Todd Lipcon (Code Review)]

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

Change subject: WIP: KUDU-2264. Automatically attempt to re-acquire Kerberos 
credentials before expiration
..


Patch Set 1:

Thanks for the review Dan. Before I go finish up the patch, wanted to make sure 
you think the overall policy is a reasonable one for the client. Should we try 
to do something to automatically hook into hadoop's UGI class if we find such a 
class lying around by reflection?


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I514253e0a7f067dbc8ffe4eaf5a7a2c32900b539
Gerrit-Change-Number: 9050
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 19:42:02 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2202: support for removing data directories (take two)

[Todd Lipcon (Code Review)]

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

Change subject: KUDU-2202: support for removing data directories (take two)
..


Patch Set 7:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/8978/7/src/kudu/fs/fs_manager.h
File src/kudu/fs/fs_manager.h:

http://gerrit.cloudera.org:8080/#/c/8978/7/src/kudu/fs/fs_manager.h@104
PS7, Line 104:   // If 'update_on_disk' and 'read_only' are both true, the 
FsManager allows
 :   // 'data_roots' to deviate from their on-disk data structures 
without
 :   // updating the latter to match the former.
I'm not really a fan of these semantics -- it seems somewhat confusing to both 
ask to "update" and also to "not write". I prefer read_only to just enforce a 
sort of "fail any write ops" rather than in some cases also meaning "skip the 
write op."

Would it be feasible to change this to some kind of enum like:

enum ConsistencyCheckBehavior {
  // If data_roots doesn't match the on-disk path sets, fail.
  ENFORCE_CONSISTENCY,

  // If data_roots doesn't match the on-disk path sets, update the
  // on-disk data to match. Requires 'read_only' to be false.
  UPDATE_ON_DISK,

  // If data_roots doesn't match the on-disk path sets,
  // continue without updating the on-disk data.
  IGNORE_INCONSISTENCY
};


http://gerrit.cloudera.org:8080/#/c/8978/7/src/kudu/fs/fs_manager.cc
File src/kudu/fs/fs_manager.cc:

http://gerrit.cloudera.org:8080/#/c/8978/7/src/kudu/fs/fs_manager.cc@347
PS7, Line 347:   if (!missing_roots.empty() && !opts_.read_only) {
huh, is this an existing bug? if you use an fs_manager tool and pass an extra 
dir, it gets created?


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

http://gerrit.cloudera.org:8080/#/c/8978/7/src/kudu/tools/kudu-tool-test.cc@2251
PS7, Line 2251:   // second table should have all failed.
can we add one more step in this test which re-adds the force-removed 
directory? Would that properly re-enable the missing tablets? Seems like a 
useful "oops!" recovery option for an admin who runs the wrong command.


http://gerrit.cloudera.org:8080/#/c/8978/7/src/kudu/tools/tool_action_fs.cc
File src/kudu/tools/tool_action_fs.cc:

http://gerrit.cloudera.org:8080/#/c/8978/7/src/kudu/tools/tool_action_fs.cc@330
PS7, Line 330:   // If the user specifies --force, we assume they know what 
they're doing and
 :   // skip this check.
would it be possible to do the check in a new function, and something like:

Status check_result = CheckTabletsWontFailWithUpdate(...);
if (FLAGS_force) {
   WARN_NOT_OK(check_result, "blah blah continuing because you said force");
} else {
  RETURN_NOT_OK_PREPEND(check_result, "blah blah");
}

This way if someone uses --force and then pastes us the logs of what they did, 
we'll at least see what happened



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
Gerrit-Change-Number: 8978
Gerrit-PatchSet: 7
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 19:35:47 +
Gerrit-HasComments: Yes

[kudu-CR](branch-1.5.x) rpc: allow setting --rpc tls min protocol on older RHEL versions

[Dan Burkert (Code Review)]

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

Change subject: rpc: allow setting --rpc_tls_min_protocol on older RHEL versions
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9056/1/src/kudu/security/tls_context.cc
File src/kudu/security/tls_context.cc:

http://gerrit.cloudera.org:8080/#/c/9056/1/src/kudu/security/tls_context.cc@157
PS1, Line 157: no
> does not? is this a typo in the orig patch?
Yeah looks like this is a typo on master.  I'll fixup these backports and 
submit a new patch for master.



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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.5.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Gerrit-Change-Number: 9056
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 19:20:53 +
Gerrit-HasComments: Yes

[kudu-CR](branch-1.5.x) rpc: allow setting --rpc tls min protocol on older RHEL versions

[Todd Lipcon (Code Review)]

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

Change subject: rpc: allow setting --rpc_tls_min_protocol on older RHEL versions
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9056/1/src/kudu/security/tls_context.cc
File src/kudu/security/tls_context.cc:

http://gerrit.cloudera.org:8080/#/c/9056/1/src/kudu/security/tls_context.cc@157
PS1, Line 157: no
does not? is this a typo in the orig patch?



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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.5.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Gerrit-Change-Number: 9056
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 19:15:31 +
Gerrit-HasComments: Yes

[kudu-CR](branch-1.5.x) rpc: allow setting --rpc tls min protocol on older RHEL versions

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9056 )

Change subject: rpc: allow setting --rpc_tls_min_protocol on older RHEL versions
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.5.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Gerrit-Change-Number: 9056
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 19:15:36 +
Gerrit-HasComments: No

[kudu-CR] Add unsigned-integer overflow checking to UBSAN

[Todd Lipcon (Code Review)]

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

Change subject: Add unsigned-integer overflow checking to UBSAN
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6637/4/src/kudu/gutil/port.h
File src/kudu/gutil/port.h:

http://gerrit.cloudera.org:8080/#/c/6637/4/src/kudu/gutil/port.h@425
PS4, Line 425: #if defined(__SANITIZE_ADDRESS__) || defined(ADDRESS_SANITIZER)
hrm, so this only will trigger if ASAN is _enabled_, not _available_. So if we 
only enable UBSAN and not ASAN it won't get turned on. You didn't like 
has_feature?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0712f5307dcbab7fdffbe2364e1fdb691d3aaab
Gerrit-Change-Number: 6637
Gerrit-PatchSet: 4
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 19:09:05 +
Gerrit-HasComments: Yes

[kudu-CR] WIP: KUDU-2264. Automatically attempt to re-acquire Kerberos credentials before expiration

[Dan Burkert (Code Review)]

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

Change subject: WIP: KUDU-2264. Automatically attempt to re-acquire Kerberos 
credentials before expiration
..


Patch Set 1:

(7 comments)

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

http://gerrit.cloudera.org:8080/#/c/9050/1/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java@90
PS1, Line 90:   private Object subjectLock = new Object();
add 'final'


http://gerrit.cloudera.org:8080/#/c/9050/1/java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java
File java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java:

http://gerrit.cloudera.org:8080/#/c/9050/1/java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java@78
PS1, Line 78: logins
'a new subject is logged in'


http://gerrit.cloudera.org:8080/#/c/9050/1/java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java@186
PS1, Line 186: if (principal.getName().equals("krbtgt/" + 
principal.getRealm() +
simplify this to

  return principal.getName().equals("krbtgt/" + principal.getRealm() + "@" + 
principal.getRealm())


http://gerrit.cloudera.org:8080/#/c/9050/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java:

http://gerrit.cloudera.org:8080/#/c/9050/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java@107
PS1, Line 107:   Thread.sleep(5000);
Could you add an assert which guarantees the if branch is actually taken? Right 
now the test is a bit brittle to the TICKED_LIFETIME_SECS value changing in the 
other file, since the number of loops and sleep time is constant.


http://gerrit.cloudera.org:8080/#/c/9050/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java@128
PS1, Line 128:   // TODO(todd) add test which externally provides a subject and 
ensures that it doesn't
I guess one downside of removing the Java MiniKdc is that this becomes much 
harder.


http://gerrit.cloudera.org:8080/#/c/9050/1/src/kudu/tools/tool.proto
File src/kudu/tools/tool.proto:

http://gerrit.cloudera.org:8080/#/c/9050/1/src/kudu/tools/tool.proto@157
PS1, Line 157:   // is kinitted by default when the cluster starts.
Instead of documenting this, consider adding it as the protobuf field default:

  optional string usernam = 1 [default = "test-admin"];


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

http://gerrit.cloudera.org:8080/#/c/9050/1/src/kudu/tools/tool_action_test.cc@288
PS1, Line 288:   string username = req.kinit().has_username() ? 
req.kinit().username() : "test-admin";
This dance becomes unnecessary with the default defined in the .proto.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I514253e0a7f067dbc8ffe4eaf5a7a2c32900b539
Gerrit-Change-Number: 9050
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 18 Jan 2018 19:06:49 +
Gerrit-HasComments: Yes

[kudu-CR] fs: defer failure from metadata load to bootstrap when data dir is missing

[Todd Lipcon (Code Review)]

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

Change subject: fs: defer failure from metadata load to bootstrap when data dir 
is missing
..


Patch Set 8:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/8376/8/src/kudu/fs/data_dirs.cc
File src/kudu/fs/data_dirs.cc:

http://gerrit.cloudera.org:8080/#/c/8376/8/src/kudu/fs/data_dirs.cc@327
PS8, Line 327: FindOrDie(uuid_by_uuid_idx, uuid_idx)
is this not just 'uuid' above?


http://gerrit.cloudera.org:8080/#/c/8376/8/src/kudu/fs/data_dirs.cc@330
PS8, Line 330:   pb->Swap();
I think now that we are on newish protobuf we could do '*pb = 
std::move(group);' which reads a bit better


http://gerrit.cloudera.org:8080/#/c/8376/8/src/kudu/tablet/tablet_bootstrap.cc
File src/kudu/tablet/tablet_bootstrap.cc:

http://gerrit.cloudera.org:8080/#/c/8376/8/src/kudu/tablet/tablet_bootstrap.cc@557
PS8, Line 557:   // Ensure the tablet's data dirs are present and healthy 
before it is opened.
not 100% sure I follow why you defer it all the way until bootstrap time vs 
marking it FAILED in TSTabletManager::Init immediately after it opens the 
TabletMeta (the old behavior)? Seems that way we'd have the correct failed 
status for missing tablets as early as possible vs the possibility that those 
missing tablets end up waiting on the bootstrap threadpool queue for many 
minutes behind non-failed tablets doing their log replay


http://gerrit.cloudera.org:8080/#/c/8376/8/src/kudu/tablet/tablet_bootstrap.cc@561
PS8, Line 561: error retrieving tablet data dir group
will this be easy enough for an operator to understand? perhaps we should say 
something explanatory like "(data directory likely removed)" or somesuch?


http://gerrit.cloudera.org:8080/#/c/8376/8/src/kudu/tablet/tablet_bootstrap.cc@563
PS8, Line 563: return Status::IOError("tablet data is in a failed 
directory");
perhaps more accurate to say that _some_ tablet data is in a failed directory?

Do you think we could include the uuid in the message for shits and giggles?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e8d5697c2bb08287cce11fbdab6fb8d6e37d1ad
Gerrit-Change-Number: 8376
Gerrit-PatchSet: 8
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 18:51:48 +
Gerrit-HasComments: Yes

[kudu-CR] Add unsigned-integer overflow checking to UBSAN

[Dan Burkert (Code Review)]

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

Change subject: Add unsigned-integer overflow checking to UBSAN
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6637/3/src/kudu/gutil/port.h
File src/kudu/gutil/port.h:

http://gerrit.cloudera.org:8080/#/c/6637/3/src/kudu/gutil/port.h@424
PS3, Line 424: #if defined(__llvm__)
> gcc >=4.9 also has UBSAN I believe. Perhaps we should has has_feature(addre
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0712f5307dcbab7fdffbe2364e1fdb691d3aaab
Gerrit-Change-Number: 6637
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 18:46:23 +
Gerrit-HasComments: Yes

[kudu-CR] Add unsigned-integer overflow checking to UBSAN

[Dan Burkert (Code Review)]

Hello Kudu Jenkins, Adar Dembo, Todd Lipcon,

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

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

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

Change subject: Add unsigned-integer overflow checking to UBSAN
..

Add unsigned-integer overflow checking to UBSAN

Background:

We recently found a very subtle bug caused by unsigned integer overflow
in tablet_peer_mm_ops.cc (KUDU-2017). This patch was able to repro that
overflow from at least two different unit tests, so the value is high.
Additionally, it lets us be a little more aggressive about using
unsigned integers. I've never been much of a fan of the style guide's
ban.

A bunch of the overflow instances are expected, like hash functions and
random number generators. A few bugs were exposed, including a missing
statement in Schema::swap, and an accounting error in the thread manager
(fixed in a preceding commit).

Change-Id: Ib0712f5307dcbab7fdffbe2364e1fdb691d3aaab
---
M CMakeLists.txt
A build-support/ubsan-blacklist.txt
M src/kudu/cfile/cfile-test-base.h
M src/kudu/cfile/index_btree.cc
M src/kudu/clock/hybrid_clock-test.cc
M src/kudu/clock/hybrid_clock.cc
M src/kudu/common/key_util.cc
M src/kudu/common/partition.cc
M src/kudu/common/schema.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/gutil/dynamic_annotations.h
M src/kudu/gutil/hash/city.cc
M src/kudu/gutil/hash/hash128to64.h
M src/kudu/gutil/hash/jenkins.cc
M src/kudu/gutil/hash/jenkins_lookup2.h
M src/kudu/gutil/port.h
M src/kudu/gutil/spinlock_internal.cc
M src/kudu/gutil/strings/fastmem.h
M src/kudu/gutil/strings/numbers.cc
M src/kudu/integration-tests/alter_table-randomized-test.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/tablet/mt-tablet-test.cc
M src/kudu/util/alignment.h
M src/kudu/util/bit-stream-utils.inline.h
M src/kudu/util/bitmap-test.cc
M src/kudu/util/bloom_filter.h
M src/kudu/util/debug-util.cc
M src/kudu/util/debug/trace_event_impl.cc
M src/kudu/util/hash_util.h
M src/kudu/util/memory/overwrite.cc
M src/kudu/util/random_util.cc
M src/kudu/util/rle-encoding.h
M src/kudu/util/stopwatch.h
M src/kudu/util/threadpool.cc
34 files changed, 170 insertions(+), 58 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib0712f5307dcbab7fdffbe2364e1fdb691d3aaab
Gerrit-Change-Number: 6637
Gerrit-PatchSet: 4
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR](branch-1.6.x) rpc: allow setting --rpc tls min protocol on older RHEL versions

[Sailesh Mukil (Code Review)]

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

Change subject: rpc: allow setting --rpc_tls_min_protocol on older RHEL versions
..


Patch Set 1: Code-Review+1


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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.6.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Gerrit-Change-Number: 9055
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 18:32:23 +
Gerrit-HasComments: No

[kudu-CR](branch-1.6.x) rpc: allow setting --rpc tls min protocol on older RHEL versions

[Dan Burkert (Code Review)]

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

Change subject: rpc: allow setting --rpc_tls_min_protocol on older RHEL versions
..


Patch Set 1:

This had to be done manually due to a conflict with 
https://github.com/apache/kudu/commit/b357fa9b729ce52627a862e08ebac822ae470bc9, 
however it's the same patch as https://gerrit.cloudera.org/#/c/9056/ 
(branch-1.5.x).


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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.6.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Gerrit-Change-Number: 9055
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 18:31:43 +
Gerrit-HasComments: No

[kudu-CR](branch-1.5.x) rpc: allow setting --rpc tls min protocol on older RHEL versions

[Dan Burkert (Code Review)]

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

Change subject: rpc: allow setting --rpc_tls_min_protocol on older RHEL versions
..


Patch Set 1:

This had to be done manually due to a conflict with 
https://github.com/apache/kudu/commit/b357fa9b729ce52627a862e08ebac822ae470bc9.


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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.5.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Gerrit-Change-Number: 9056
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 18:30:53 +
Gerrit-HasComments: No

[kudu-CR](branch-1.5.x) rpc: allow setting --rpc tls min protocol on older RHEL versions

[Dan Burkert (Code Review)]

Hello Alexey Serbin, Jean-Daniel Cryans, Kudu Jenkins, Sailesh Mukil, Todd 
Lipcon,

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

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

to review the following change.


Change subject: rpc: allow setting --rpc_tls_min_protocol on older RHEL versions
..

rpc: allow setting --rpc_tls_min_protocol on older RHEL versions

Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Reviewed-on: http://gerrit.cloudera.org:8080/7821
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
Reviewed-by: Alexey Serbin 
Reviewed-by: Sailesh Mukil 
---
M src/kudu/security/tls_context.cc
1 file changed, 36 insertions(+), 12 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.5.x
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Gerrit-Change-Number: 9056
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR](branch-1.6.x) rpc: allow setting --rpc tls min protocol on older RHEL versions

[Dan Burkert (Code Review)]

Hello Alexey Serbin, Jean-Daniel Cryans, Kudu Jenkins, Sailesh Mukil, Todd 
Lipcon,

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

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

to review the following change.


Change subject: rpc: allow setting --rpc_tls_min_protocol on older RHEL versions
..

rpc: allow setting --rpc_tls_min_protocol on older RHEL versions

Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Reviewed-on: http://gerrit.cloudera.org:8080/7821
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
Reviewed-by: Alexey Serbin 
Reviewed-by: Sailesh Mukil 
---
M src/kudu/security/tls_context.cc
1 file changed, 36 insertions(+), 12 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.6.x
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Gerrit-Change-Number: 9055
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Add unsigned-integer overflow checking to UBSAN

[Todd Lipcon (Code Review)]

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

Change subject: Add unsigned-integer overflow checking to UBSAN
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6637/3/src/kudu/gutil/port.h
File src/kudu/gutil/port.h:

http://gerrit.cloudera.org:8080/#/c/6637/3/src/kudu/gutil/port.h@424
PS3, Line 424: #if defined(__llvm__)
gcc >=4.9 also has UBSAN I believe. Perhaps we should has 
has_feature(address_sanitizer) as a proxy? (I think all versions with UBSAN 
also have ASAN, and at least the attribute should be valid?)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0712f5307dcbab7fdffbe2364e1fdb691d3aaab
Gerrit-Change-Number: 6637
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 18:00:02 +
Gerrit-HasComments: Yes

[kudu-CR] Enable vptr UBSAN checker

[Todd Lipcon (Code Review)]

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

Change subject: Enable vptr UBSAN checker
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2ce8631964292aec4d7be56577f5a6ef3b596cbd
Gerrit-Change-Number: 9051
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 17:54:01 +
Gerrit-HasComments: No

[kudu-CR] Enable vptr UBSAN checker

[Alexey Serbin (Code Review)]

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

Change subject: Enable vptr UBSAN checker
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2ce8631964292aec4d7be56577f5a6ef3b596cbd
Gerrit-Change-Number: 9051
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 18 Jan 2018 17:00:03 +
Gerrit-HasComments: No

[kudu-CR] WIP [master] no half-baked responses on ConnectoToMaster

[Alexey Serbin (Code Review)]

Hello Kudu Jenkins, Todd Lipcon,

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

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

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

Change subject: WIP [master] no half-baked responses on ConnectoToMaster
..

WIP [master] no half-baked responses on ConnectoToMaster

Do not send half-baked responses to clients from master that declares
itself a leader when its catalog manager is not yet in proper state.

WIP: already existing AuthTokenIssuingTest.ChannelConfidentiality test
 provides some coverage for the new code, but maybe a dedicated
 test is needed?

Change-Id: Ib9c906863f5f0e1995041281b122135e1b2cd3a4
---
M src/kudu/integration-tests/security-itest.cc
M src/kudu/master/master_service.cc
2 files changed, 15 insertions(+), 20 deletions(-)


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

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