[kudu-CR] tserver: log a message when a scanner is not found

2018-09-26 Thread Mike Percy (Code Review)
Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11516 )

Change subject: tserver: log a message when a scanner is not found
..


Patch Set 5: Verified+1

Overriding flaky HMS test


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If4e9c69160605d4a839ac2d28def67f4d3402b90
Gerrit-Change-Number: 11516
Gerrit-PatchSet: 5
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Thu, 27 Sep 2018 01:20:10 +
Gerrit-HasComments: No


[kudu-CR] tserver: log a message when a scanner is not found

2018-09-26 Thread Mike Percy (Code Review)
Mike Percy has removed a vote on this change.

Change subject: tserver: log a message when a scanner is not found
..


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: If4e9c69160605d4a839ac2d28def67f4d3402b90
Gerrit-Change-Number: 11516
Gerrit-PatchSet: 5
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot


[kudu-CR] tserver: log a message when a scanner is not found

2018-09-26 Thread Mike Percy (Code Review)
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Grant Henke,

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

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

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

Change subject: tserver: log a message when a scanner is not found
..

tserver: log a message when a scanner is not found

Right now there is a class of issues where an "upstream" service or
query engine such as Impala or Spark may not be able to find a Kudu
scanner, resulting in a failed query. Often, this is because the scanner
timed out. The query engine will receive and typically log a message
indicating that the scanner could not be found, however it's difficult
to trace this back to the original query because the client wouldn't
know the scanner id and the server would not log the event.

With this change, it will be much easier to match up query engine
failures with expiring scanners by looking at the logs on both sides
because when a request comes in for a scanner that cannot be found, the
client will get a bad status:

  Not found: Scanner 7672e46ed30d42938c54bf6e7e24946e not found (it may have 
expired)

And the server will log a slightly more verbose message:

  I0926 14:59:50.600463 21137 tablet_service.cc:2020] Scan: Not found: Scanner 
7672e46ed30d42938c54bf6e7e24946e not found (it may have expired): call sequence 
id=1, remote={username='mpercy'} at 127.0.0.1:42660

Additionally, the above situation is handled in a similar way in the
case of a TabletService::ScannerKeepAlive() RPC call.

Change-Id: If4e9c69160605d4a839ac2d28def67f4d3402b90
---
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
M src/kudu/client/client-test.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tablet_service.h
5 files changed, 48 insertions(+), 20 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If4e9c69160605d4a839ac2d28def67f4d3402b90
Gerrit-Change-Number: 11516
Gerrit-PatchSet: 5
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot


[kudu-CR] [spark] Cleanup hasNext logic

2018-09-26 Thread Grant Henke (Code Review)
Grant Henke has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/11529 )

Change subject: [spark] Cleanup hasNext logic
..

[spark] Cleanup hasNext logic

When returning to the while loop in
RowIterator.hasNext() I always have to do a
double take on the logic. This change makes
the logic a bit more clear.

Change-Id: I340af13a695b3a8489e03018542b8ad7db614758
Reviewed-on: http://gerrit.cloudera.org:8080/11529
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo 
---
M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackupRDD.scala
M java/kudu-client/src/main/java/org/apache/kudu/client/RowResultIterator.java
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduRDD.scala
3 files changed, 5 insertions(+), 7 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I340af13a695b3a8489e03018542b8ad7db614758
Gerrit-Change-Number: 11529
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 


[kudu-CR] KUDU-428: add Sentry to thirdparty, mini-sentry

2018-09-26 Thread Dan Burkert (Code Review)
Dan Burkert has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/11347 )

Change subject: KUDU-428: add Sentry to thirdparty, mini-sentry
..

KUDU-428: add Sentry to thirdparty, mini-sentry

This commit adds Sentry to thirdparty, and fills out the MiniSentry
class with an initial implementation. Notable features that aren't
implemented:

- Stripped Sentry packaging. I've put an unmodified version of Sentry
  2.0.1 into thirdparty. It weighs in at almost 200MiB and takes about
  5s to startup on my laptop. We will probably want to add a stripped
  version later to reduce both of these.

- Kerberos support for mini-sentry. Right now Kerberos is disabled,
  which is an atypical configuration. A follow-up commit will add a
  Kerberos support configuration taking advantage of the mini KDC.

- The mini Sentry is not yet configured with the location of the HMS,
  which will be necessary to do anything non-trivial with it.

Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5
Reviewed-on: http://gerrit.cloudera.org:8080/11347
Reviewed-by: Adar Dembo 
Reviewed-by: Hao Hao 
Tested-by: Kudu Jenkins
---
M build-support/dist_test.py
M build-support/run_dist_test.py
M src/kudu/hms/mini_hms.cc
M src/kudu/sentry/CMakeLists.txt
M src/kudu/sentry/mini_sentry.cc
M src/kudu/sentry/mini_sentry.h
M src/kudu/sentry/sentry_client-test.cc
M src/kudu/util/test_util.cc
M src/kudu/util/test_util.h
M thirdparty/build-thirdparty.sh
M thirdparty/download-thirdparty.sh
M thirdparty/vars.sh
12 files changed, 281 insertions(+), 43 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5
Gerrit-Change-Number: 11347
Gerrit-PatchSet: 7
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot


[kudu-CR] KUDU-2541: Fill out basic sentry client API

2018-09-26 Thread Dan Burkert (Code Review)
Dan Burkert has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/11524 )

Change subject: KUDU-2541: Fill out basic sentry client API
..

KUDU-2541: Fill out basic sentry client API

This commit contains more plumbing for the upcoming Sentry integration
(KUDU-428). The Sentry client class is filled out with enough methods to
ensure the Thrift RPC is working correctly, as well as error conversion
from Sentry and Thrift's format to Kudu's Status.

Change-Id: I1004a11506ed109d1f23389ca73d95a34c32c194
Reviewed-on: http://gerrit.cloudera.org:8080/11524
Reviewed-by: Adar Dembo 
Reviewed-by: Hao Hao 
Tested-by: Kudu Jenkins
---
M src/kudu/sentry/CMakeLists.txt
M src/kudu/sentry/mini_sentry.cc
M src/kudu/sentry/mini_sentry.h
M src/kudu/sentry/sentry_client-test.cc
M src/kudu/sentry/sentry_client.cc
M src/kudu/sentry/sentry_client.h
6 files changed, 299 insertions(+), 9 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I1004a11506ed109d1f23389ca73d95a34c32c194
Gerrit-Change-Number: 11524
Gerrit-PatchSet: 4
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot


[kudu-CR] KUDU-2541: add Kerberos support to Sentry client and mini-cluster

2018-09-26 Thread Dan Burkert (Code Review)
Dan Burkert has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/11525 )

Change subject: KUDU-2541: add Kerberos support to Sentry client and 
mini-cluster
..

KUDU-2541: add Kerberos support to Sentry client and mini-cluster

More infrastructure work towards KUDU-428.

Change-Id: I3f7e27137135cb9b463a90b98e4aba864cece3c1
Reviewed-on: http://gerrit.cloudera.org:8080/11525
Reviewed-by: Hao Hao 
Reviewed-by: Adar Dembo 
Tested-by: Kudu Jenkins
---
M src/kudu/sentry/CMakeLists.txt
M src/kudu/sentry/mini_sentry.cc
M src/kudu/sentry/mini_sentry.h
M src/kudu/sentry/sentry_client-test.cc
4 files changed, 95 insertions(+), 10 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I3f7e27137135cb9b463a90b98e4aba864cece3c1
Gerrit-Change-Number: 11525
Gerrit-PatchSet: 4
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR] tserver: log a message when a scanner is not found

2018-09-26 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11516 )

Change subject: tserver: log a message when a scanner is not found
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If4e9c69160605d4a839ac2d28def67f4d3402b90
Gerrit-Change-Number: 11516
Gerrit-PatchSet: 4
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Wed, 26 Sep 2018 23:52:09 +
Gerrit-HasComments: No


[kudu-CR] tserver: log a message when a scanner is not found

2018-09-26 Thread Mike Percy (Code Review)
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Grant Henke,

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

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

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

Change subject: tserver: log a message when a scanner is not found
..

tserver: log a message when a scanner is not found

Right now there is a class of issues where an "upstream" service or
query engine such as Impala or Spark may not be able to find a Kudu
scanner, resulting in a failed query. Often, this is because the scanner
timed out. The query engine will receive and typically log a message
indicating that the scanner could not be found, however it's difficult
to trace this back to the original query because the client wouldn't
know the scanner id and the server would not log the event.

With this change, it will be much easier to match up query engine
failures with expiring scanners by looking at the logs on both sides
because when a request comes in for a scanner that cannot be found, the
client will get a bad status:

  Not found: Scanner 7672e46ed30d42938c54bf6e7e24946e not found (it may have 
expired)

And the server will log a slightly more verbose message:

  I0926 14:59:50.600463 21137 tablet_service.cc:2020] Scan: Not found: Scanner 
7672e46ed30d42938c54bf6e7e24946e not found (it may have expired): call sequence 
id=1, remote={username='mpercy'} at 127.0.0.1:42660

Additionally, the above situation is handled in a similar way in the
case of a TabletService::ScannerKeepAlive() RPC call.

Change-Id: If4e9c69160605d4a839ac2d28def67f4d3402b90
---
M src/kudu/client/client-test.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tablet_service.h
4 files changed, 46 insertions(+), 19 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If4e9c69160605d4a839ac2d28def67f4d3402b90
Gerrit-Change-Number: 11516
Gerrit-PatchSet: 4
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot


[kudu-CR] tserver: log a message when a scanner is not found

2018-09-26 Thread Mike Percy (Code Review)
Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11516 )

Change subject: tserver: log a message when a scanner is not found
..


Patch Set 3:

(3 comments)

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

http://gerrit.cloudera.org:8080/#/c/11516/2//COMMIT_MSG@26
PS2, Line 26:
> nit: extra space
Done


http://gerrit.cloudera.org:8080/#/c/11516/3/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

http://gerrit.cloudera.org:8080/#/c/11516/3/src/kudu/tserver/tablet_service.cc@1271
PS3, Line 1271: "ScannerKeepAlive: " << s.ToString() << ": "
  :   << Substitute("remote=$0", 
context->requestor_string());
> nit: maybe, use Substitute() for the whole message here
Done


http://gerrit.cloudera.org:8080/#/c/11516/3/src/kudu/tserver/tablet_service.cc@2020
PS3, Line 2020: LOG(INFO) << "Scan: " << s.ToString() << ": "
  :   << Substitute("call sequence id=$0, remote=$1",
  : req->call_seq_id(), 
rpc_context->requestor_string());
> nit: maybe, use Substitute() for the whole message here
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If4e9c69160605d4a839ac2d28def67f4d3402b90
Gerrit-Change-Number: 11516
Gerrit-PatchSet: 3
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Wed, 26 Sep 2018 23:48:02 +
Gerrit-HasComments: Yes


[kudu-CR] tserver: log a message when a scanner is not found

2018-09-26 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11516 )

Change subject: tserver: log a message when a scanner is not found
..


Patch Set 3: Code-Review+2

(2 comments)

http://gerrit.cloudera.org:8080/#/c/11516/3/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

http://gerrit.cloudera.org:8080/#/c/11516/3/src/kudu/tserver/tablet_service.cc@1271
PS3, Line 1271: "ScannerKeepAlive: " << s.ToString() << ": "
  :   << Substitute("remote=$0", 
context->requestor_string());
nit: maybe, use Substitute() for the whole message here

LOG(INFO) << Substitute("ScannerKeepAlive: $0: remote=$1", ...);


http://gerrit.cloudera.org:8080/#/c/11516/3/src/kudu/tserver/tablet_service.cc@2020
PS3, Line 2020: LOG(INFO) << "Scan: " << s.ToString() << ": "
  :   << Substitute("call sequence id=$0, remote=$1",
  : req->call_seq_id(), 
rpc_context->requestor_string());
nit: maybe, use Substitute() for the whole message here

LOG(INFO) << Substitute("Scan: $0: call sequence id=$1, remote=$2", ...);



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If4e9c69160605d4a839ac2d28def67f4d3402b90
Gerrit-Change-Number: 11516
Gerrit-PatchSet: 3
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Wed, 26 Sep 2018 23:30:04 +
Gerrit-HasComments: Yes


[kudu-CR] [spark] Cleanup hasNext logic

2018-09-26 Thread Grant Henke (Code Review)
Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11529 )

Change subject: [spark] Cleanup hasNext logic
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/11529/1/java/kudu-client/src/main/java/org/apache/kudu/client/RowResultIterator.java@96
PS1, Line 96:   public static RowResultIterator empty() {
> Does this need to be annotated so as to be even more constrained than Publi
I though about marking it as @InterfaceAudiance.Private, but realistically 
being able to create an empty RowResultIterator is useful. The fact that I use 
it in the Spark integration implies that it would be useful to other 
integrations. Additionally it's not likely we would ever need to "break" this 
functionality.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I340af13a695b3a8489e03018542b8ad7db614758
Gerrit-Change-Number: 11529
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Wed, 26 Sep 2018 23:21:16 +
Gerrit-HasComments: Yes


[kudu-CR] [spark] Cleanup hasNext logic

2018-09-26 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11529 )

Change subject: [spark] Cleanup hasNext logic
..


Patch Set 1: Code-Review+2

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/11529/1/java/kudu-client/src/main/java/org/apache/kudu/client/RowResultIterator.java@96
PS1, Line 96:   public static RowResultIterator empty() {
Does this need to be annotated so as to be even more constrained than 
Public/Evolving? It seems like an implementation detail (i.e. not something 
that a 3rd party application should use).



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I340af13a695b3a8489e03018542b8ad7db614758
Gerrit-Change-Number: 11529
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Wed, 26 Sep 2018 23:19:18 +
Gerrit-HasComments: Yes


[kudu-CR] [spark] Cleanup hasNext logic

2018-09-26 Thread Grant Henke (Code Review)
Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11529 )

Change subject: [spark] Cleanup hasNext logic
..


Patch Set 1:

Sure, anything to eliminate unnecessary logic gymnastics. I would prefer to 
make that a separate patch though.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I340af13a695b3a8489e03018542b8ad7db614758
Gerrit-Change-Number: 11529
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Wed, 26 Sep 2018 23:15:37 +
Gerrit-HasComments: No


[kudu-CR] tserver: log a message when a scanner is not found

2018-09-26 Thread Mike Percy (Code Review)
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Grant Henke,

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

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

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

Change subject: tserver: log a message when a scanner is not found
..

tserver: log a message when a scanner is not found

Right now there is a class of issues where an "upstream" service or
query engine such as Impala or Spark may not be able to find a Kudu
scanner, resulting in a failed query. Often, this is because the scanner
timed out. The query engine will receive and typically log a message
indicating that the scanner could not be found, however it's difficult
to trace this back to the original query because the client wouldn't
know the scanner id and the server would not log the event.

With this change, it will be much easier to match up query engine
failures with expiring scanners by looking at the logs on both sides
because when a request comes in for a scanner that cannot be found, the
client will get a bad status:

  Not found: Scanner 7672e46ed30d42938c54bf6e7e24946e not found (it may have 
expired)

And the server will log a slightly more verbose message:

  I0926 14:59:50.600463 21137 tablet_service.cc:2020] Scan: Not found: Scanner 
7672e46ed30d42938c54bf6e7e24946e not found (it may have expired): call sequence 
id=1, remote={username='mpercy'} at 127.0.0.1:42660

Additionally, the above situation is handled in a similar way in the
case of a TabletService::ScannerKeepAlive() RPC call.

Change-Id: If4e9c69160605d4a839ac2d28def67f4d3402b90
---
M src/kudu/client/client-test.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tablet_service.h
4 files changed, 47 insertions(+), 19 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If4e9c69160605d4a839ac2d28def67f4d3402b90
Gerrit-Change-Number: 11516
Gerrit-PatchSet: 3
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot


[kudu-CR] KUDU-2541: Fill out basic sentry client API

2018-09-26 Thread Hao Hao (Code Review)
Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11524 )

Change subject: KUDU-2541: Fill out basic sentry client API
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1004a11506ed109d1f23389ca73d95a34c32c194
Gerrit-Change-Number: 11524
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Wed, 26 Sep 2018 22:59:08 +
Gerrit-HasComments: No


[kudu-CR] tserver: log a message when a scanner is not found

2018-09-26 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11516 )

Change subject: tserver: log a message when a scanner is not found
..


Patch Set 2:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/11516/2//COMMIT_MSG@26
PS2, Line 26:
nit: extra space


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

http://gerrit.cloudera.org:8080/#/c/11516/1/src/kudu/tserver/tablet_service.cc@2015
PS1, Line 2015: }
  :
  : *error_code = TabletServerErrorPB::SCANNER_EXPIRED;
> All right, that makes sense to me, thank you for the explanation.
Ah, it seems the anonymous variable nit has been addressed already.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If4e9c69160605d4a839ac2d28def67f4d3402b90
Gerrit-Change-Number: 11516
Gerrit-PatchSet: 2
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Wed, 26 Sep 2018 22:49:26 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-2541: add Kerberos support to Sentry client and mini-cluster

2018-09-26 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11525 )

Change subject: KUDU-2541: add Kerberos support to Sentry client and 
mini-cluster
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f7e27137135cb9b463a90b98e4aba864cece3c1
Gerrit-Change-Number: 11525
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 26 Sep 2018 22:46:16 +
Gerrit-HasComments: No


[kudu-CR] KUDU-2541: add Kerberos support to Sentry client and mini-cluster

2018-09-26 Thread Hao Hao (Code Review)
Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11525 )

Change subject: KUDU-2541: add Kerberos support to Sentry client and 
mini-cluster
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f7e27137135cb9b463a90b98e4aba864cece3c1
Gerrit-Change-Number: 11525
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 26 Sep 2018 22:44:40 +
Gerrit-HasComments: No


[kudu-CR] tserver: log a message when a scanner is not found

2018-09-26 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11516 )

Change subject: tserver: log a message when a scanner is not found
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/11516/1/src/kudu/tserver/tablet_service.cc@2015
PS1, Line 2015:   Status verbose = s.CloneAndAppend(Substitute("call 
sequence id=$0, remote=$1",
  :
req->call_seq_id(),
  :
rpc_context->requestor_string()));
> The returned Status message will show up in an Impala query error message,
All right, that makes sense to me, thank you for the explanation.

BTW, just another nit (feel free to ignore): if the 'verbose' is not referenced 
elsewhere but while producing the log message, maybe drop the 'verbose' 
variable and write something like

LOG(INFO) << s.CloneAndAppend(Substitute(
"call sequence id=$0, remote=$1",
req->call_seq_id(),
rpc_context->requestor_string()));



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If4e9c69160605d4a839ac2d28def67f4d3402b90
Gerrit-Change-Number: 11516
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Wed, 26 Sep 2018 22:39:37 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-428: add Sentry to thirdparty, mini-sentry

2018-09-26 Thread Hao Hao (Code Review)
Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11347 )

Change subject: KUDU-428: add Sentry to thirdparty, mini-sentry
..


Patch Set 6: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11347/5/thirdparty/vars.sh
File thirdparty/vars.sh:

http://gerrit.cloudera.org:8080/#/c/11347/5/thirdparty/vars.sh@230
PS5, Line 230: apache-sentry-$SENTRY_VERSION-bin
> I'm planning to do the repackaging in a follow up commit, Adar and I talked
Ah sorry, I missed that.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5
Gerrit-Change-Number: 11347
Gerrit-PatchSet: 6
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Wed, 26 Sep 2018 22:38:05 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-2541: add Kerberos support to Sentry client and mini-cluster

2018-09-26 Thread Dan Burkert (Code Review)
Hello Andrew Wong, Kudu Jenkins, Adar Dembo, Hao Hao,

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

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

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

Change subject: KUDU-2541: add Kerberos support to Sentry client and 
mini-cluster
..

KUDU-2541: add Kerberos support to Sentry client and mini-cluster

More infrastructure work towards KUDU-428.

Change-Id: I3f7e27137135cb9b463a90b98e4aba864cece3c1
---
M src/kudu/sentry/CMakeLists.txt
M src/kudu/sentry/mini_sentry.cc
M src/kudu/sentry/mini_sentry.h
M src/kudu/sentry/sentry_client-test.cc
4 files changed, 95 insertions(+), 10 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3f7e27137135cb9b463a90b98e4aba864cece3c1
Gerrit-Change-Number: 11525
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR] KUDU-2541: Fill out basic sentry client API

2018-09-26 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11524 )

Change subject: KUDU-2541: Fill out basic sentry client API
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1004a11506ed109d1f23389ca73d95a34c32c194
Gerrit-Change-Number: 11524
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Wed, 26 Sep 2018 22:30:07 +
Gerrit-HasComments: No


[kudu-CR] KUDU-428: add Sentry to thirdparty, mini-sentry

2018-09-26 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11347 )

Change subject: KUDU-428: add Sentry to thirdparty, mini-sentry
..


Patch Set 6: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5
Gerrit-Change-Number: 11347
Gerrit-PatchSet: 6
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Wed, 26 Sep 2018 22:30:24 +
Gerrit-HasComments: No


[kudu-CR] [DOCS] Added a version notice for changing managed Kudu table names in Impala

2018-09-26 Thread Alexey Serbin (Code Review)
Alexey Serbin has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/11515 )

Change subject: [DOCS] Added a version notice for changing managed Kudu table 
names in Impala
..

[DOCS] Added a version notice for changing managed Kudu table names in Impala

With IMPALA-5654, users can no longer change kudu.table_name property
for managed Kudu tables in Impala.

Change-Id: If26f103d931dd7ed57b56cf34b0010d14c098928
Reviewed-on: http://gerrit.cloudera.org:8080/11515
Reviewed-by: Alexey Serbin 
Tested-by: Kudu Jenkins
---
M docs/kudu_impala_integration.adoc
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/11515
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: If26f103d931dd7ed57b56cf34b0010d14c098928
Gerrit-Change-Number: 11515
Gerrit-PatchSet: 4
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 


[kudu-CR] KUDU-2541: Fill out basic sentry client API

2018-09-26 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11524 )

Change subject: KUDU-2541: Fill out basic sentry client API
..


Patch Set 3:

(10 comments)

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

http://gerrit.cloudera.org:8080/#/c/11524/1//COMMIT_MSG@9
PS1, Line 9: This commit contains more plumbing for the upcoming Sentry 
integration
> plumbing. Or plumbin'
Done


http://gerrit.cloudera.org:8080/#/c/11524/1//COMMIT_MSG@12
PS1, Line 12: Thrift'
> Thrift's
Done


http://gerrit.cloudera.org:8080/#/c/11524/1/src/kudu/sentry/mini_sentry.cc
File src/kudu/sentry/mini_sentry.cc:

http://gerrit.cloudera.org:8080/#/c/11524/1/src/kudu/sentry/mini_sentry.cc@142
PS1, Line 142: Hadoop'
> Hadoop's
Done


http://gerrit.cloudera.org:8080/#/c/11524/1/src/kudu/sentry/mini_sentry.cc@147
PS1, Line 147: hav
> have
Done


http://gerrit.cloudera.org:8080/#/c/11524/1/src/kudu/sentry/mini_sentry.cc@199
PS1, Line 199:   // Simple file format containing mapping of user to groups in 
INI syntax, see
> Maybe a short comment documenting the effect these values have?
Done


http://gerrit.cloudera.org:8080/#/c/11524/2/src/kudu/sentry/sentry_client-test.cc
File src/kudu/sentry/sentry_client-test.cc:

http://gerrit.cloudera.org:8080/#/c/11524/2/src/kudu/sentry/sentry_client-test.cc@53
PS2, Line 53: TEST_F(SentryClientTest, TestCreateDropRole) {
> Other simple tests for error conversion:
Done


http://gerrit.cloudera.org:8080/#/c/11524/2/src/kudu/sentry/sentry_client-test.cc@61
PS2, Line 61: ::sentry::TCreateSentryRoleRequest req;
> How do you feel about building simple Kudu abstractions for the request/res
Right, the way this works on the HMS side is that we have the base hms-client 
which exposes the raw types, then a higher level component which provides a 
nicer API on top of that (hms-catalog).  I'm planning to do the same thing for 
Sentry.


http://gerrit.cloudera.org:8080/#/c/11524/1/src/kudu/sentry/sentry_client.h
File src/kudu/sentry/sentry_client.h:

http://gerrit.cloudera.org:8080/#/c/11524/1/src/kudu/sentry/sentry_client.h@60
PS1, Line 60:  thrift
> provided
Done


http://gerrit.cloudera.org:8080/#/c/11524/1/src/kudu/sentry/sentry_client.h@61
PS1, Line 61:
> sentry
Done


http://gerrit.cloudera.org:8080/#/c/11524/1/src/kudu/sentry/sentry_client.h@61
PS1, Line 61:   ~SentryClient();
> warning: function 'kudu::sentry::SentryClient::SentryClient' has a definiti
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1004a11506ed109d1f23389ca73d95a34c32c194
Gerrit-Change-Number: 11524
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Wed, 26 Sep 2018 22:25:50 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-428: add Sentry to thirdparty, mini-sentry

2018-09-26 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11347 )

Change subject: KUDU-428: add Sentry to thirdparty, mini-sentry
..


Patch Set 6:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/11347/5/src/kudu/sentry/mini_sentry.cc
File src/kudu/sentry/mini_sentry.cc:

http://gerrit.cloudera.org:8080/#/c/11347/5/src/kudu/sentry/mini_sentry.cc@86
PS5, Line 86: Su
> nit: 4 space indent?
Done


http://gerrit.cloudera.org:8080/#/c/11347/5/thirdparty/vars.sh
File thirdparty/vars.sh:

http://gerrit.cloudera.org:8080/#/c/11347/5/thirdparty/vars.sh@230
PS5, Line 230: apache-sentry-$SENTRY_VERSION-bin
> Can we use the same naming rule as for Hive and Hadoop?
I'm planning to do the repackaging in a follow up commit, Adar and I talked 
about it on a different thread in this review.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5
Gerrit-Change-Number: 11347
Gerrit-PatchSet: 6
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Wed, 26 Sep 2018 22:25:37 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-428: add Sentry to thirdparty, mini-sentry

2018-09-26 Thread Dan Burkert (Code Review)
Hello Tidy Bot, Andrew Wong, Kudu Jenkins, Andrew Wong, Adar Dembo, Hao Hao,

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

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

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

Change subject: KUDU-428: add Sentry to thirdparty, mini-sentry
..

KUDU-428: add Sentry to thirdparty, mini-sentry

This commit adds Sentry to thirdparty, and fills out the MiniSentry
class with an initial implementation. Notable features that aren't
implemented:

- Stripped Sentry packaging. I've put an unmodified version of Sentry
  2.0.1 into thirdparty. It weighs in at almost 200MiB and takes about
  5s to startup on my laptop. We will probably want to add a stripped
  version later to reduce both of these.

- Kerberos support for mini-sentry. Right now Kerberos is disabled,
  which is an atypical configuration. A follow-up commit will add a
  Kerberos support configuration taking advantage of the mini KDC.

- The mini Sentry is not yet configured with the location of the HMS,
  which will be necessary to do anything non-trivial with it.

Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5
---
M build-support/dist_test.py
M build-support/run_dist_test.py
M src/kudu/hms/mini_hms.cc
M src/kudu/sentry/CMakeLists.txt
M src/kudu/sentry/mini_sentry.cc
M src/kudu/sentry/mini_sentry.h
M src/kudu/sentry/sentry_client-test.cc
M src/kudu/util/test_util.cc
M src/kudu/util/test_util.h
M thirdparty/build-thirdparty.sh
M thirdparty/download-thirdparty.sh
M thirdparty/vars.sh
12 files changed, 281 insertions(+), 43 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5
Gerrit-Change-Number: 11347
Gerrit-PatchSet: 6
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot


[kudu-CR] KUDU-2541: Fill out basic sentry client API

2018-09-26 Thread Dan Burkert (Code Review)
Hello Tidy Bot, Andrew Wong, Kudu Jenkins, Adar Dembo, Hao Hao,

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

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

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

Change subject: KUDU-2541: Fill out basic sentry client API
..

KUDU-2541: Fill out basic sentry client API

This commit contains more plumbing for the upcoming Sentry integration
(KUDU-428). The Sentry client class is filled out with enough methods to
ensure the Thrift RPC is working correctly, as well as error conversion
from Sentry and Thrift's format to Kudu's Status.

Change-Id: I1004a11506ed109d1f23389ca73d95a34c32c194
---
M src/kudu/sentry/CMakeLists.txt
M src/kudu/sentry/mini_sentry.cc
M src/kudu/sentry/mini_sentry.h
M src/kudu/sentry/sentry_client-test.cc
M src/kudu/sentry/sentry_client.cc
M src/kudu/sentry/sentry_client.h
6 files changed, 299 insertions(+), 9 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1004a11506ed109d1f23389ca73d95a34c32c194
Gerrit-Change-Number: 11524
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot


[kudu-CR] create-table-stress-test: deflake TestConcurrentCreateTableAndReloadMetadata

2018-09-26 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11523 )

Change subject: create-table-stress-test: deflake 
TestConcurrentCreateTableAndReloadMetadata
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I607a8e278e8e8f370cda512f365a97fbb81e1ae3
Gerrit-Change-Number: 11523
Gerrit-PatchSet: 3
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 26 Sep 2018 21:12:07 +
Gerrit-HasComments: No


[kudu-CR] [spark] Cleanup hasNext logic

2018-09-26 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11529 )

Change subject: [spark] Cleanup hasNext logic
..


Patch Set 1:

If you think that's bad, take a look at 
https://github.com/apache/kudu/blob/master/java/kudu-mapreduce/src/main/java/org/apache/kudu/mapreduce/KuduTableInputFormat.java#L428.
 Maybe that's something you can fix too?


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I340af13a695b3a8489e03018542b8ad7db614758
Gerrit-Change-Number: 11529
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Wed, 26 Sep 2018 22:04:23 +
Gerrit-HasComments: No


[kudu-CR] tserver: log a message when a scanner is not found

2018-09-26 Thread Mike Percy (Code Review)
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Grant Henke,

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

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

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

Change subject: tserver: log a message when a scanner is not found
..

tserver: log a message when a scanner is not found

Right now there is a class of issues where an "upstream" service or
query engine such as Impala or Spark may not be able to find a Kudu
scanner, resulting in a failed query. Often, this is because the scanner
timed out. The query engine will receive and typically log a message
indicating that the scanner could not be found, however it's difficult
to trace this back to the original query because the client wouldn't
know the scanner id and the server would not log the event.

With this change, it will be much easier to match up query engine
failures with expiring scanners by looking at the logs on both sides
because when a request comes in for a scanner that cannot be found, the
client will get a bad status like so:

  Not found: Scanner f87cc39d55d24c52bcf547f211e46b8b not found (it may have 
expired)

And the server will log a slightly more verbose message like:

  I0925 15:29:38.875579 22293 tablet_service.cc:2017] Not found: Scanner 
f87cc39d55d24c52bcf547f211e46b8b not found  (it may have expired): call 
sequence id=1, remote={username='mpercy'} at 127.0.0.1:57118

Change-Id: If4e9c69160605d4a839ac2d28def67f4d3402b90
---
M src/kudu/client/client-test.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tablet_service.h
4 files changed, 47 insertions(+), 19 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If4e9c69160605d4a839ac2d28def67f4d3402b90
Gerrit-Change-Number: 11516
Gerrit-PatchSet: 2
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot


[kudu-CR] tserver: log a message when a scanner is not found

2018-09-26 Thread Mike Percy (Code Review)
Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11516 )

Change subject: tserver: log a message when a scanner is not found
..


Patch Set 1:

(3 comments)

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

http://gerrit.cloudera.org:8080/#/c/11516/1/src/kudu/tserver/tablet_service.cc@1268
PS1, Line 1268: StatusToPB(Status::NotFound("Scanner not found"), 
resp->mutable_error()->mutable_status());
> Does it make sense to add a log message here as well?
Nice catch, seems like a good idea. Done.


http://gerrit.cloudera.org:8080/#/c/11516/1/src/kudu/tserver/tablet_service.cc@2015
PS1, Line 2015:   Status verbose = s.CloneAndAppend(Substitute("call 
sequence id=$0, remote=$1",
  :
req->call_seq_id(),
  :
rpc_context->requestor_string()));
> Why not to put this information into the returned status as is?
The returned Status message will show up in an Impala query error message, and 
I think dumping excessive information onto the user would be more confusing 
than helpful. I don't think the verbose message will help most people because 
it probably wouldn't mean much to anyone who isn't a Kudu technical expert. The 
scanner id, on the other hand, is greppable so I think it's worth surfacing.


http://gerrit.cloudera.org:8080/#/c/11516/1/src/kudu/tserver/tablet_service.cc@2038
PS1, Line 2038:   if (req->call_seq_id() != scanner->call_seq_id()) {
  : *error_code = TabletServerErrorPB::INVALID_SCAN_CALL_SEQ_ID;
  : return Status::InvalidArgument("Invalid call sequence ID in 
scan request");
  :   }
> Some unrelated to this particular change, but maybe relevant stuff when cha
These are all great points. I'll follow up on those with a separate change list.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If4e9c69160605d4a839ac2d28def67f4d3402b90
Gerrit-Change-Number: 11516
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Wed, 26 Sep 2018 21:46:24 +
Gerrit-HasComments: Yes


[kudu-CR] [spark] Cleanup hasNext logic

2018-09-26 Thread Grant Henke (Code Review)
Grant Henke has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/11529


Change subject: [spark] Cleanup hasNext logic
..

[spark] Cleanup hasNext logic

When returning to the while loop in
RowIterator.hasNext() I always have to do a
double take on the logic. This change makes
the logic a bit more clear.

Change-Id: I340af13a695b3a8489e03018542b8ad7db614758
---
M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackupRDD.scala
M java/kudu-client/src/main/java/org/apache/kudu/client/RowResultIterator.java
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduRDD.scala
3 files changed, 5 insertions(+), 7 deletions(-)



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

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


[kudu-CR] create-table-stress-test: deflake TestConcurrentCreateTableAndReloadMetadata

2018-09-26 Thread Adar Dembo (Code Review)
Adar Dembo has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/11523 )

Change subject: create-table-stress-test: deflake 
TestConcurrentCreateTableAndReloadMetadata
..

create-table-stress-test: deflake TestConcurrentCreateTableAndReloadMetadata

First, if the ASSERT_OK fired, the test would hang. No bueno.

Second, in TSAN environments the test would sometimes fail with a table
already present error, typically happening either 30s or 60s after that
table was created. I believe this is due to a client-side timeout _after_
the table was created but _before_ its RPC response was sent. We could
try to address this by bumping the client's timeout, but because
reload_metadata_thread is DoS'ing the cluster, that doesn't seem robust.
Instead, let's have the thread ease up in TSAN environments.

Without the change, the test failed 4/1000 times (--stress_cpu_threads=4).

With the change, the test failed 0/1000 times (--stress_cpu_threads=8).

Change-Id: I607a8e278e8e8f370cda512f365a97fbb81e1ae3
Reviewed-on: http://gerrit.cloudera.org:8080/11523
Reviewed-by: Alexey Serbin 
Tested-by: Kudu Jenkins
---
M src/kudu/integration-tests/create-table-stress-test.cc
1 file changed, 19 insertions(+), 4 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I607a8e278e8e8f370cda512f365a97fbb81e1ae3
Gerrit-Change-Number: 11523
Gerrit-PatchSet: 4
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR] KUDU-428: add Sentry to thirdparty, mini-sentry

2018-09-26 Thread Hao Hao (Code Review)
Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11347 )

Change subject: KUDU-428: add Sentry to thirdparty, mini-sentry
..


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/11347/5/src/kudu/sentry/mini_sentry.cc
File src/kudu/sentry/mini_sentry.cc:

http://gerrit.cloudera.org:8080/#/c/11347/5/src/kudu/sentry/mini_sentry.cc@86
PS5, Line 86:
nit: 4 space indent?


http://gerrit.cloudera.org:8080/#/c/11347/5/thirdparty/vars.sh
File thirdparty/vars.sh:

http://gerrit.cloudera.org:8080/#/c/11347/5/thirdparty/vars.sh@230
PS5, Line 230: apache-sentry-$SENTRY_VERSION-bin
Can we use the same naming rule as for Hive and Hadoop?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5
Gerrit-Change-Number: 11347
Gerrit-PatchSet: 5
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Wed, 26 Sep 2018 21:29:15 +
Gerrit-HasComments: Yes


[kudu-CR] [DOCS] Added a version notice for changing managed Kudu table names in Impala

2018-09-26 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11515 )

Change subject: [DOCS] Added a version notice for changing managed Kudu table 
names in Impala
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If26f103d931dd7ed57b56cf34b0010d14c098928
Gerrit-Change-Number: 11515
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Wed, 26 Sep 2018 21:13:04 +
Gerrit-HasComments: No


[kudu-CR](gh-pages) gh-pages: Make site-tool Python 3 compatible

2018-09-26 Thread Mike Percy (Code Review)
Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11522 )

Change subject: gh-pages: Make site-tool Python 3 compatible
..


Patch Set 1: Verified+1


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

Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-MessageType: comment
Gerrit-Change-Id: I5e1f69c80e5f327b0d0d017bacf717a97f572e5b
Gerrit-Change-Number: 11522
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Wed, 26 Sep 2018 21:09:21 +
Gerrit-HasComments: No


[kudu-CR](gh-pages) gh-pages: Make site-tool Python 3 compatible

2018-09-26 Thread Mike Percy (Code Review)
Mike Percy has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/11522 )

Change subject: gh-pages: Make site-tool Python 3 compatible
..

gh-pages: Make site-tool Python 3 compatible

Change-Id: I5e1f69c80e5f327b0d0d017bacf717a97f572e5b
Reviewed-on: http://gerrit.cloudera.org:8080/11522
Reviewed-by: Adar Dembo 
Tested-by: Mike Percy 
---
M _tools/kudu_util.py
M site_tool
2 files changed, 26 insertions(+), 22 deletions(-)

Approvals:
  Adar Dembo: Looks good to me, approved
  Mike Percy: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-MessageType: merged
Gerrit-Change-Id: I5e1f69c80e5f327b0d0d017bacf717a97f572e5b
Gerrit-Change-Number: 11522
Gerrit-PatchSet: 2
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Mike Percy 


[kudu-CR] [DOCS] Added a version notice for changing managed Kudu table names in Impala

2018-09-26 Thread Alex Rodoni (Code Review)
Alex Rodoni has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11515 )

Change subject: [DOCS] Added a version notice for changing managed Kudu table 
names in Impala
..


Patch Set 2:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/11515/2//COMMIT_MSG@7
PS2, Line 7: ]
> nit: an extra closing brace; please remove it
Done


http://gerrit.cloudera.org:8080/#/c/11515/2//COMMIT_MSG@9
PS2, Line 9: With IMPALA-5654, users cannot change kudu.table_name in IMPALA for
   : managed Kudu tables created in Impala.
> It looks like nitpicking, but this sentence contains mention of Impala twic
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If26f103d931dd7ed57b56cf34b0010d14c098928
Gerrit-Change-Number: 11515
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Wed, 26 Sep 2018 21:08:34 +
Gerrit-HasComments: Yes


[kudu-CR] [DOCS] Added a version notice for changing managed Kudu table names in Impala

2018-09-26 Thread Alex Rodoni (Code Review)
Hello Mike Percy, Alexey Serbin, Kudu Jenkins,

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

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

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

Change subject: [DOCS] Added a version notice for changing managed Kudu table 
names in Impala
..

[DOCS] Added a version notice for changing managed Kudu table names in Impala

With IMPALA-5654, users can no longer change kudu.table_name property
for managed Kudu tables in Impala.

Change-Id: If26f103d931dd7ed57b56cf34b0010d14c098928
---
M docs/kudu_impala_integration.adoc
1 file changed, 2 insertions(+), 2 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If26f103d931dd7ed57b56cf34b0010d14c098928
Gerrit-Change-Number: 11515
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 


[kudu-CR] KUDU-2541: add Kerberos support to Sentry client and mini-cluster

2018-09-26 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11525 )

Change subject: KUDU-2541: add Kerberos support to Sentry client and 
mini-cluster
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f7e27137135cb9b463a90b98e4aba864cece3c1
Gerrit-Change-Number: 11525
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 26 Sep 2018 20:42:19 +
Gerrit-HasComments: No


[kudu-CR] KUDU-428: add Sentry to thirdparty, mini-sentry

2018-09-26 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11347 )

Change subject: KUDU-428: add Sentry to thirdparty, mini-sentry
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5
Gerrit-Change-Number: 11347
Gerrit-PatchSet: 4
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Wed, 26 Sep 2018 20:38:19 +
Gerrit-HasComments: No


[kudu-CR] KUDU-2541: Fill out basic sentry client API

2018-09-26 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11524 )

Change subject: KUDU-2541: Fill out basic sentry client API
..


Patch Set 2:

(8 comments)

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

http://gerrit.cloudera.org:8080/#/c/11524/1//COMMIT_MSG@9
PS1, Line 9: This commit contains more plumbin for the upcoming Sentry 
integration
plumbing. Or plumbin'


http://gerrit.cloudera.org:8080/#/c/11524/1//COMMIT_MSG@12
PS1, Line 12: Thrifts
Thrift's


http://gerrit.cloudera.org:8080/#/c/11524/1/src/kudu/sentry/mini_sentry.cc
File src/kudu/sentry/mini_sentry.cc:

http://gerrit.cloudera.org:8080/#/c/11524/1/src/kudu/sentry/mini_sentry.cc@142
PS1, Line 142: Hadoops
Hadoop's


http://gerrit.cloudera.org:8080/#/c/11524/1/src/kudu/sentry/mini_sentry.cc@147
PS1, Line 147: has
have


http://gerrit.cloudera.org:8080/#/c/11524/1/src/kudu/sentry/mini_sentry.cc@199
PS1, Line 199:   static const string kUsers = R"(
Maybe a short comment documenting the effect these values have?


http://gerrit.cloudera.org:8080/#/c/11524/2/src/kudu/sentry/sentry_client-test.cc
File src/kudu/sentry/sentry_client-test.cc:

http://gerrit.cloudera.org:8080/#/c/11524/2/src/kudu/sentry/sentry_client-test.cc@53
PS2, Line 53: TEST_F(SentryClientTest, TestCreateDropRole) {
Other simple tests for error conversion:
1. Create a duplicate role. AlreadyPresent.
2. Drop a non-existent role. NotFound.


http://gerrit.cloudera.org:8080/#/c/11524/2/src/kudu/sentry/sentry_client-test.cc@61
PS2, Line 61: ::sentry::TCreateSentryRoleRequest req;
How do you feel about building simple Kudu abstractions for the 
request/response classes? To avoid having ::sentry stuff leak out into the rest 
of Kudu?

Looks like we didn't do that for HMS integration though.


http://gerrit.cloudera.org:8080/#/c/11524/1/src/kudu/sentry/sentry_client.h
File src/kudu/sentry/sentry_client.h:

http://gerrit.cloudera.org:8080/#/c/11524/1/src/kudu/sentry/sentry_client.h@60
PS1, Line 60: proided
provided



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1004a11506ed109d1f23389ca73d95a34c32c194
Gerrit-Change-Number: 11524
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Wed, 26 Sep 2018 20:36:52 +
Gerrit-HasComments: Yes


[kudu-CR] [java] Fix kudu-spark build failure

2018-09-26 Thread Grant Henke (Code Review)
Grant Henke has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/11526 )

Change subject: [java] Fix kudu-spark build failure
..

[java] Fix kudu-spark build failure

Fixes a build break due to 2 patches comming in at
similar times without a rebase.

Change-Id: Id362bec3e6629c519ec908a2c29b5438b6bddee8
Reviewed-on: http://gerrit.cloudera.org:8080/11526
Reviewed-by: Adar Dembo 
Tested-by: Kudu Jenkins
---
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/StreamingTest.scala
1 file changed, 2 insertions(+), 2 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Id362bec3e6629c519ec908a2c29b5438b6bddee8
Gerrit-Change-Number: 11526
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR] KUDU-2541: add Kerberos support to Sentry client and mini-cluster

2018-09-26 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11525 )

Change subject: KUDU-2541: add Kerberos support to Sentry client and 
mini-cluster
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11525/1/src/kudu/sentry/sentry_client-test.cc
File src/kudu/sentry/sentry_client-test.cc:

http://gerrit.cloudera.org:8080/#/c/11525/1/src/kudu/sentry/sentry_client-test.cc@57
PS1, Line 57:
> Move directly under the test class.
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f7e27137135cb9b463a90b98e4aba864cece3c1
Gerrit-Change-Number: 11525
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 26 Sep 2018 20:25:56 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-2541: Fill out basic sentry client API

2018-09-26 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11524 )

Change subject: KUDU-2541: Fill out basic sentry client API
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11524/1/src/kudu/sentry/sentry_client.cc
File src/kudu/sentry/sentry_client.cc:

http://gerrit.cloudera.org:8080/#/c/11524/1/src/kudu/sentry/sentry_client.cc@80
PS1, Line 80:   }
> warning: do not use 'else' after 'return' [readability-else-after-return]
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1004a11506ed109d1f23389ca73d95a34c32c194
Gerrit-Change-Number: 11524
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Wed, 26 Sep 2018 20:25:44 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-428: add Sentry to thirdparty, mini-sentry

2018-09-26 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11347 )

Change subject: KUDU-428: add Sentry to thirdparty, mini-sentry
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11347/3/src/kudu/hms/mini_hms.cc
File src/kudu/hms/mini_hms.cc:

http://gerrit.cloudera.org:8080/#/c/11347/3/src/kudu/hms/mini_hms.cc@77
PS3, Line 77:   data_root_ = std::move(data_root);
> warning: parameter 'data_root' is passed by value and only copied once; con
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5
Gerrit-Change-Number: 11347
Gerrit-PatchSet: 4
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Wed, 26 Sep 2018 20:25:38 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-2541: add Kerberos support to Sentry client and mini-cluster

2018-09-26 Thread Dan Burkert (Code Review)
Hello Andrew Wong, Kudu Jenkins, Adar Dembo, Hao Hao,

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

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

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

Change subject: KUDU-2541: add Kerberos support to Sentry client and 
mini-cluster
..

KUDU-2541: add Kerberos support to Sentry client and mini-cluster

More infrastructure work towards KUDU-428.

Change-Id: I3f7e27137135cb9b463a90b98e4aba864cece3c1
---
M src/kudu/sentry/CMakeLists.txt
M src/kudu/sentry/mini_sentry.cc
M src/kudu/sentry/mini_sentry.h
M src/kudu/sentry/sentry_client-test.cc
4 files changed, 94 insertions(+), 10 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3f7e27137135cb9b463a90b98e4aba864cece3c1
Gerrit-Change-Number: 11525
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR] KUDU-2541: Fill out basic sentry client API

2018-09-26 Thread Dan Burkert (Code Review)
Hello Tidy Bot, Andrew Wong, Kudu Jenkins, Adar Dembo, Hao Hao,

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

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

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

Change subject: KUDU-2541: Fill out basic sentry client API
..

KUDU-2541: Fill out basic sentry client API

This commit contains more plumbin for the upcoming Sentry integration
(KUDU-428). The Sentry client class is filled out with enough methods to
ensure the Thrift RPC is working correctly, as well as error conversion
from Sentry and Thrifts format to Kudu's Status.

Change-Id: I1004a11506ed109d1f23389ca73d95a34c32c194
---
M src/kudu/sentry/CMakeLists.txt
M src/kudu/sentry/mini_sentry.cc
M src/kudu/sentry/mini_sentry.h
M src/kudu/sentry/sentry_client-test.cc
M src/kudu/sentry/sentry_client.cc
M src/kudu/sentry/sentry_client.h
6 files changed, 284 insertions(+), 9 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1004a11506ed109d1f23389ca73d95a34c32c194
Gerrit-Change-Number: 11524
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot


[kudu-CR] KUDU-428: add Sentry to thirdparty, mini-sentry

2018-09-26 Thread Dan Burkert (Code Review)
Hello Tidy Bot, Andrew Wong, Kudu Jenkins, Andrew Wong, Adar Dembo, Hao Hao,

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

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

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

Change subject: KUDU-428: add Sentry to thirdparty, mini-sentry
..

KUDU-428: add Sentry to thirdparty, mini-sentry

This commit adds Sentry to thirdparty, and fills out the MiniSentry
class with an initial implementation. Notable features that aren't
implemented:

- Stripped Sentry packaging. I've put an unmodified version of Sentry
  2.0.1 into thirdparty. It weighs in at almost 200MiB and takes about
  5s to startup on my laptop. We will probably want to add a stripped
  version later to reduce both of these.

- Kerberos support for mini-sentry. Right now Kerberos is disabled,
  which is an atypical configuration. A follow-up commit will add a
  Kerberos support configuration taking advantage of the mini KDC.

- The mini Sentry is not yet configured with the location of the HMS,
  which will be necessary to do anything non-trivial with it.

Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5
---
M build-support/dist_test.py
M build-support/run_dist_test.py
M src/kudu/hms/mini_hms.cc
M src/kudu/sentry/CMakeLists.txt
M src/kudu/sentry/mini_sentry.cc
M src/kudu/sentry/mini_sentry.h
M src/kudu/sentry/sentry_client-test.cc
M src/kudu/util/test_util.cc
M src/kudu/util/test_util.h
M thirdparty/build-thirdparty.sh
M thirdparty/download-thirdparty.sh
M thirdparty/vars.sh
12 files changed, 281 insertions(+), 43 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5
Gerrit-Change-Number: 11347
Gerrit-PatchSet: 4
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot


[kudu-CR] KUDU-428: add Sentry to thirdparty, mini-sentry

2018-09-26 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11347 )

Change subject: KUDU-428: add Sentry to thirdparty, mini-sentry
..


Patch Set 3: Code-Review+2

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/11347/2//COMMIT_MSG@13
PS2, Line 13: - Stripped Sentry packaging. I've put an unmodified version of 
Sentry
:   2.0.1 into thirdparty. It weighs in at almost 200MiB and takes 
about
:   5s to startup on my laptop. We will probably want to add a 
stripped
:   version later to reduce both of these.
> Also I should note that it appears that Sentry's startup times are much bet
5s startup is still pretty bad, though I suppose that's on par with the 
(stripped) HMS.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5
Gerrit-Change-Number: 11347
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Wed, 26 Sep 2018 20:03:53 +
Gerrit-HasComments: Yes


[kudu-CR] create-table-stress-test: deflake TestConcurrentCreateTableAndReloadMetadata

2018-09-26 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11523 )

Change subject: create-table-stress-test: deflake 
TestConcurrentCreateTableAndReloadMetadata
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I607a8e278e8e8f370cda512f365a97fbb81e1ae3
Gerrit-Change-Number: 11523
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 26 Sep 2018 19:30:14 +
Gerrit-HasComments: No


[kudu-CR] [java] Fix kudu-spark build failure

2018-09-26 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11526 )

Change subject: [java] Fix kudu-spark build failure
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id362bec3e6629c519ec908a2c29b5438b6bddee8
Gerrit-Change-Number: 11526
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 26 Sep 2018 19:11:12 +
Gerrit-HasComments: No


[kudu-CR] KUDU-428: add Sentry to thirdparty, mini-sentry

2018-09-26 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11347 )

Change subject: KUDU-428: add Sentry to thirdparty, mini-sentry
..


Patch Set 3:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/11347/2//COMMIT_MSG@13
PS2, Line 13: - Stripped Sentry packaging. I've put an unmodified version of 
Sentry
:   2.0.1 into thirdparty. It weighs in at almost 200MiB and takes 
about
:   5s to startup on my laptop. We will probably want to add a 
stripped
:   version later to reduce both of these.
> This is difficult to frontload, because we can't know all the necessary lib
Also I should note that it appears that Sentry's startup times are much better 
than the HMS's, which was the original motivation for repackaging.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5
Gerrit-Change-Number: 11347
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 26 Sep 2018 19:07:50 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-2541: add Kerberos support to Sentry client and mini-cluster

2018-09-26 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11525 )

Change subject: KUDU-2541: add Kerberos support to Sentry client and 
mini-cluster
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11525/1/src/kudu/sentry/sentry_client-test.cc
File src/kudu/sentry/sentry_client-test.cc:

http://gerrit.cloudera.org:8080/#/c/11525/1/src/kudu/sentry/sentry_client-test.cc@57
PS1, Line 57: INSTANTIATE_TEST_CASE_P(KerberosEnabled, SentryClientTest, 
::testing::Bool());
Move directly under the test class.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f7e27137135cb9b463a90b98e4aba864cece3c1
Gerrit-Change-Number: 11525
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 26 Sep 2018 19:06:55 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-2541: Fill out basic sentry client API

2018-09-26 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11524 )

Change subject: KUDU-2541: Fill out basic sentry client API
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11524/1/src/kudu/sentry/sentry_client.h
File src/kudu/sentry/sentry_client.h:

http://gerrit.cloudera.org:8080/#/c/11524/1/src/kudu/sentry/sentry_client.h@61
PS1, Line 61: hms_address
sentry



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1004a11506ed109d1f23389ca73d95a34c32c194
Gerrit-Change-Number: 11524
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 26 Sep 2018 19:05:19 +
Gerrit-HasComments: Yes


[kudu-CR] [java] Fix kudu-spark build failure

2018-09-26 Thread Grant Henke (Code Review)
Grant Henke has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/11526


Change subject: [java] Fix kudu-spark build failure
..

[java] Fix kudu-spark build failure

Fixes a build break due to 2 patches comming in at
similar times without a rebase.

Change-Id: Id362bec3e6629c519ec908a2c29b5438b6bddee8
---
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/StreamingTest.scala
1 file changed, 2 insertions(+), 2 deletions(-)



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

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


[kudu-CR] create-table-stress-test: deflake TestConcurrentCreateTableAndReloadMetadata

2018-09-26 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11523 )

Change subject: create-table-stress-test: deflake 
TestConcurrentCreateTableAndReloadMetadata
..


Patch Set 2:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/11523/1//COMMIT_MSG@13
PS1, Line 13: I believe this is due to a client-side timeout _after_
: the table was created but _before_ its RPC response was sent
> Could you add a note about the possibility of this scenario as a comment in
Sure.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I607a8e278e8e8f370cda512f365a97fbb81e1ae3
Gerrit-Change-Number: 11523
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 26 Sep 2018 19:03:10 +
Gerrit-HasComments: Yes


[kudu-CR] create-table-stress-test: deflake TestConcurrentCreateTableAndReloadMetadata

2018-09-26 Thread Adar Dembo (Code Review)
Hello Alexey Serbin, Kudu Jenkins,

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

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

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

Change subject: create-table-stress-test: deflake 
TestConcurrentCreateTableAndReloadMetadata
..

create-table-stress-test: deflake TestConcurrentCreateTableAndReloadMetadata

First, if the ASSERT_OK fired, the test would hang. No bueno.

Second, in TSAN environments the test would sometimes fail with a table
already present error, typically happening either 30s or 60s after that
table was created. I believe this is due to a client-side timeout _after_
the table was created but _before_ its RPC response was sent. We could
try to address this by bumping the client's timeout, but because
reload_metadata_thread is DoS'ing the cluster, that doesn't seem robust.
Instead, let's have the thread ease up in TSAN environments.

Without the change, the test failed 4/1000 times (--stress_cpu_threads=4).

With the change, the test failed 0/1000 times (--stress_cpu_threads=8).

Change-Id: I607a8e278e8e8f370cda512f365a97fbb81e1ae3
---
M src/kudu/integration-tests/create-table-stress-test.cc
1 file changed, 19 insertions(+), 4 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I607a8e278e8e8f370cda512f365a97fbb81e1ae3
Gerrit-Change-Number: 11523
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR] KUDU-428: add Sentry to thirdparty, mini-sentry

2018-09-26 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11347 )

Change subject: KUDU-428: add Sentry to thirdparty, mini-sentry
..


Patch Set 3:

(9 comments)

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

http://gerrit.cloudera.org:8080/#/c/11347/2//COMMIT_MSG@13
PS2, Line 13: - Stripped Sentry packaging. I've put an unmodified version of 
Sentry
:   2.0.1 into thirdparty. It weighs in at almost 200MiB and takes 
about
:   5s to startup on my laptop. We will probably want to add a 
stripped
:   version later to reduce both of these.
> The Sentry integration isn't in any particular hurry, right? Maybe we can f
This is difficult to frontload, because we can't know all the necessary 
libraries in the tarball until we are exercising all of the sentry 
functionality we need.  Unfortunately it doesn't look like there will be 'easy' 
wins with Sentry like there were for Hadoop where the share/ (docs) directory 
was huge.  This needs to be revisited after we hook up more of the 
functionality.


http://gerrit.cloudera.org:8080/#/c/11347/2//COMMIT_MSG@22
PS2, Line 22: The mini Sentry is not yet configured with the location of the 
HMS,
:   which will be necessary to do anything non-trivial with it.
> So it's impossible to use ephemeral ports for both? Do either allow already
I agree with all of this.  I'm going to leave it to a follow-up commit to 
implement it, since it's not strictly necessary to do in this commit.


http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/sentry/mini_sentry.h
File src/kudu/sentry/mini_sentry.h:

http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/sentry/mini_sentry.h@47
PS2, Line 47:   // Stops the mini Sentry service.
:   Status Stop() WARN_UNUSED_RESULT;
:
:   // Pause the Sentry service.
:   Status Pause() WARN_UNUSED_RESULT;
:
:   // Unpause the Sentry service.
:   Status Resume() WARN_UNUSED_RESULT;
> At some point we should consider inserting MiniSentry, MiniHMS, and MiniKDC
I'd prefer to wait for a situation in which we want to work with these 
components in a generic way, otherwise the hierarchy will just be boilerplate.


http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/sentry/mini_sentry.cc
File src/kudu/sentry/mini_sentry.cc:

http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/sentry/mini_sentry.cc@88
PS2, Line 88: JoinPathSegments(tmp_dir, "sentry-site.xm
> nit: use JoinPathSegments?
Done


http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/sentry/mini_sentry.cc@142
PS2, Line 142:   
 : sentry.service.security.mode
 : none
 :   
> Right, I think the purpose if the flag is to allow Sentry to start up in a
Yep.  A follow-up commit will flip this between the none and kerberos value 
depending on whether kerberos is enabled.


http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/sentry/mini_sentry.cc@169
PS2, Line 169: Substitut
> nit: not needed
Done


http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/sentry/mini_sentry.cc@173
PS2, Line 173: JoinPathSegments(tmp_dir, "sentry-site.xml")
> Do you foresee CreateSentrySite() being used with any directory besides `Ge
No, however there are multiple things which need the raw tmp_dir (line 169 
here, as well as an additional file in a follow-up commit).


http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/sentry/sentry_client-test.cc
File src/kudu/sentry/sentry_client-test.cc:

http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/sentry/sentry_client-test.cc@46
PS2, Line 46:
:
:
:
> As useful as this was, you can remove it now.
Done


http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/util/test_util.cc
File src/kudu/util/test_util.cc:

http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/util/test_util.cc@453
PS2, Line 453:   string dir = env == nullptr ? JoinPathSegments(bin_dir, 
Substitute("$0-home", name)) : env
> nit: Maybe don't set this if we're returning an error? Or document that it
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5
Gerrit-Change-Number: 11347
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 26 Sep 2018 19:01:09 +
Gerrit-HasComments: Yes


[kudu-CR] ][DOCS] Added a version notice for changing managed Kudu table names in Impala

2018-09-26 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11515 )

Change subject: ][DOCS] Added a version notice for changing managed Kudu table 
names in Impala
..


Patch Set 2:

(2 comments)

> (4 comments)

Thank you for the fix!  I think it's just a couple of nits to correct and it's 
good to go.

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

http://gerrit.cloudera.org:8080/#/c/11515/2//COMMIT_MSG@7
PS2, Line 7: ]
nit: an extra closing brace; please remove it


http://gerrit.cloudera.org:8080/#/c/11515/2//COMMIT_MSG@9
PS2, Line 9: With IMPALA-5654, users cannot change kudu.table_name in IMPALA for
   : managed Kudu tables created in Impala.
It looks like nitpicking, but this sentence contains mention of Impala twice, 
first as 'IMPALA' and then as 'Impala' (I don't count the reference to Impala 
JIRA ticket).

How about:

With IMPALA-5654, users can no longer change kudu.table_name property for 
managed Kudu tables in Impala.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If26f103d931dd7ed57b56cf34b0010d14c098928
Gerrit-Change-Number: 11515
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Wed, 26 Sep 2018 18:58:21 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-2541: Fill out basic sentry client API

2018-09-26 Thread Dan Burkert (Code Review)
Hello Andrew Wong, Adar Dembo, Hao Hao,

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

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

to review the following change.


Change subject: KUDU-2541: Fill out basic sentry client API
..

KUDU-2541: Fill out basic sentry client API

This commit contains more plumbin for the upcoming Sentry integration
(KUDU-428). The Sentry client class is filled out with enough methods to
ensure the Thrift RPC is working correctly, as well as error conversion
from Sentry and Thrifts format to Kudu's Status.

Change-Id: I1004a11506ed109d1f23389ca73d95a34c32c194
---
M src/kudu/sentry/CMakeLists.txt
M src/kudu/sentry/mini_sentry.cc
M src/kudu/sentry/mini_sentry.h
M src/kudu/sentry/sentry_client-test.cc
M src/kudu/sentry/sentry_client.cc
M src/kudu/sentry/sentry_client.h
6 files changed, 276 insertions(+), 9 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1004a11506ed109d1f23389ca73d95a34c32c194
Gerrit-Change-Number: 11524
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Hao Hao 


[kudu-CR] KUDU-428: add Sentry to thirdparty, mini-sentry

2018-09-26 Thread Dan Burkert (Code Review)
Hello Andrew Wong, Kudu Jenkins, Andrew Wong, Adar Dembo, Hao Hao,

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

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

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

Change subject: KUDU-428: add Sentry to thirdparty, mini-sentry
..

KUDU-428: add Sentry to thirdparty, mini-sentry

This commit adds Sentry to thirdparty, and fills out the MiniSentry
class with an initial implementation. Notable features that aren't
implemented:

- Stripped Sentry packaging. I've put an unmodified version of Sentry
  2.0.1 into thirdparty. It weighs in at almost 200MiB and takes about
  5s to startup on my laptop. We will probably want to add a stripped
  version later to reduce both of these.

- Kerberos support for mini-sentry. Right now Kerberos is disabled,
  which is an atypical configuration. A follow-up commit will add a
  Kerberos support configuration taking advantage of the mini KDC.

- The mini Sentry is not yet configured with the location of the HMS,
  which will be necessary to do anything non-trivial with it.

Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5
---
M build-support/dist_test.py
M build-support/run_dist_test.py
M src/kudu/hms/mini_hms.cc
M src/kudu/sentry/CMakeLists.txt
M src/kudu/sentry/mini_sentry.cc
M src/kudu/sentry/mini_sentry.h
M src/kudu/sentry/sentry_client-test.cc
M src/kudu/util/test_util.cc
M src/kudu/util/test_util.h
M thirdparty/build-thirdparty.sh
M thirdparty/download-thirdparty.sh
M thirdparty/vars.sh
12 files changed, 280 insertions(+), 40 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5
Gerrit-Change-Number: 11347
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR] KUDU-2541: add Kerberos support to Sentry client and mini-cluster

2018-09-26 Thread Dan Burkert (Code Review)
Hello Andrew Wong, Adar Dembo, Hao Hao,

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

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

to review the following change.


Change subject: KUDU-2541: add Kerberos support to Sentry client and 
mini-cluster
..

KUDU-2541: add Kerberos support to Sentry client and mini-cluster

More infrastructure work towards KUDU-428.

Change-Id: I3f7e27137135cb9b463a90b98e4aba864cece3c1
---
M src/kudu/sentry/CMakeLists.txt
M src/kudu/sentry/mini_sentry.cc
M src/kudu/sentry/mini_sentry.h
M src/kudu/sentry/sentry_client-test.cc
4 files changed, 95 insertions(+), 10 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3f7e27137135cb9b463a90b98e4aba864cece3c1
Gerrit-Change-Number: 11525
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Hao Hao 


[kudu-CR] create-table-stress-test: deflake TestConcurrentCreateTableAndReloadMetadata

2018-09-26 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11523 )

Change subject: create-table-stress-test: deflake 
TestConcurrentCreateTableAndReloadMetadata
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/11523/1//COMMIT_MSG@13
PS1, Line 13: I believe this is due to a client-side timeout _after_
: the table was created but _before_ its RPC response was sent
Could you add a note about the possibility of this scenario as a comment in the 
test's code around corresponding ASSERT_OK (line 395 in the original code)?

I think that might be useful while updating the test and might save some time 
troubleshooting the issue is the test fails again in the future due to the same 
problem.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I607a8e278e8e8f370cda512f365a97fbb81e1ae3
Gerrit-Change-Number: 11523
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 26 Sep 2018 18:47:41 +
Gerrit-HasComments: Yes


[kudu-CR] tserver: log a message when a scanner is not found

2018-09-26 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11516 )

Change subject: tserver: log a message when a scanner is not found
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/11516/1/src/kudu/tserver/tablet_service.cc@1268
PS1, Line 1268: StatusToPB(Status::NotFound("Scanner not found"), 
resp->mutable_error()->mutable_status());
Does it make sense to add a log message here as well?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If4e9c69160605d4a839ac2d28def67f4d3402b90
Gerrit-Change-Number: 11516
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Wed, 26 Sep 2018 18:32:26 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-686 (part 2/2): use DeltaPreparer in DeltaFileIterator

2018-09-26 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11395 )

Change subject: KUDU-686 (part 2/2): use DeltaPreparer in DeltaFileIterator
..


Patch Set 7: Verified+1

Overriding Jenkins, unrelated flaky test. I published 
http://gerrit.cloudera.org:8080/11523 for it.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I87de52092262c4b42c1bd5107f9139edfc3888b5
Gerrit-Change-Number: 11395
Gerrit-PatchSet: 7
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 26 Sep 2018 18:31:37 +
Gerrit-HasComments: No


[kudu-CR] KUDU-686 (part 2/2): use DeltaPreparer in DeltaFileIterator

2018-09-26 Thread Adar Dembo (Code Review)
Adar Dembo has removed Kudu Jenkins from this change.  ( 
http://gerrit.cloudera.org:8080/11395 )

Change subject: KUDU-686 (part 2/2): use DeltaPreparer in DeltaFileIterator
..


Removed reviewer Kudu Jenkins with the following votes:

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: I87de52092262c4b42c1bd5107f9139edfc3888b5
Gerrit-Change-Number: 11395
Gerrit-PatchSet: 7
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] create-table-stress-test: deflake TestConcurrentCreateTableAndReloadMetadata

2018-09-26 Thread Adar Dembo (Code Review)
Hello Alexey Serbin,

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

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

to review the following change.


Change subject: create-table-stress-test: deflake 
TestConcurrentCreateTableAndReloadMetadata
..

create-table-stress-test: deflake TestConcurrentCreateTableAndReloadMetadata

First, if the ASSERT_OK fired, the test would hang. No bueno.

Second, in TSAN environments the test would sometimes fail with a table
already present error, typically happening either 30s or 60s after that
table was created. I believe this is due to a client-side timeout _after_
the table was created but _before_ its RPC response was sent. We could
try to address this by bumping the client's timeout, but because
reload_metadata_thread is DoS'ing the cluster, that doesn't seem robust.
Instead, let's have the thread ease up in TSAN environments.

Without the change, the test failed 4/1000 times (--stress_cpu_threads=4).

With the change, the test failed 0/1000 times (--stress_cpu_threads=8).

Change-Id: I607a8e278e8e8f370cda512f365a97fbb81e1ae3
---
M src/kudu/integration-tests/create-table-stress-test.cc
1 file changed, 13 insertions(+), 4 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I607a8e278e8e8f370cda512f365a97fbb81e1ae3
Gerrit-Change-Number: 11523
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 


[kudu-CR](gh-pages) gh-pages: Make site-tool Python 3 compatible

2018-09-26 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11522 )

Change subject: gh-pages: Make site-tool Python 3 compatible
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-MessageType: comment
Gerrit-Change-Id: I5e1f69c80e5f327b0d0d017bacf717a97f572e5b
Gerrit-Change-Number: 11522
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Comment-Date: Wed, 26 Sep 2018 18:22:05 +
Gerrit-HasComments: No


[kudu-CR] tserver: log a message when a scanner is not found

2018-09-26 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11516 )

Change subject: tserver: log a message when a scanner is not found
..


Patch Set 1:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/11516/1/src/kudu/tserver/tablet_service.cc@2015
PS1, Line 2015:   Status verbose = s.CloneAndAppend(Substitute("call 
sequence id=$0, remote=$1",
  :
req->call_seq_id(),
  :
rpc_context->requestor_string()));
Why not to put this information into the returned status as is?


http://gerrit.cloudera.org:8080/#/c/11516/1/src/kudu/tserver/tablet_service.cc@2038
PS1, Line 2038:   if (req->call_seq_id() != scanner->call_seq_id()) {
  : *error_code = TabletServerErrorPB::INVALID_SCAN_CALL_SEQ_ID;
  : return Status::InvalidArgument("Invalid call sequence ID in 
scan request");
  :   }
Some unrelated to this particular change, but maybe relevant stuff when chasing 
the not-found-scanner issue.

While you are here, do you think it's worth making some other changes (maybe, 
in a separate changelist):

1) Moving this verification before instantiating ScopedUnregisterScanner.  The 
reason is that current code inadvertently (I think) closes the scanner if a 
client sends in wrong scan call sequence id with its request for a valid 
scanner identifier.

2) Add a trace or VLOG() message for the case of the scenario in L2026 of the 
original code, since it makes the scanner to be automatically closed after the 
return.

3) Fill in the 'has_more_results output' parameter in case of situation as in 
L2007 in the original code.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If4e9c69160605d4a839ac2d28def67f4d3402b90
Gerrit-Change-Number: 11516
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Wed, 26 Sep 2018 17:58:21 +
Gerrit-HasComments: Yes


[kudu-CR](gh-pages) Blogpost describing index skip scan optimization.

2018-09-26 Thread Mike Percy (Code Review)
Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11263 )

Change subject: Blogpost describing index skip scan optimization.
..


Patch Set 12:

This post is now live: 
http://kudu.apache.org/2018/09/26/index-skip-scan-optimization-in-kudu.html


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

Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-MessageType: comment
Gerrit-Change-Id: I2250652dcba3d1b0a06f1ffb7f23c11bf533d35e
Gerrit-Change-Number: 11263
Gerrit-PatchSet: 12
Gerrit-Owner: Anupama Gupta 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Anupama Gupta 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Wed, 26 Sep 2018 17:57:21 +
Gerrit-HasComments: No


[kudu-CR](gh-pages) gh-pages: Make site-tool Python 3 compatible

2018-09-26 Thread Mike Percy (Code Review)
Mike Percy has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/11522


Change subject: gh-pages: Make site-tool Python 3 compatible
..

gh-pages: Make site-tool Python 3 compatible

Change-Id: I5e1f69c80e5f327b0d0d017bacf717a97f572e5b
---
M _tools/kudu_util.py
M site_tool
2 files changed, 26 insertions(+), 22 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5e1f69c80e5f327b0d0d017bacf717a97f572e5b
Gerrit-Change-Number: 11522
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy 


[kudu-CR](gh-pages) Blogpost describing index skip scan optimization.

2018-09-26 Thread Mike Percy (Code Review)
Mike Percy has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/11263 )

Change subject: Blogpost describing index skip scan optimization.
..

Blogpost describing index skip scan optimization.

Link to the version with images:
https://github.com/AnupamaGupta01/kudu/blob/blogpost-2/_posts/2018-09-25-index-skip-scan-optimization-in-kudu.md

Change-Id: I2250652dcba3d1b0a06f1ffb7f23c11bf533d35e
Reviewed-on: http://gerrit.cloudera.org:8080/11263
Reviewed-by: Mike Percy 
Tested-by: Mike Percy 
---
A _posts/2018-09-26-index-skip-scan-optimization-in-kudu.md
A img/index-skip-scan/example-table.png
A img/index-skip-scan/skip-scan-example-table.png
A img/index-skip-scan/skip-scan-performance-graph.png
4 files changed, 114 insertions(+), 0 deletions(-)

Approvals:
  Mike Percy: Looks good to me, approved; Verified

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

Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-MessageType: merged
Gerrit-Change-Id: I2250652dcba3d1b0a06f1ffb7f23c11bf533d35e
Gerrit-Change-Number: 11263
Gerrit-PatchSet: 12
Gerrit-Owner: Anupama Gupta 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Anupama Gupta 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Mike Percy 


[kudu-CR](gh-pages) Blogpost describing index skip scan optimization.

2018-09-26 Thread Mike Percy (Code Review)
Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11263 )

Change subject: Blogpost describing index skip scan optimization.
..


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

Rendered locally with site_tool jekyll serve and it looks good. I'm about to 
push this live.


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

Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-MessageType: comment
Gerrit-Change-Id: I2250652dcba3d1b0a06f1ffb7f23c11bf533d35e
Gerrit-Change-Number: 11263
Gerrit-PatchSet: 11
Gerrit-Owner: Anupama Gupta 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Anupama Gupta 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Wed, 26 Sep 2018 17:50:21 +
Gerrit-HasComments: No


[kudu-CR](gh-pages) Blogpost describing index skip scan optimization.

2018-09-26 Thread Mike Percy (Code Review)
Mike Percy has uploaded a new patch set (#11) to the change originally created 
by Anupama Gupta. ( http://gerrit.cloudera.org:8080/11263 )

Change subject: Blogpost describing index skip scan optimization.
..

Blogpost describing index skip scan optimization.

Link to the version with images:
https://github.com/AnupamaGupta01/kudu/blob/blogpost-2/_posts/2018-09-25-index-skip-scan-optimization-in-kudu.md

Change-Id: I2250652dcba3d1b0a06f1ffb7f23c11bf533d35e
---
A _posts/2018-09-26-index-skip-scan-optimization-in-kudu.md
A img/index-skip-scan/example-table.png
A img/index-skip-scan/skip-scan-example-table.png
A img/index-skip-scan/skip-scan-performance-graph.png
4 files changed, 114 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/63/11263/11
--
To view, visit http://gerrit.cloudera.org:8080/11263
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2250652dcba3d1b0a06f1ffb7f23c11bf533d35e
Gerrit-Change-Number: 11263
Gerrit-PatchSet: 11
Gerrit-Owner: Anupama Gupta 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Anupama Gupta 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Mike Percy 


[kudu-CR](gh-pages) Blogpost describing index skip scan optimization.

2018-09-26 Thread Mike Percy (Code Review)
Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11263 )

Change subject: Blogpost describing index skip scan optimization.
..


Patch Set 10:

I noticed yesterday afternoon that I got distracted and didn't post this. I'm 
going to wrangle up a new filename and push it out right now.


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

Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-MessageType: comment
Gerrit-Change-Id: I2250652dcba3d1b0a06f1ffb7f23c11bf533d35e
Gerrit-Change-Number: 11263
Gerrit-PatchSet: 10
Gerrit-Owner: Anupama Gupta 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Anupama Gupta 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Wed, 26 Sep 2018 17:25:57 +
Gerrit-HasComments: No


[kudu-CR] tserver: log a message when a scanner is not found

2018-09-26 Thread Grant Henke (Code Review)
Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11516 )

Change subject: tserver: log a message when a scanner is not found
..


Patch Set 1:

Looks like client-test.cc needs to be updated too.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If4e9c69160605d4a839ac2d28def67f4d3402b90
Gerrit-Change-Number: 11516
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Wed, 26 Sep 2018 17:11:10 +
Gerrit-HasComments: No


[kudu-CR] deltas: fuzz tests for delta file and dms

2018-09-26 Thread Adar Dembo (Code Review)
Adar Dembo has removed Kudu Jenkins from this change.  ( 
http://gerrit.cloudera.org:8080/11140 )

Change subject: deltas: fuzz tests for delta file and dms
..


Removed reviewer Kudu Jenkins with the following votes:

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: I539ff0f2055cf398a42efb824238188230e25516
Gerrit-Change-Number: 11140
Gerrit-PatchSet: 11
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] deltas: fuzz tests for delta file and dms

2018-09-26 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11140 )

Change subject: deltas: fuzz tests for delta file and dms
..


Patch Set 11: Verified+1

Overriding Jenkins, another instance of KUDU-2212.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I539ff0f2055cf398a42efb824238188230e25516
Gerrit-Change-Number: 11140
Gerrit-PatchSet: 11
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 26 Sep 2018 16:05:53 +
Gerrit-HasComments: No


[kudu-CR] Implement BloomFilter Predicate in server side.

2018-09-26 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11100 )

Change subject: Implement BloomFilter Predicate in server side.
..


Patch Set 9: Code-Review+1

Looks good, though I'd like Dan to take another look as he's our resident 
predicate expert.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I62c2de42667d0255d94e19db773240f7f9ee636c
Gerrit-Change-Number: 11100
Gerrit-PatchSet: 9
Gerrit-Owner: ZhangYao 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: ZhangYao 
Gerrit-Comment-Date: Wed, 26 Sep 2018 15:51:42 +
Gerrit-HasComments: No


[kudu-CR] Supporting Spark streaming DataFrame in KuduContext.

2018-09-26 Thread Grant Henke (Code Review)
Grant Henke has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/11199 )

Change subject: Supporting Spark streaming DataFrame in KuduContext.
..

Supporting Spark streaming DataFrame in KuduContext.

KUDU-2539: Supporting Spark streaming DataFrame in KuduContext.

This solution follows the way how other sinks ie. KafkaSink
is implemented, for details see
https://github.com/apache/spark/blob/master/external/kafka-0-10-sql/src/main/scala/org/apache/spark/sql/kafka010/KafkaWriter.scala#L87

Where on the DataFrame a queryExecution.toRdd.foreachPartition
is called to access the InternalRows which mapped to Rows by Catalyst
converters.

Change-Id: Iead04539d3514920a5d6803c34715e5686124572
Reviewed-on: http://gerrit.cloudera.org:8080/11199
Tested-by: Kudu Jenkins
Reviewed-by: Attila Bukor 
Reviewed-by: Grant Henke 
---
M java/gradle/dependencies.gradle
M java/kudu-spark/build.gradle
M java/kudu-spark/pom.xml
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
A java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/StreamingTest.scala
5 files changed, 126 insertions(+), 6 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Attila Bukor: Looks good to me, but someone else must approve
  Grant Henke: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Iead04539d3514920a5d6803c34715e5686124572
Gerrit-Change-Number: 11199
Gerrit-PatchSet: 6
Gerrit-Owner: Attila Piros 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Attila Piros 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR] Supporting Spark streaming DataFrame in KuduContext.

2018-09-26 Thread Grant Henke (Code Review)
Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11199 )

Change subject: Supporting Spark streaming DataFrame in KuduContext.
..


Patch Set 5: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iead04539d3514920a5d6803c34715e5686124572
Gerrit-Change-Number: 11199
Gerrit-PatchSet: 5
Gerrit-Owner: Attila Piros 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Attila Piros 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 26 Sep 2018 15:45:18 +
Gerrit-HasComments: No


[kudu-CR] KUDU-2012 Kudu Flume sink auth support

2018-09-26 Thread Ferenc Szabo (Code Review)
Hello Mike Percy, Attila Bukor, Kudu Jenkins, Grant Henke,

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

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

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

Change subject: KUDU-2012 Kudu Flume sink auth support
..

KUDU-2012 Kudu Flume sink auth support

Adding FlumeAuthenticator to KuduSink and creating KuduClient
inside a PrivilegedExecutor action.

Added an extra step to the mini cluster to create
a keyTab for the client used for testing.

Added automated test with short KDC ticket lifetime
to test reacquiring.

Manual testing was done on a secure cluster as well.

Change-Id: I11b5f08802883afa178d346af48d3bcd15281917
---
M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java
M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java
M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSink.java
M 
java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSinkConfigurationConstants.java
M 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducerTest.java
M 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/KeyedKuduOperationsProducerTest.java
M 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/KuduSinkTest.java
A 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/KuduSinkTestUtil.java
M 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducerTest.java
A 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/SecureKuduSinkTest.java
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/security/test/mini_kdc.cc
M src/kudu/security/test/mini_kdc.h
13 files changed, 372 insertions(+), 254 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I11b5f08802883afa178d346af48d3bcd15281917
Gerrit-Change-Number: 11334
Gerrit-PatchSet: 7
Gerrit-Owner: Ferenc Szabo 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Ferenc Szabo 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 


[kudu-CR] KUDU-2012 Kudu Flume sink auth support

2018-09-26 Thread Ferenc Szabo (Code Review)
Hello Mike Percy, Attila Bukor, Kudu Jenkins, Grant Henke,

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

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

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

Change subject: KUDU-2012 Kudu Flume sink auth support
..

KUDU-2012 Kudu Flume sink auth support

Adding FlumeAuthenticator to KuduSink and creating KuduClient
inside a PrivilegedExecutor action.

Added an extra step to the mini cluster to create
a keyTab for the client used for testing.

Added automated test with short KDC ticket lifetime
to test reacquiring.

Manual testing was done on a secure cluster as well.

Change-Id: I11b5f08802883afa178d346af48d3bcd15281917
---
M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java
M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSink.java
M 
java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSinkConfigurationConstants.java
M 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducerTest.java
M 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/KeyedKuduOperationsProducerTest.java
M 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/KuduSinkTest.java
A 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/KuduSinkTestUtil.java
M 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducerTest.java
A 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/SecureKuduSinkTest.java
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/security/test/mini_kdc.cc
M src/kudu/security/test/mini_kdc.h
12 files changed, 365 insertions(+), 254 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I11b5f08802883afa178d346af48d3bcd15281917
Gerrit-Change-Number: 11334
Gerrit-PatchSet: 6
Gerrit-Owner: Ferenc Szabo 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Ferenc Szabo 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 


[kudu-CR] KUDU-2012 Kudu Flume sink auth support

2018-09-26 Thread Ferenc Szabo (Code Review)
Hello Mike Percy, Attila Bukor, Kudu Jenkins, Grant Henke,

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

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

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

Change subject: KUDU-2012 Kudu Flume sink auth support
..

KUDU-2012 Kudu Flume sink auth support

Adding FlumeAuthenticator to KuduSink and creating KuduClient
inside a PrivilegedExecutor action.

Added an extra step to the mini cluster to create
a keyTab for the client used for testing.

Added automated test with short KDC ticket lifetime
to test reacquiring.

Manual testing was done on a secure cluster as well.

Change-Id: I11b5f08802883afa178d346af48d3bcd15281917
---
M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java
M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSink.java
M 
java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSinkConfigurationConstants.java
M 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducerTest.java
M 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/KeyedKuduOperationsProducerTest.java
M 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/KuduSinkTest.java
A 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/KuduSinkTestUtil.java
M 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducerTest.java
A 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/SecureKuduSinkTest.java
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/security/test/mini_kdc.cc
M src/kudu/security/test/mini_kdc.h
12 files changed, 365 insertions(+), 254 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I11b5f08802883afa178d346af48d3bcd15281917
Gerrit-Change-Number: 11334
Gerrit-PatchSet: 5
Gerrit-Owner: Ferenc Szabo 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Ferenc Szabo 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 


[kudu-CR] KUDU-2012 Kudu Flume sink auth support

2018-09-26 Thread Ferenc Szabo (Code Review)
Hello Mike Percy, Attila Bukor, Kudu Jenkins, Grant Henke,

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

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

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

Change subject: KUDU-2012 Kudu Flume sink auth support
..

KUDU-2012 Kudu Flume sink auth support

Adding FlumeAuthenticator to KuduSink and creating KuduClient
inside a PrivilegedExecutor action.

Added an extra step to the mini cluster to create
a keyTab for the client used for testing.

Added automated test with short KDC ticket lifetime
to test reacquiring.

Manual testing was done on a secure cluster as well.

Change-Id: I11b5f08802883afa178d346af48d3bcd15281917
---
M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java
M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSink.java
M 
java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSinkConfigurationConstants.java
M 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducerTest.java
M 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/KeyedKuduOperationsProducerTest.java
M 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/KuduSinkTest.java
A 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/KuduSinkTestUtil.java
M 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducerTest.java
A 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/SecureKuduSinkTest.java
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/security/test/mini_kdc.cc
M src/kudu/security/test/mini_kdc.h
12 files changed, 365 insertions(+), 254 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I11b5f08802883afa178d346af48d3bcd15281917
Gerrit-Change-Number: 11334
Gerrit-PatchSet: 4
Gerrit-Owner: Ferenc Szabo 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Ferenc Szabo 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 


[kudu-CR] KUDU-2012 Kudu Flume sink auth support

2018-09-26 Thread Ferenc Szabo (Code Review)
Ferenc Szabo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11334 )

Change subject: KUDU-2012 Kudu Flume sink auth support
..


Patch Set 3:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/11334/3/java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSink.java
File 
java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSink.java:

http://gerrit.cloudera.org:8080/#/c/11334/3/java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSink.java@217
PS3, Line 217: 
> nit: indent at least 4 spaces on a line continuation
Checkstyle did not mention this indentation error.
You might need to fine-tune the checkstyle config.


http://gerrit.cloudera.org:8080/#/c/11334/3/java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/KuduSinkTestUtil.java
File 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/KuduSinkTestUtil.java:

http://gerrit.cloudera.org:8080/#/c/11334/3/java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/KuduSinkTestUtil.java@54
PS3, Line 54: context.put(KERBEROS_KEYTAB, clusterRoot + 
"/krb5kdc/test-user.keytab");
: context.put(KERBEROS_PRINCIPAL, "test-u...@krbtest.com");
> Are there some kinds of prerequisites we should document in the javadoc for
This user is created by the mini cluster. I have only added the creation of the 
keytab file.


http://gerrit.cloudera.org:8080/#/c/11334/3/java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/SecureKuduSinkTest.java
File 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/SecureKuduSinkTest.java:

http://gerrit.cloudera.org:8080/#/c/11334/3/java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/SecureKuduSinkTest.java@57
PS3, Line 57: System.clearProperty(KUDU_TICKETCACHE_PROPERTY);
> Could this affect other Kudu client tests? Should we be restoring this when
The current config is to fork jvm for every test class, so this does not affect 
the other test. The mini cluster sets it in the begining of the tests as well. 
It would be restored by the minicluster even without jvm isolation in the 
@Before of the BaseKuduTest



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I11b5f08802883afa178d346af48d3bcd15281917
Gerrit-Change-Number: 11334
Gerrit-PatchSet: 3
Gerrit-Owner: Ferenc Szabo 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Ferenc Szabo 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Wed, 26 Sep 2018 10:14:37 +
Gerrit-HasComments: Yes


[kudu-CR] Implement BloomFilter Predicate in server side.

2018-09-26 Thread ZhangYao (Code Review)
ZhangYao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11100 )

Change subject: Implement BloomFilter Predicate in server side.
..


Patch Set 9:

(5 comments)

Thanks a lot :)

http://gerrit.cloudera.org:8080/#/c/11100/8/src/kudu/common/column_predicate-test.cc
File src/kudu/common/column_predicate-test.cc:

http://gerrit.cloudera.org:8080/#/c/11100/8/src/kudu/common/column_predicate-test.cc@77
PS8, Line 77: BloomFilterBuilder* bfb2) {
> You shouldn't call this more than once per test. So maybe add it to the fix
Get your point, I add rand_ as a member of Test and initialize it in 
constructor so each test case will have different rand seed and in the same 
test case will use the same seed.


http://gerrit.cloudera.org:8080/#/c/11100/8/src/kudu/common/column_predicate-test.cc@1399
PS8, Line 1399:   ColumnPredicate::BloomFilterInner bf2(slice2, 
bfb2.n_hashes(), MURMUR_HASH_2);
> Still got some usage of push_back in this test instead of emplace_back.
Done


http://gerrit.cloudera.org:8080/#/c/11100/6/src/kudu/common/column_predicate.cc
File src/kudu/common/column_predicate.cc:

http://gerrit.cloudera.org:8080/#/c/11100/6/src/kudu/common/column_predicate.cc@275
PS6, Line 275:   } else {
> Yes, please do.
Done


http://gerrit.cloudera.org:8080/#/c/11100/6/src/kudu/common/wire_protocol-test.cc
File src/kudu/common/wire_protocol-test.cc:

http://gerrit.cloudera.org:8080/#/c/11100/6/src/kudu/common/wire_protocol-test.cc@555
PS6, Line 555: ASSERT_EQ(predicate->predicate_type(), 
PredicateType::InBloomFilter);
> Yeah, decomposing the one for InList would be great too.
I decompose the bloomfilter test and use unique_ptr to wrap the 
BloomFilterBuilder because it doesn't have a default constructor so I have to 
use it's point format as member variable.


http://gerrit.cloudera.org:8080/#/c/11100/8/src/kudu/util/bloom_filter.h
File src/kudu/util/bloom_filter.h:

http://gerrit.cloudera.org:8080/#/c/11100/8/src/kudu/util/bloom_filter.h@67
PS8, Line 67:   case CITY_HASH:
> Nit: don't need the comment here; it's obvious from the default value.
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I62c2de42667d0255d94e19db773240f7f9ee636c
Gerrit-Change-Number: 11100
Gerrit-PatchSet: 9
Gerrit-Owner: ZhangYao 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: ZhangYao 
Gerrit-Comment-Date: Wed, 26 Sep 2018 07:18:58 +
Gerrit-HasComments: Yes


[kudu-CR] Implement BloomFilter Predicate in server side.

2018-09-26 Thread ZhangYao (Code Review)
Hello Tidy Bot, Dan Burkert, Kudu Jenkins, Andrew Wong, Adar Dembo, Todd Lipcon,

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

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

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

Change subject: Implement BloomFilter Predicate in server side.
..

Implement BloomFilter Predicate in server side.

Change-Id: I62c2de42667d0255d94e19db773240f7f9ee636c
---
M src/kudu/common/column_predicate-test.cc
M src/kudu/common/column_predicate.cc
M src/kudu/common/column_predicate.h
M src/kudu/common/common.proto
M src/kudu/common/key_util.cc
M src/kudu/common/scan_spec.cc
M src/kudu/common/wire_protocol-test.cc
M src/kudu/common/wire_protocol.cc
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/util/bloom_filter.h
10 files changed, 1,112 insertions(+), 27 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I62c2de42667d0255d94e19db773240f7f9ee636c
Gerrit-Change-Number: 11100
Gerrit-PatchSet: 9
Gerrit-Owner: ZhangYao 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: ZhangYao