subject:"\[kudu\-CR\] KUDU\-2191 \(2\/n\)\: Hive Metastore client"

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-11-02 Thread Todd Lipcon (Code Review)

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..

KUDU-2191 (2/n): Hive Metastore client

This patch lays the groundwork for integrating the Kudu catalog with the
Hive MetaStore.

The focus of this patch is a Kudu-specific C++ HMS client
(hms_client.[h|cc]) in a new hms module. This client provides bindings
for the Hive Metastore APIs that Kudu will use in follow-up commits.

- Thrift has been added as a dependency, and a mechanism for performing
  Thrift codegen at compile time has been added (see FindThrift.cmake,
  based on FindProtobuf.cmake)

- Bison has been added as a build-time dependency, because the system
  bison version on RHEL 6 is not sufficiently new enough for Thrift 0.10.

- Hive and Hadoop have been added to thirdparty as test-only dependencies.

- A Hive MetaStore external mini server is included for testing. See
  mini_hms.[h|cc].

- The Kudu Metastore plugin is compiled from CMake as a standalone jar
  for loading into the HMS mini server.

Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Reviewed-on: http://gerrit.cloudera.org:8080/7053
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M CMakeLists.txt
M build-support/dist_test.py
M build-support/iwyu/iwyu-filter.awk
M build-support/run_dist_test.py
A cmake_modules/FindJavaHome.cmake
A cmake_modules/FindThrift.cmake
A src/kudu/hms/CMakeLists.txt
A src/kudu/hms/hive_metastore.thrift
A src/kudu/hms/hms_client-test.cc
A src/kudu/hms/hms_client.cc
A src/kudu/hms/hms_client.h
A src/kudu/hms/mini_hms.cc
A src/kudu/hms/mini_hms.h
M thirdparty/LICENSE.txt
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
M thirdparty/download-thirdparty.sh
M thirdparty/vars.sh
18 files changed, 2,928 insertions(+), 7 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 38
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-11-02 Thread Todd Lipcon (Code Review)

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


Patch Set 37: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 37
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 03 Nov 2017 00:07:51 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-11-01 Thread Dan Burkert (Code Review)

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


Patch Set 30:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/hms_client.h
File src/kudu/hms/hms_client.h:

http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/hms_client.h@85
PS30, Line 85: bool cascade = false,
 :   bool delete_data = true)
> I'm generally against bool parameters for readability. I liked this C++ tip
Done


http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/hms_client.h@121
PS30, Line 121:int32_t max_events,
> I guess there is no way to limit the request by a certain size? eg we might
No, right now the interface only allows you to set the maximum number of 
events.  I'd expect we can set this pretty low, at 20 or 100 events and be able 
to keep pace with the HMS.  I'll look into what Sentry HA sets their limit to.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 30
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 01 Nov 2017 15:15:43 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-11-01 Thread Dan Burkert (Code Review)

Hello Tidy Bot, Alexey Serbin, Jean-Daniel Cryans, Kudu Jenkins, Adar Dembo, 
Todd Lipcon,

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

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

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..

KUDU-2191 (2/n): Hive Metastore client

This patch lays the groundwork for integrating the Kudu catalog with the
Hive MetaStore.

The focus of this patch is a Kudu-specific C++ HMS client
(hms_client.[h|cc]) in a new hms module. This client provides bindings
for the Hive Metastore APIs that Kudu will use in follow-up commits.

- Thrift has been added as a dependency, and a mechanism for performing
  Thrift codegen at compile time has been added (see FindThrift.cmake,
  based on FindProtobuf.cmake)

- Bison has been added as a build-time dependency, because the system
  bison version on RHEL 6 is not sufficiently new enough for Thrift 0.10.

- Hive and Hadoop have been added to thirdparty as test-only dependencies.

- A Hive MetaStore external mini server is included for testing. See
  mini_hms.[h|cc].

- The Kudu Metastore plugin is compiled from CMake as a standalone jar
  for loading into the HMS mini server.

Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
---
M CMakeLists.txt
M build-support/dist_test.py
M build-support/iwyu/iwyu-filter.awk
M build-support/run_dist_test.py
A cmake_modules/FindJavaHome.cmake
A cmake_modules/FindThrift.cmake
A src/kudu/hms/CMakeLists.txt
A src/kudu/hms/hive_metastore.thrift
A src/kudu/hms/hms_client-test.cc
A src/kudu/hms/hms_client.cc
A src/kudu/hms/hms_client.h
A src/kudu/hms/mini_hms.cc
A src/kudu/hms/mini_hms.h
M thirdparty/LICENSE.txt
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
M thirdparty/download-thirdparty.sh
M thirdparty/vars.sh
18 files changed, 2,928 insertions(+), 7 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 37
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-31 Thread Todd Lipcon (Code Review)

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


Patch Set 36:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/hms_client.h
File src/kudu/hms/hms_client.h:

http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/hms_client.h@85
PS30, Line 85: in the HMS.
 :   //
> Not in my opinion.  What advantage would that have?  This method will only
I'm generally against bool parameters for readability. I liked this C++ tip: 
https://abseil.io/tips/94


http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/hms_client.h@121
PS30, Line 121:   // Retrieves all tables in an HMS database.
> They can be very large for HDFS tables with a lot of partitions.  There's c
I guess there is no way to limit the request by a certain size? eg we might 
want to get 1000 events normally, but if we cross some threshold like 10MB then 
stop at that point?

I'm just not sure how we'll appropriately decide on a value for 'max_events' 
that is both high-throughput and memory-safe. 1000 events is probably fine most 
of the time, but if we get 1000x 10MB events in some catch-up scenario then all 
of a sudden we have memory issues to worry about.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 36
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 31 Oct 2017 23:36:07 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-30 Thread Dan Burkert (Code Review)

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


Patch Set 29:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7053/29/src/kudu/hms/mini_hms.h
File src/kudu/hms/mini_hms.h:

http://gerrit.cloudera.org:8080/#/c/7053/29/src/kudu/hms/mini_hms.h@30
PS29, Line 30: #include "kudu/util/subprocess.h"
> You sure about that? I'm looking at https://stackoverflow.com/questions/601
Sorry for not responding before.  This appears to me to be toolchain dependent 
(probably libstdcxx vs libcxx), because this won't compile on macos (libcxx) 
with a forwards declare.  I'm not crazy about the idea of forgoing the 
simplicity of '= default;' just to satisfy IWYU, so I'd like to keep it as is.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 29
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 30 Oct 2017 23:06:44 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-30 Thread Dan Burkert (Code Review)

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


Patch Set 36:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7053/36/src/kudu/hms/mini_hms.h
File src/kudu/hms/mini_hms.h:

http://gerrit.cloudera.org:8080/#/c/7053/36/src/kudu/hms/mini_hms.h@30
PS36, Line 30: #include "kudu/util/subprocess.h" // IWYU pragma: keep
> Any particular reason you don't want to move MiniHms constructor into mini_
I don't like the idea of making the code more complex in order to satisfy IWYU.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 36
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 30 Oct 2017 22:41:15 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-30 Thread Adar Dembo (Code Review)

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


Patch Set 36:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7053/36/src/kudu/hms/mini_hms.h
File src/kudu/hms/mini_hms.h:

http://gerrit.cloudera.org:8080/#/c/7053/36/src/kudu/hms/mini_hms.h@30
PS36, Line 30: #include "kudu/util/subprocess.h" // IWYU pragma: keep
Any particular reason you don't want to move MiniHms constructor into 
mini_hms.cc, thus allowing you to cleanly forward declare Subprocess and avoid 
this inclusion? Alexey and I both suggested that back in PS29/PS30.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 36
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 30 Oct 2017 22:12:48 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-30 Thread Dan Burkert (Code Review)

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


Patch Set 36:

OK I think this is ready to review again.  Sorry for the churn.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 36
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 30 Oct 2017 18:40:18 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-30 Thread Dan Burkert (Code Review)

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


Patch Set 30:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7053/30/build-support/run_dist_test.py
File build-support/run_dist_test.py:

http://gerrit.cloudera.org:8080/#/c/7053/30/build-support/run_dist_test.py@128
PS30, Line 128:   env['HIVE_HOME'] = glob.glob(os.path.join(ROOT, 
"thirdparty/src/apache-hive-*-bin"))[0]
  :   env['HADOOP_HOME'] = glob.glob(os.path.join(ROOT, 
"thirdparty/src/hadoop-*"))[0]
> Done
Woops, turns out this was correct the first time.  dist_test.py ensures that 
only the correct versions of hive and hadoop are copied, because they use the 
build/latest/bin/[hadoop-home|hive-home] symlinks.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 30
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 30 Oct 2017 16:15:36 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-30 Thread Dan Burkert (Code Review)

Hello Tidy Bot, Alexey Serbin, Jean-Daniel Cryans, Kudu Jenkins, Adar Dembo, 
Todd Lipcon,

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

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

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..

KUDU-2191 (2/n): Hive Metastore client

This patch lays the groundwork for integrating the Kudu catalog with the
Hive MetaStore.

The focus of this patch is a Kudu-specific C++ HMS client
(hms_client.[h|cc]) in a new hms module. This client provides bindings
for the Hive Metastore APIs that Kudu will use in follow-up commits.

- Thrift has been added as a dependency, and a mechanism for performing
  Thrift codegen at compile time has been added (see FindThrift.cmake,
  based on FindProtobuf.cmake)

- Bison has been added as a build-time dependency, because the system
  bison version on RHEL 6 is not sufficiently new enough for Thrift 0.10.

- Hive and Hadoop have been added to thirdparty as test-only dependencies.

- A Hive MetaStore external mini server is included for testing. See
  mini_hms.[h|cc].

- The Kudu Metastore plugin is compiled from CMake as a standalone jar
  for loading into the HMS mini server.

Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
---
M CMakeLists.txt
M build-support/dist_test.py
M build-support/iwyu/iwyu-filter.awk
M build-support/run_dist_test.py
A cmake_modules/FindJavaHome.cmake
A cmake_modules/FindThrift.cmake
A src/kudu/hms/CMakeLists.txt
A src/kudu/hms/hive_metastore.thrift
A src/kudu/hms/hms_client-test.cc
A src/kudu/hms/hms_client.cc
A src/kudu/hms/hms_client.h
A src/kudu/hms/mini_hms.cc
A src/kudu/hms/mini_hms.h
M thirdparty/LICENSE.txt
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
M thirdparty/download-thirdparty.sh
M thirdparty/vars.sh
18 files changed, 2,928 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/7053/36
--
To view, visit http://gerrit.cloudera.org:8080/7053
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 36
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-30 Thread Dan Burkert (Code Review)

Hello Tidy Bot, Alexey Serbin, Jean-Daniel Cryans, Kudu Jenkins, Adar Dembo, 
Todd Lipcon,

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

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

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..

KUDU-2191 (2/n): Hive Metastore client

This patch lays the groundwork for integrating the Kudu catalog with the
Hive MetaStore.

The focus of this patch is a Kudu-specific C++ HMS client
(hms_client.[h|cc]) in a new hms module. This client provides bindings
for the Hive Metastore APIs that Kudu will use in follow-up commits.

- Thrift has been added as a dependency, and a mechanism for performing
  Thrift codegen at compile time has been added (see FindThrift.cmake,
  based on FindProtobuf.cmake)

- Bison has been added as a build-time dependency, because the system
  bison version on RHEL 6 is not sufficiently new enough for Thrift 0.10.

- Hive and Hadoop have been added to thirdparty as test-only dependencies.

- A Hive MetaStore external mini server is included for testing. See
  mini_hms.[h|cc].

- The Kudu Metastore plugin is compiled from CMake as a standalone jar
  for loading into the HMS mini server.

Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
---
M CMakeLists.txt
M build-support/dist_test.py
M build-support/iwyu/iwyu-filter.awk
M build-support/run_dist_test.py
A cmake_modules/FindJavaHome.cmake
A cmake_modules/FindThrift.cmake
A src/kudu/hms/CMakeLists.txt
A src/kudu/hms/hive_metastore.thrift
A src/kudu/hms/hms_client-test.cc
A src/kudu/hms/hms_client.cc
A src/kudu/hms/hms_client.h
A src/kudu/hms/mini_hms.cc
A src/kudu/hms/mini_hms.h
M thirdparty/LICENSE.txt
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
M thirdparty/download-thirdparty.sh
M thirdparty/vars.sh
18 files changed, 2,928 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/7053/35
--
To view, visit http://gerrit.cloudera.org:8080/7053
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 35
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-30 Thread Dan Burkert (Code Review)

Hello Tidy Bot, Alexey Serbin, Jean-Daniel Cryans, Kudu Jenkins, Adar Dembo, 
Todd Lipcon,

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

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

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..

KUDU-2191 (2/n): Hive Metastore client

This patch lays the groundwork for integrating the Kudu catalog with the
Hive MetaStore.

The focus of this patch is a Kudu-specific C++ HMS client
(hms_client.[h|cc]) in a new hms module. This client provides bindings
for the Hive Metastore APIs that Kudu will use in follow-up commits.

- Thrift has been added as a dependency, and a mechanism for performing
  Thrift codegen at compile time has been added (see FindThrift.cmake,
  based on FindProtobuf.cmake)

- Bison has been added as a build-time dependency, because the system
  bison version on RHEL 6 is not sufficiently new enough for Thrift 0.10.

- Hive and Hadoop have been added to thirdparty as test-only dependencies.

- A Hive MetaStore external mini server is included for testing. See
  mini_hms.[h|cc].

- The Kudu Metastore plugin is compiled from CMake as a standalone jar
  for loading into the HMS mini server.

Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
---
M CMakeLists.txt
M build-support/dist_test.py
M build-support/iwyu/iwyu-filter.awk
M build-support/run_dist_test.py
A cmake_modules/FindJavaHome.cmake
A cmake_modules/FindThrift.cmake
A src/kudu/hms/CMakeLists.txt
A src/kudu/hms/hive_metastore.thrift
A src/kudu/hms/hms_client-test.cc
A src/kudu/hms/hms_client.cc
A src/kudu/hms/hms_client.h
A src/kudu/hms/mini_hms.cc
A src/kudu/hms/mini_hms.h
M thirdparty/LICENSE.txt
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
M thirdparty/download-thirdparty.sh
M thirdparty/vars.sh
18 files changed, 2,928 insertions(+), 7 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 34
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-30 Thread Dan Burkert (Code Review)

Hello Tidy Bot, Alexey Serbin, Jean-Daniel Cryans, Kudu Jenkins, Adar Dembo, 
Todd Lipcon,

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

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

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..

KUDU-2191 (2/n): Hive Metastore client

This patch lays the groundwork for integrating the Kudu catalog with the
Hive MetaStore.

The focus of this patch is a Kudu-specific C++ HMS client
(hms_client.[h|cc]) in a new hms module. This client provides bindings
for the Hive Metastore APIs that Kudu will use in follow-up commits.

- Thrift has been added as a dependency, and a mechanism for performing
  Thrift codegen at compile time has been added (see FindThrift.cmake,
  based on FindProtobuf.cmake)

- Bison has been added as a build-time dependency, because the system
  bison version on RHEL 6 is not sufficiently new enough for Thrift 0.10.

- Hive and Hadoop have been added to thirdparty as test-only dependencies.

- A Hive MetaStore external mini server is included for testing. See
  mini_hms.[h|cc].

- The Kudu Metastore plugin is compiled from CMake as a standalone jar
  for loading into the HMS mini server.

Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
---
M CMakeLists.txt
M build-support/dist_test.py
M build-support/iwyu/iwyu-filter.awk
M build-support/run_dist_test.py
A cmake_modules/FindJavaHome.cmake
A cmake_modules/FindThrift.cmake
A src/kudu/hms/CMakeLists.txt
A src/kudu/hms/hive_metastore.thrift
A src/kudu/hms/hms_client-test.cc
A src/kudu/hms/hms_client.cc
A src/kudu/hms/hms_client.h
A src/kudu/hms/mini_hms.cc
A src/kudu/hms/mini_hms.h
M thirdparty/LICENSE.txt
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
M thirdparty/download-thirdparty.sh
M thirdparty/vars.sh
18 files changed, 2,926 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/7053/33
--
To view, visit http://gerrit.cloudera.org:8080/7053
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 33
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-30 Thread Dan Burkert (Code Review)

Hello Tidy Bot, Alexey Serbin, Jean-Daniel Cryans, Kudu Jenkins, Adar Dembo, 
Todd Lipcon,

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

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

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..

KUDU-2191 (2/n): Hive Metastore client

This patch lays the groundwork for integrating the Kudu catalog with the
Hive MetaStore.

The focus of this patch is a Kudu-specific C++ HMS client
(hms_client.[h|cc]) in a new hms module. This client provides bindings
for the Hive Metastore APIs that Kudu will use in follow-up commits.

- Thrift has been added as a dependency, and a mechanism for performing
  Thrift codegen at compile time has been added (see FindThrift.cmake,
  based on FindProtobuf.cmake)

- Bison has been added as a build-time dependency, because the system
  bison version on RHEL 6 is not sufficiently new enough for Thrift 0.10.

- Hive and Hadoop have been added to thirdparty as test-only dependencies.

- A Hive MetaStore external mini server is included for testing. See
  mini_hms.[h|cc].

- The Kudu Metastore plugin is compiled from CMake as a standalone jar
  for loading into the HMS mini server.

Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
---
M CMakeLists.txt
M build-support/dist_test.py
M build-support/iwyu/iwyu-filter.awk
M build-support/run_dist_test.py
A cmake_modules/FindJavaHome.cmake
A cmake_modules/FindThrift.cmake
A src/kudu/hms/CMakeLists.txt
A src/kudu/hms/hive_metastore.thrift
A src/kudu/hms/hms_client-test.cc
A src/kudu/hms/hms_client.cc
A src/kudu/hms/hms_client.h
A src/kudu/hms/mini_hms.cc
A src/kudu/hms/mini_hms.h
M thirdparty/LICENSE.txt
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
M thirdparty/download-thirdparty.sh
M thirdparty/vars.sh
18 files changed, 2,924 insertions(+), 5 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 32
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-30 Thread Dan Burkert (Code Review)

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


Patch Set 31:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7053/31//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/7053/31//COMMIT_MSG@4
PS31, Line 4: d...@danburkert.com
revert



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 31
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 30 Oct 2017 14:56:46 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-30 Thread Dan Burkert (Code Review)

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


Patch Set 30:

(14 comments)

I've added TODOs to expand the error handling behavior and documentation in 
HmsClient.  I'd like for these things to be tested thoroughly, but this commit 
is already quite large when considering the build changes, so I'd like to leave 
it to a follow-up commit.

http://gerrit.cloudera.org:8080/#/c/7053/30/build-support/run_dist_test.py
File build-support/run_dist_test.py:

http://gerrit.cloudera.org:8080/#/c/7053/30/build-support/run_dist_test.py@128
PS30, Line 128:   env['HIVE_HOME'] = glob.glob(os.path.join(ROOT, 
"thirdparty/src/apache-hive-*-bin"))[0]
  :   env['HADOOP_HOME'] = glob.glob(os.path.join(ROOT, 
"thirdparty/src/hadoop-*"))[0]
> when we end up revving these dependencies it seems likely we'll end up with
Done


http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/CMakeLists.txt
File src/kudu/hms/CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/CMakeLists.txt@40
PS30, Line 40: execute_process(COMMAND ln -nsf
 : 
"${CMAKE_SOURCE_DIR}/thirdparty/installed/common/opt/hive"
 : "${EXECUTABLE_OUTPUT_PATH}/hive-home")
 : execute_process(COMMAND ln -nsf
 : 
"${CMAKE_SOURCE_DIR}/thirdparty/installed/common/opt/hadoop"
 : "${EXECUTABLE_OUTPUT_PATH}/hadoop-home")
 : execute_process(COMMAND ln -nsf
 : "${JAVA_HOME}"
 : "${EXECUTABLE_OUTPUT_PATH}/java-home")
> does it make more sense to do these in build-thirdparty? even though it's n
I don't think it's possible, since we can't know what directory the user will 
build Kudu in during the thirdparty build.


http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/hive_metastore.thrift
File src/kudu/hms/hive_metastore.thrift:

http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/hive_metastore.thrift@21
PS30, Line 21: # DO NOT MODIFY! Copied from
> should we be setting any C++ specific options like 'moveable types'?
I added moveable_types to FindThrift generation.  I don't think it really buys 
us much since all the service interfaces still takes args by const ref, but I 
guess it helps for building the arguments to begin with.


http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/hms_client-test.cc
File src/kudu/hms/hms_client-test.cc:

http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/hms_client-test.cc@103
PS30, Line 103:   ASSERT_TRUE(CreateTable(, database_name, table_name, 
"").IsRuntimeError());
> i think it's worth ASSERT_STR_MATCHES the error string here since RuntimeEr
Done


http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/hms_client.h
File src/kudu/hms/hms_client.h:

http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/hms_client.h@60
PS30, Line 60:   // can not be reached, an error is returned.
> is there some default timeout, etc?
Not currently, I've added a TODO.


http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/hms_client.h@73
PS30, Line 73:   // returned.
> it's probably worth a class-wide comment on what the errors returned will b
I've added a TODO.  I think we should have test coverage if we're going to 
document it, and I'd like to leave that to a follow-up commit.


http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/hms_client.h@85
PS30, Line 85: bool cascade = false,
 :   bool delete_data = true)
> would this be better off as a bitset of flags?
Not in my opinion.  What advantage would that have?  This method will only be 
used in tests, per the current design.


http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/hms_client.h@121
PS30, Line 121:int32_t max_events,
> do we have any idea of the expected size of these events? wondering if we n
They can be very large for HDFS tables with a lot of partitions.  There's 
currently no way to filter the events to just include Kudu tables, 
unfortunately.


http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/hms_client.h@124
PS30, Line 124:   // Deserializes a JSON encoded table.
> can you add a little more context as to where one would see the JSON-encode
Done


http://gerrit.cloudera.org:8080/#/c/7053/29/src/kudu/hms/hms_client.h
File src/kudu/hms/hms_client.h:

http://gerrit.cloudera.org:8080/#/c/7053/29/src/kudu/hms/hms_client.h@22
PS29, Line 22: #include 
> Could you at least add the IWYU pragma, please?  It should be something lik
sure, I completely forgot pragmas were an option.


http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/hms_client.cc
File src/kudu/hms/hms_client.cc:

http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/hms_client.cc@78
PS30, Line 78: IOError
> would RemoteError

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-30 Thread Dan Burkert (Code Review)

Hello Tidy Bot, Alexey Serbin, Jean-Daniel Cryans, Kudu Jenkins, Adar Dembo, 
Todd Lipcon,

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

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

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..

KUDU-2191 (2/n): Hive Metastore client

This patch lays the groundwork for integrating the Kudu catalog with the
Hive MetaStore.

The focus of this patch is a Kudu-specific C++ HMS client
(hms_client.[h|cc]) in a new hms module. This client provides bindings
for the Hive Metastore APIs that Kudu will use in follow-up commits.

- Thrift has been added as a dependency, and a mechanism for performing
  Thrift codegen at compile time has been added (see FindThrift.cmake,
  based on FindProtobuf.cmake)

- Bison has been added as a build-time dependency, because the system
  bison version on RHEL 6 is not sufficiently new enough for Thrift 0.10.

- Hive and Hadoop have been added to thirdparty as test-only dependencies.

- A Hive MetaStore external mini server is included for testing. See
  mini_hms.[h|cc].

- The Kudu Metastore plugin is compiled from CMake as a standalone jar
  for loading into the HMS mini server.

Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
---
M CMakeLists.txt
M build-support/dist_test.py
M build-support/iwyu/iwyu-filter.awk
M build-support/run_dist_test.py
A cmake_modules/FindJavaHome.cmake
A cmake_modules/FindThrift.cmake
A src/kudu/hms/CMakeLists.txt
A src/kudu/hms/hive_metastore.thrift
A src/kudu/hms/hms_client-test.cc
A src/kudu/hms/hms_client.cc
A src/kudu/hms/hms_client.h
A src/kudu/hms/mini_hms.cc
A src/kudu/hms/mini_hms.h
M thirdparty/LICENSE.txt
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
M thirdparty/download-thirdparty.sh
M thirdparty/vars.sh
18 files changed, 2,920 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/7053/31
--
To view, visit http://gerrit.cloudera.org:8080/7053
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 31
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-23 Thread Todd Lipcon (Code Review)

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


Patch Set 30:

(15 comments)

http://gerrit.cloudera.org:8080/#/c/7053/30/build-support/run_dist_test.py
File build-support/run_dist_test.py:

http://gerrit.cloudera.org:8080/#/c/7053/30/build-support/run_dist_test.py@128
PS30, Line 128:   env['HIVE_HOME'] = glob.glob(os.path.join(ROOT, 
"thirdparty/src/apache-hive-*-bin"))[0]
  :   env['HADOOP_HOME'] = glob.glob(os.path.join(ROOT, 
"thirdparty/src/hadoop-*"))[0]
when we end up revving these dependencies it seems likely we'll end up with 
multiple ones in src/. Could we do use the installed/opt/ symlink that you make 
elsewhere for this?


http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/CMakeLists.txt
File src/kudu/hms/CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/CMakeLists.txt@40
PS30, Line 40: execute_process(COMMAND ln -nsf
 : 
"${CMAKE_SOURCE_DIR}/thirdparty/installed/common/opt/hive"
 : "${EXECUTABLE_OUTPUT_PATH}/hive-home")
 : execute_process(COMMAND ln -nsf
 : 
"${CMAKE_SOURCE_DIR}/thirdparty/installed/common/opt/hadoop"
 : "${EXECUTABLE_OUTPUT_PATH}/hadoop-home")
 : execute_process(COMMAND ln -nsf
 : "${JAVA_HOME}"
 : "${EXECUTABLE_OUTPUT_PATH}/java-home")
does it make more sense to do these in build-thirdparty? even though it's not a 
"build" step it seems to fit a little closer there (at least for the 
hive/hadoop ones, not necessarily the java one)


http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/hive_metastore.thrift
File src/kudu/hms/hive_metastore.thrift:

http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/hive_metastore.thrift@21
PS30, Line 21: # DO NOT MODIFY! Copied from
should we be setting any C++ specific options like 'moveable types'?


http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/hms_client-test.cc
File src/kudu/hms/hms_client-test.cc:

http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/hms_client-test.cc@103
PS30, Line 103:   ASSERT_TRUE(CreateTable(, database_name, table_name, 
"").IsRuntimeError());
i think it's worth ASSERT_STR_MATCHES the error string here since RuntimeError 
is so generic


http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/hms_client.h
File src/kudu/hms/hms_client.h:

http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/hms_client.h@60
PS30, Line 60:   // can not be reached, an error is returned.
is there some default timeout, etc?


http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/hms_client.h@73
PS30, Line 73:   // returned.
it's probably worth a class-wide comment on what the errors returned will be 
for other types of issues, such as if the RPC times out or the HMS has 
disconnected, etc.


http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/hms_client.h@85
PS30, Line 85: bool cascade = false,
 :   bool delete_data = true)
would this be better off as a bitset of flags?


http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/hms_client.h@121
PS30, Line 121:int32_t max_events,
do we have any idea of the expected size of these events? wondering if we need 
to be thinking about memory consumption of this component carefully or not


http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/hms_client.h@124
PS30, Line 124:   // Deserializes a JSON encoded table.
can you add a little more context as to where one would see the JSON-encoded 
table format?


http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/hms_client.cc
File src/kudu/hms/hms_client.cc:

http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/hms_client.cc@78
PS30, Line 78: IOError
would RemoteError be better here?

Is there any more specific error to distinguish network issues and timeouts vs 
exceptions thrown on the remote end?


http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/hms_client.cc@94
PS30, Line 94: HmsClient::HmsClient(const HostPort& hms_address)
is there a world in which people run multiple HMS in an HA configuration? If so 
is that typically done by configuring clients or putting the HMSs behind a 
proxy or vip?

Do we need to support auto-reconnect after failure? eg if someone just bounces 
the HMS how does the Kudu master know to reconnect/retry? Is that assumed to be 
at a higher layer? Might be worth documenting this at the class-doc level.


http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/hms_client.cc@96
PS30, Line 96:   auto socket = make_shared(hms_address.host(), 
hms_address.port());
no need to set timeouts on the TSocket?


http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/hms_client.cc@99
PS30, Line 99:   client_ =

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-23 Thread Adar Dembo (Code Review)

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


Patch Set 30:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7053/29/src/kudu/hms/mini_hms.h
File src/kudu/hms/mini_hms.h:

http://gerrit.cloudera.org:8080/#/c/7053/29/src/kudu/hms/mini_hms.h@30
PS29, Line 30: #include "kudu/util/subprocess.h"
> unique_ptr doesn't work with forward declared classes.
You sure about that? I'm looking at 
https://stackoverflow.com/questions/6012157/is-stdunique-ptrt-required-to-know-the-full-definition-of-t
 and it suggests that, for most operations incomplete knowledge of the class is 
OK.

I also wrote a small test that's similar to mini_hms to prove this to myself:

  --bar.cc--
  #include "bar.h"
  #include "foo.h"

  Bar::~Bar() {}
  void Bar::CreateFoo() {
f.reset(new Foo());
f->a = "hello world";
  }

  --test.cc--
  #include "bar.h"
  #include "foo.h"

  int main(void) {
Bar b;
b.CreateFoo();
return 0;
  }

  --bar.h--
  #include 

  struct Foo;
  struct Bar {
Bar() = default;
~Bar();
void CreateFoo();
   private:
std::unique_ptr f;
  };

  --foo.h--
  #include 

  struct Foo {
std::string a;
  };

This test compiles and links. As you can see, bar.h has only partial knowledge 
of Foo, but full knowledge of it where needed (bar.cc and test.cc).

Also, if I follow Alexey's advice and move Bar's noarg constructor from bar.h 
to bar.cc, I can remove the inclusion of foo.h from test.cc.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 30
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 23 Oct 2017 19:47:45 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-23 Thread Alexey Serbin (Code Review)

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


Patch Set 29:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7053/29/src/kudu/hms/hms_client-test.cc
File src/kudu/hms/hms_client-test.cc:

http://gerrit.cloudera.org:8080/#/c/7053/29/src/kudu/hms/hms_client-test.cc@25
PS29, Line 25: #include 
 : #include 
> stl_logging.h is necessary for the stream formatting on line 194.  I'll rem
It seems stl_loggin.h is a frequent point of confusion for IWYU.  I'll take a 
look whether it's possible to handle it with a mapping.

Meanwhile, could you add IWYU pragma, please:

#include   // IWYU pragma: keep


http://gerrit.cloudera.org:8080/#/c/7053/29/src/kudu/hms/hms_client.h
File src/kudu/hms/hms_client.h:

http://gerrit.cloudera.org:8080/#/c/7053/29/src/kudu/hms/hms_client.h@22
PS29, Line 22: #include 
> perhaps, but I don't think I should remove it.
Could you at least add the IWYU pragma, please?  It should be something like:

#include  // IWYU pragma: keep



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 29
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 23 Oct 2017 19:41:36 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-23 Thread Alexey Serbin (Code Review)

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


Patch Set 29:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7053/29/src/kudu/hms/mini_hms.h
File src/kudu/hms/mini_hms.h:

http://gerrit.cloudera.org:8080/#/c/7053/29/src/kudu/hms/mini_hms.h@30
PS29, Line 30: #include "kudu/util/subprocess.h"
> unique_ptr doesn't work with forward declared classes.
It depends on what 'work' is required :)

The funny fact here is that if you move the definition of the MiniHms 
constructor into the .cc file, it will work with forward-declared Subprocess 
class.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 29
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 23 Oct 2017 19:31:18 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-23 Thread Dan Burkert (Code Review)

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


Patch Set 30: Verified+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 30
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 23 Oct 2017 14:45:40 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-23 Thread Dan Burkert (Code Review)

Dan Burkert has removed a vote on this change.

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 30
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-23 Thread Dan Burkert (Code Review)

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


Patch Set 30:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/7053/29/src/kudu/hms/hms_client-test.cc
File src/kudu/hms/hms_client-test.cc:

http://gerrit.cloudera.org:8080/#/c/7053/29/src/kudu/hms/hms_client-test.cc@25
PS29, Line 25: #include 
 : #include 
> I think IWYU is right about removing these; I don't see any LOG/VLOG usage
stl_logging.h is necessary for the stream formatting on line 194.  I'll remove 
glog/logging.h.


http://gerrit.cloudera.org:8080/#/c/7053/29/src/kudu/hms/hms_client.h
File src/kudu/hms/hms_client.h:

http://gerrit.cloudera.org:8080/#/c/7053/29/src/kudu/hms/hms_client.h@22
PS29, Line 22: #include 
> Maybe IWYU wants to remove this because:
perhaps, but I don't think I should remove it.


http://gerrit.cloudera.org:8080/#/c/7053/29/src/kudu/hms/hms_client.cc
File src/kudu/hms/hms_client.cc:

http://gerrit.cloudera.org:8080/#/c/7053/29/src/kudu/hms/hms_client.cc@26
PS29, Line 26:
> Indeed, it seems IWYU is confused, and I cannot see why this happens.
Done


http://gerrit.cloudera.org:8080/#/c/7053/29/src/kudu/hms/mini_hms.h
File src/kudu/hms/mini_hms.h:

http://gerrit.cloudera.org:8080/#/c/7053/29/src/kudu/hms/mini_hms.h@30
PS29, Line 30: #include "kudu/util/subprocess.h"
> IWYU's recommendation to remove this makes sense: you can forward declare S
unique_ptr doesn't work with forward declared classes.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 30
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 23 Oct 2017 14:05:41 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-23 Thread Dan Burkert (Code Review)

Hello Tidy Bot, Alexey Serbin, Jean-Daniel Cryans, Kudu Jenkins, Adar Dembo, 
Todd Lipcon,

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

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

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..

KUDU-2191 (2/n): Hive Metastore client

This patch lays the groundwork for integrating the Kudu catalog with the
Hive MetaStore.

The focus of this patch is a Kudu-specific C++ HMS client
(hms_client.[h|cc]) in a new hms module. This client provides bindings
for the Hive Metastore APIs that Kudu will use in follow-up commits.

- Thrift has been added as a dependency, and a mechanism for performing
  Thrift codegen at compile time has been added (see FindThrift.cmake,
  based on FindProtobuf.cmake)

- Bison has been added as a build-time dependency, because the system
  bison version on RHEL 6 is not sufficiently new enough for Thrift 0.10.

- Hive and Hadoop have been added to thirdparty as test-only dependencies.

- A Hive MetaStore external mini server is included for testing. See
  mini_hms.[h|cc].

- The Kudu Metastore plugin is compiled from CMake as a standalone jar
  for loading into the HMS mini server.

Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
---
M CMakeLists.txt
M build-support/dist_test.py
M build-support/iwyu/iwyu-filter.awk
M build-support/run_dist_test.py
A cmake_modules/FindJavaHome.cmake
A cmake_modules/FindThrift.cmake
A src/kudu/hms/CMakeLists.txt
A src/kudu/hms/hive_metastore.thrift
A src/kudu/hms/hms_client-test.cc
A src/kudu/hms/hms_client.cc
A src/kudu/hms/hms_client.h
A src/kudu/hms/mini_hms.cc
A src/kudu/hms/mini_hms.h
M thirdparty/LICENSE.txt
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
M thirdparty/download-thirdparty.sh
M thirdparty/vars.sh
18 files changed, 2,911 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/7053/30
--
To view, visit http://gerrit.cloudera.org:8080/7053
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 30
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-20 Thread Alexey Serbin (Code Review)

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


Patch Set 29:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7053/29/src/kudu/hms/hms_client.cc
File src/kudu/hms/hms_client.cc:

http://gerrit.cloudera.org:8080/#/c/7053/29/src/kudu/hms/hms_client.cc@26
PS29, Line 26:
> I'll take a look at that.
Indeed, it seems IWYU is confused, and I cannot see why this happens.

As a temporary workaround, please add path to this file into the 'muted'   in 
$KUDU_ROOT/build-support/iwyu/iwyu-filter.awk

...
  muted["kudu/hms/hms_client.cc"]
...

I'll deal with it a bit later.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 29
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Sat, 21 Oct 2017 00:35:43 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-20 Thread Alexey Serbin (Code Review)

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


Patch Set 24:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/7053/24/src/kudu/hms/mini_hms.cc@101
PS24, Line 101:   map env_vars {
> Why should this variable in particular be const?
Since it's not intended to change in the scope, it might make sense to mark in 
const.  It's just a nit, feel free to ignore.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 24
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Sat, 21 Oct 2017 00:12:49 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-20 Thread Alexey Serbin (Code Review)

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


Patch Set 29:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7053/29/src/kudu/hms/hms_client.cc
File src/kudu/hms/hms_client.cc:

http://gerrit.cloudera.org:8080/#/c/7053/29/src/kudu/hms/hms_client.cc@26
PS29, Line 26:
> Yeah, IWYU's recommendations for this file don't make sense. Could you let
I'll take a look at that.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 29
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Sat, 21 Oct 2017 00:10:27 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-19 Thread Adar Dembo (Code Review)

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


Patch Set 29:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/7053/29/src/kudu/hms/hms_client-test.cc
File src/kudu/hms/hms_client-test.cc:

http://gerrit.cloudera.org:8080/#/c/7053/29/src/kudu/hms/hms_client-test.cc@25
PS29, Line 25: #include 
 : #include 
I think IWYU is right about removing these; I don't see any LOG/VLOG usage here.


http://gerrit.cloudera.org:8080/#/c/7053/29/src/kudu/hms/hms_client.h
File src/kudu/hms/hms_client.h:

http://gerrit.cloudera.org:8080/#/c/7053/29/src/kudu/hms/hms_client.h@22
PS29, Line 22: #include 
Maybe IWYU wants to remove this because:
1. All of the references are OUT by-pointer params, and
2. One of the included headers forward declares std::vector?


http://gerrit.cloudera.org:8080/#/c/7053/29/src/kudu/hms/hms_client.cc
File src/kudu/hms/hms_client.cc:

http://gerrit.cloudera.org:8080/#/c/7053/29/src/kudu/hms/hms_client.cc@26
PS29, Line 26:
Yeah, IWYU's recommendations for this file don't make sense. Could you let 
Alexey know?

The problem with ignoring IWYU outright is that the next patch to touch this 
file will continue to fail IWYU.


http://gerrit.cloudera.org:8080/#/c/7053/29/src/kudu/hms/mini_hms.h
File src/kudu/hms/mini_hms.h:

http://gerrit.cloudera.org:8080/#/c/7053/29/src/kudu/hms/mini_hms.h@30
PS29, Line 30: #include "kudu/util/subprocess.h"
IWYU's recommendation to remove this makes sense: you can forward declare 
Subprocess.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 29
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 19 Oct 2017 23:20:45 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-19 Thread Dan Burkert (Code Review)

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


Patch Set 29:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/7053/29/src/kudu/hms/mini_hms.cc@121
PS29, Line 121:   return WaitForTcpBind(hms_process_->pid(), _, 
MonoDelta::FromSeconds(20));
> Any particular reason this timeout shouldn't be ridiculously high? Like, 90
No tests wait for this to elapse unless Hive fails to start up.  I verified 20 
seconds is long enough on a dist-test run.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 29
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 19 Oct 2017 23:04:20 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-19 Thread Dan Burkert (Code Review)

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


Patch Set 29: Code-Review+1

IWYU is confused.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 29
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 19 Oct 2017 23:02:39 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-19 Thread Dan Burkert (Code Review)

Dan Burkert has removed a vote on this change.

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 29
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-19 Thread Dan Burkert (Code Review)

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


Patch Set 29:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7053/22/thirdparty/vars.sh
File thirdparty/vars.sh:

http://gerrit.cloudera.org:8080/#/c/7053/22/thirdparty/vars.sh@212
PS22, Line 212: BISON_NAME=bison-$BISON_VERSION
> Every other commit hash in this file also includes a link to a repository,
That’s just the thing, though, none of those other dependencies are trivial to 
find.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 29
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 19 Oct 2017 23:01:35 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-19 Thread Adar Dembo (Code Review)

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


Patch Set 29:

(3 comments)

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

http://gerrit.cloudera.org:8080/#/c/7053/29/src/kudu/hms/mini_hms.cc@121
PS29, Line 121:   return WaitForTcpBind(hms_process_->pid(), _, 
MonoDelta::FromSeconds(20));
Any particular reason this timeout shouldn't be ridiculously high? Like, 90s? 
Do we have any tests that need to wait for it to fully elapse?


http://gerrit.cloudera.org:8080/#/c/7053/22/thirdparty/vars.sh
File thirdparty/vars.sh:

http://gerrit.cloudera.org:8080/#/c/7053/22/thirdparty/vars.sh@212
PS22, Line 212: BISON_NAME=bison-$BISON_VERSION
> It's a SHA from the upstream Apache git repository.  Can we not assume cont
Every other commit hash in this file also includes a link to a repository, no 
matter how trivially accessible. I don't see why we should make an exception 
for Hive (or Thrift) when it's so easy to add a one line URL to the comment.

Put another way, can you justify these exceptions, or explain why adding these 
URLs doesn't sit with you?


http://gerrit.cloudera.org:8080/#/c/7053/26/thirdparty/vars.sh
File thirdparty/vars.sh:

http://gerrit.cloudera.org:8080/#/c/7053/26/thirdparty/vars.sh@203
PS26, Line 203: # TODO(dan): bump to 0.11 when it's released. We chose to use a 
bleeding edge
> The Apache project page is linked to in the license file.  The apache git r
You could link to the github mirror: https://github.com/apache/thrift. That 
would be fine too.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 29
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 19 Oct 2017 17:48:50 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-18 Thread Dan Burkert (Code Review)

Hello Tidy Bot, Alexey Serbin, Jean-Daniel Cryans, Kudu Jenkins, Adar Dembo, 
Todd Lipcon,

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

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

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..

KUDU-2191 (2/n): Hive Metastore client

This patch lays the groundwork for integrating the Kudu catalog with the
Hive MetaStore.

The focus of this patch is a Kudu-specific C++ HMS client
(hms_client.[h|cc]) in a new hms module. This client provides bindings
for the Hive Metastore APIs that Kudu will use in follow-up commits.

- Thrift has been added as a dependency, and a mechanism for performing
  Thrift codegen at compile time has been added (see FindThrift.cmake,
  based on FindProtobuf.cmake)

- Bison has been added as a build-time dependency, because the system
  bison version on RHEL 6 is not sufficiently new enough for Thrift 0.10.

- Hive and Hadoop have been added to thirdparty as test-only dependencies.

- A Hive MetaStore external mini server is included for testing. See
  mini_hms.[h|cc].

- The Kudu Metastore plugin is compiled from CMake as a standalone jar
  for loading into the HMS mini server.

Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
---
M CMakeLists.txt
M build-support/dist_test.py
M build-support/run_dist_test.py
A cmake_modules/FindJavaHome.cmake
A cmake_modules/FindThrift.cmake
A src/kudu/hms/CMakeLists.txt
A src/kudu/hms/hive_metastore.thrift
A src/kudu/hms/hms_client-test.cc
A src/kudu/hms/hms_client.cc
A src/kudu/hms/hms_client.h
A src/kudu/hms/mini_hms.cc
A src/kudu/hms/mini_hms.h
M thirdparty/LICENSE.txt
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
M thirdparty/download-thirdparty.sh
M thirdparty/vars.sh
17 files changed, 2,911 insertions(+), 5 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 29
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-18 Thread Dan Burkert (Code Review)

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


Patch Set 27:

Dissapointingly, the 10s limit on starting up the mini hms was too aggressive 
causing flakiness, so I bumped it to 20s.  No failures in 1000 runs.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 27
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 19 Oct 2017 03:38:31 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-18 Thread Dan Burkert (Code Review)

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


Patch Set 27:

(4 comments)

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

http://gerrit.cloudera.org:8080/#/c/7053/24/src/kudu/hms/mini_hms.cc@101
PS24, Line 101:   map env_vars {
> nit: const ?
Why should this variable in particular be const?


http://gerrit.cloudera.org:8080/#/c/7053/22/thirdparty/vars.sh
File thirdparty/vars.sh:

http://gerrit.cloudera.org:8080/#/c/7053/22/thirdparty/vars.sh@212
PS22, Line 212: BISON_NAME=bison-$BISON_VERSION
> PS26 has more info about how we don't use a release, but the provenance is
It's a SHA from the upstream Apache git repository.  Can we not assume 
contributors to Apache Kudu can find the Apache Hive repository?


http://gerrit.cloudera.org:8080/#/c/7053/26/thirdparty/vars.sh
File thirdparty/vars.sh:

http://gerrit.cloudera.org:8080/#/c/7053/26/thirdparty/vars.sh@203
PS26, Line 203: # TODO(dan): bump to 0.11 when it's released. We chose to use a 
bleeding edge
> Could you also include a URL here to the Thrift repo?
The Apache project page is linked to in the license file.  The apache git repo 
has a pretty useless web ui.


http://gerrit.cloudera.org:8080/#/c/7053/26/thirdparty/vars.sh@206
PS26, Line 206: .
> Nit: got an extra period here.
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 27
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 19 Oct 2017 01:26:51 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-18 Thread Dan Burkert (Code Review)

Hello Tidy Bot, Alexey Serbin, Jean-Daniel Cryans, Kudu Jenkins, Adar Dembo, 
Todd Lipcon,

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

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

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..

KUDU-2191 (2/n): Hive Metastore client

This patch lays the groundwork for integrating the Kudu catalog with the
Hive MetaStore.

The focus of this patch is a Kudu-specific C++ HMS client
(hms_client.[h|cc]) in a new hms module. This client provides bindings
for the Hive Metastore APIs that Kudu will use in follow-up commits.

- Thrift has been added as a dependency, and a mechanism for performing
  Thrift codegen at compile time has been added (see FindThrift.cmake,
  based on FindProtobuf.cmake)

- Bison has been added as a build-time dependency, because the system
  bison version on RHEL 6 is not sufficiently new enough for Thrift 0.10.

- Hive and Hadoop have been added to thirdparty as test-only dependencies.

- A Hive MetaStore external mini server is included for testing. See
  mini_hms.[h|cc].

- The Kudu Metastore plugin is compiled from CMake as a standalone jar
  for loading into the HMS mini server.

Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
---
M CMakeLists.txt
M build-support/dist_test.py
M build-support/run_dist_test.py
A cmake_modules/FindJavaHome.cmake
A cmake_modules/FindThrift.cmake
A src/kudu/hms/CMakeLists.txt
A src/kudu/hms/hive_metastore.thrift
A src/kudu/hms/hms_client-test.cc
A src/kudu/hms/hms_client.cc
A src/kudu/hms/hms_client.h
A src/kudu/hms/mini_hms.cc
A src/kudu/hms/mini_hms.h
M src/kudu/util/test_util.cc
M thirdparty/LICENSE.txt
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
M thirdparty/download-thirdparty.sh
M thirdparty/vars.sh
18 files changed, 2,915 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/7053/28
--
To view, visit http://gerrit.cloudera.org:8080/7053
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 28
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-18 Thread Dan Burkert (Code Review)

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


Patch Set 27:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/7053/24/src/kudu/hms/hive_metastore.thrift
File src/kudu/hms/hive_metastore.thrift:

PS24:
> A possible alternative handling the necessity to refresh this file when swi
So I've actually gone back the other way and added a note that this file 
shouldn't be updated, because to do so implies that a breaking change is being 
made.  We should be very careful about doing that, since we should remain 
compatible with a range of HMS versions.


http://gerrit.cloudera.org:8080/#/c/7053/24/src/kudu/hms/hms_client.h
File src/kudu/hms/hms_client.h:

http://gerrit.cloudera.org:8080/#/c/7053/24/src/kudu/hms/hms_client.h@98
PS24, Line 98: Status AlterTable(const std::string& database_name,
 : const std::string& table_name,
 : const hive::Table& table)
> nit: does it make sense to add WARN_UNUSED_RESULT here as well?
Done


http://gerrit.cloudera.org:8080/#/c/7053/24/src/kudu/hms/hms_client.h@108
PS24, Line 108: database
> nit: here and in GetAllTables() -- consider using 'database_name' to match
Done


http://gerrit.cloudera.org:8080/#/c/7053/25/src/kudu/hms/hms_client.cc
File src/kudu/hms/hms_client.cc:

http://gerrit.cloudera.org:8080/#/c/7053/25/src/kudu/hms/hms_client.cc@99
PS25, Line 99: : client_(nullptr) {
> Can we use std::make_shared here?
Done


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

http://gerrit.cloudera.org:8080/#/c/7053/24/src/kudu/hms/mini_hms.cc@125
PS24, Line 125: 'datanucleus.schem
> Which options is this referring to? datanucleus.schema.autoCreateAll and hi
Done


http://gerrit.cloudera.org:8080/#/c/7053/24/thirdparty/build-definitions.sh
File thirdparty/build-definitions.sh:

http://gerrit.cloudera.org:8080/#/c/7053/24/thirdparty/build-definitions.sh@728
PS24, Line 728:   mkdir -p $THRIFT_BDIR
  :   pushd $THRIFT_BDIR
  :   rm -Rf CMakeCache.txt CMakeFiles/
  :
  :   # Thrift
> paranoid nit: instead, maybe
I ended up changing this per Adar's suggestion.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 27
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 18 Oct 2017 23:52:08 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-18 Thread Dan Burkert (Code Review)

Hello Alexey Serbin, Jean-Daniel Cryans, Kudu Jenkins, Adar Dembo, Todd Lipcon,

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

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

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..

KUDU-2191 (2/n): Hive Metastore client

This patch lays the groundwork for integrating the Kudu catalog with the
Hive MetaStore.

The focus of this patch is a Kudu-specific C++ HMS client
(hms_client.[h|cc]) in a new hms module. This client provides bindings
for the Hive Metastore APIs that Kudu will use in follow-up commits.

- Thrift has been added as a dependency, and a mechanism for performing
  Thrift codegen at compile time has been added (see FindThrift.cmake,
  based on FindProtobuf.cmake)

- Bison has been added as a build-time dependency, because the system
  bison version on RHEL 6 is not sufficiently new enough for Thrift 0.10.

- Hive and Hadoop have been added to thirdparty as test-only dependencies.

- A Hive MetaStore external mini server is included for testing. See
  mini_hms.[h|cc].

- The Kudu Metastore plugin is compiled from CMake as a standalone jar
  for loading into the HMS mini server.

Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
---
M CMakeLists.txt
M build-support/dist_test.py
M build-support/run_dist_test.py
A cmake_modules/FindJavaHome.cmake
A cmake_modules/FindThrift.cmake
A src/kudu/hms/CMakeLists.txt
A src/kudu/hms/hive_metastore.thrift
A src/kudu/hms/hms_client-test.cc
A src/kudu/hms/hms_client.cc
A src/kudu/hms/hms_client.h
A src/kudu/hms/mini_hms.cc
A src/kudu/hms/mini_hms.h
M thirdparty/LICENSE.txt
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
M thirdparty/download-thirdparty.sh
M thirdparty/vars.sh
17 files changed, 2,915 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/7053/27
--
To view, visit http://gerrit.cloudera.org:8080/7053
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

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

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-18 Thread Alexey Serbin (Code Review)

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


Patch Set 24:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/7053/24/src/kudu/hms/hive_metastore.thrift
File src/kudu/hms/hive_metastore.thrift:

PS24:
A possible alternative handling the necessity to refresh this file when 
switching to a new version of Hive: maybe, put this file into the Hive distro 
tarball and install it upon unpacking the apache-hive-$HIVE_VERSION-bin tarball 
somewhere under thirdparty/installed ?


http://gerrit.cloudera.org:8080/#/c/7053/24/src/kudu/hms/hms_client.h
File src/kudu/hms/hms_client.h:

http://gerrit.cloudera.org:8080/#/c/7053/24/src/kudu/hms/hms_client.h@98
PS24, Line 98: Status AlterTable(const std::string& database_name,
 : const std::string& table_name,
 : const hive::Table& table);
nit: does it make sense to add WARN_UNUSED_RESULT here as well?


http://gerrit.cloudera.org:8080/#/c/7053/24/src/kudu/hms/hms_client.h@108
PS24, Line 108: database
nit: here and in GetAllTables() -- consider using 'database_name' to match the 
rest of those methods, or just replace 'database_name' with 'database' 
elsewhere.


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

http://gerrit.cloudera.org:8080/#/c/7053/24/src/kudu/hms/mini_hms.cc@101
PS24, Line 101:   map env_vars {
nit: const ?


http://gerrit.cloudera.org:8080/#/c/7053/24/thirdparty/build-definitions.sh
File thirdparty/build-definitions.sh:

http://gerrit.cloudera.org:8080/#/c/7053/24/thirdparty/build-definitions.sh@728
PS24, Line 728:   mkdir -p $THRIFT_BDIR
  :   pushd $THRIFT_BDIR
  :   # Normally we would only remove cached CMake files, but the 
Thrift build is
  :   # flaky on CI unless all build state is removed.
  :   rm -Rf *
paranoid nit: instead, maybe

rm -Rf $THRIFT_BDIR
mkdir -p $THRIFT_BDIR
pushd $THRIFT_BDIR

?


http://gerrit.cloudera.org:8080/#/c/7053/24/thirdparty/vars.sh
File thirdparty/vars.sh:

http://gerrit.cloudera.org:8080/#/c/7053/24/thirdparty/vars.sh@211
PS24, Line 211: # When bumping the Hive versions, also update 
hive_metastore.thrift.
Consider the 'automated' way of updating the hive_metastore.thrift file as 
described in file comment for thirdparty/vars.sh



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 24
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 18 Oct 2017 22:54:37 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-18 Thread Adar Dembo (Code Review)

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


Patch Set 26:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/7053/25/src/kudu/hms/hms_client.cc
File src/kudu/hms/hms_client.cc:

http://gerrit.cloudera.org:8080/#/c/7053/25/src/kudu/hms/hms_client.cc@99
PS25, Line 99:   shared_ptr socket(new TSocket(hms_address.host(), 
hms_address.port()));
Can we use std::make_shared here?


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

http://gerrit.cloudera.org:8080/#/c/7053/24/src/kudu/hms/mini_hms.cc@125
PS24, Line 125: 'datanucleus.schem
> Which options is this referring to? datanucleus.schema.autoCreateAll and hi
I think you missed this one.


http://gerrit.cloudera.org:8080/#/c/7053/22/thirdparty/vars.sh
File thirdparty/vars.sh:

http://gerrit.cloudera.org:8080/#/c/7053/22/thirdparty/vars.sh@212
PS22, Line 212: BISON_NAME=bison-$BISON_VERSION
> I'm not sure, I always just download the tarball via GitHub's 'download' bu
PS26 has more info about how we don't use a release, but the provenance is 
still incomplete (i.e. it needs a URL to a repository).


http://gerrit.cloudera.org:8080/#/c/7053/26/thirdparty/vars.sh
File thirdparty/vars.sh:

http://gerrit.cloudera.org:8080/#/c/7053/26/thirdparty/vars.sh@203
PS26, Line 203: # TODO(dan): bump to 0.11 when it's released. We chose to use a 
bleeding edge
Could you also include a URL here to the Thrift repo?


http://gerrit.cloudera.org:8080/#/c/7053/26/thirdparty/vars.sh@206
PS26, Line 206: .
Nit: got an extra period here.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 26
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 18 Oct 2017 22:45:55 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-18 Thread Dan Burkert (Code Review)

Hello Alexey Serbin, Jean-Daniel Cryans, Kudu Jenkins, Adar Dembo, Todd Lipcon,

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

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

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..

KUDU-2191 (2/n): Hive Metastore client

This patch lays the groundwork for integrating the Kudu catalog with the
Hive MetaStore.

The focus of this patch is a Kudu-specific C++ HMS client
(hms_client.[h|cc]) in a new hms module. This client provides bindings
for the Hive Metastore APIs that Kudu will use in follow-up commits.

- Thrift has been added as a dependency, and a mechanism for performing
  Thrift codegen at compile time has been added (see FindThrift.cmake,
  based on FindProtobuf.cmake)

- Bison has been added as a build-time dependency, because the system
  bison version on RHEL 6 is not sufficiently new enough for Thrift 0.10.

- Hive and Hadoop have been added to thirdparty as test-only dependencies.

- A Hive MetaStore external mini server is included for testing. See
  mini_hms.[h|cc].

- The Kudu Metastore plugin is compiled from CMake as a standalone jar
  for loading into the HMS mini server.

Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
---
M CMakeLists.txt
M build-support/dist_test.py
M build-support/run_dist_test.py
A cmake_modules/FindJavaHome.cmake
A cmake_modules/FindThrift.cmake
A src/kudu/hms/CMakeLists.txt
A src/kudu/hms/hive_metastore.thrift
A src/kudu/hms/hms_client-test.cc
A src/kudu/hms/hms_client.cc
A src/kudu/hms/hms_client.h
A src/kudu/hms/mini_hms.cc
A src/kudu/hms/mini_hms.h
M thirdparty/LICENSE.txt
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
M thirdparty/download-thirdparty.sh
M thirdparty/vars.sh
17 files changed, 2,912 insertions(+), 5 deletions(-)


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

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

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-18 Thread Dan Burkert (Code Review)

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


Patch Set 22:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/7053/22/src/kudu/hms/CMakeLists.txt
File src/kudu/hms/CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/7053/22/src/kudu/hms/CMakeLists.txt@31
PS22, Line 31:   kudu_common
> Not done? I still see kudu_common in HMS_DEPS.
Done


http://gerrit.cloudera.org:8080/#/c/7053/5/src/kudu/hms/hive_metastore-test.cc
File src/kudu/hms/hive_metastore-test.cc:

http://gerrit.cloudera.org:8080/#/c/7053/5/src/kudu/hms/hive_metastore-test.cc@23
PS5, Line 23:
> I want to punt on this until we have two cases where it's necessary, I'm st
OK so this has come back to haunt me in a big way.


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

http://gerrit.cloudera.org:8080/#/c/7053/22/src/kudu/hms/mini_hms.cc@125
PS22, Line 125:   static const string kFileTemplate = R"(
> Could you also add a comment for hive.metastore.event.db.listener.timetoliv
Done


http://gerrit.cloudera.org:8080/#/c/7053/22/thirdparty/build-definitions.sh
File thirdparty/build-definitions.sh:

http://gerrit.cloudera.org:8080/#/c/7053/22/thirdparty/build-definitions.sh@730
PS22, Line 730:   rm -Rf *
> OK, but every single rebuild of thirdparty is going to have to rebuild thri
OK I'm going to remove this until the issue reappears.  I don't see the point 
in knowing how long it takes to build a cold Thrift when the alternative is 
that it doesn't build at all.


http://gerrit.cloudera.org:8080/#/c/7053/22/thirdparty/vars.sh
File thirdparty/vars.sh:

http://gerrit.cloudera.org:8080/#/c/7053/22/thirdparty/vars.sh@212
PS22, Line 212: HIVE_VERSION=3fb4649fa847cfec33f701f6c884f12087680cf0
> OK but could you at least leave a breadcrumb here pointing to the git repos
I'm not sure, I always just download the tarball via GitHub's 'download' button.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 22
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 18 Oct 2017 22:28:45 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-18 Thread Dan Burkert (Code Review)

Hello Alexey Serbin, Jean-Daniel Cryans, Kudu Jenkins, Adar Dembo, Todd Lipcon,

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

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

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..

KUDU-2191 (2/n): Hive Metastore client

This patch lays the groundwork for integrating the Kudu catalog with the
Hive MetaStore.

The focus of this patch is a Kudu-specific C++ HMS client
(hms_client.[h|cc]) in a new hms module. This client provides bindings
for the Hive Metastore APIs that Kudu will use in follow-up commits.

- Thrift has been added as a dependency, and a mechanism for performing
  Thrift codegen at compile time has been added (see FindThrift.cmake,
  based on FindProtobuf.cmake)

- Bison has been added as a build-time dependency, because the system
  bison version on RHEL 6 is not sufficiently new enough for Thrift 0.10.

- Hive and Hadoop have been added to thirdparty as test-only dependencies.

- A Hive MetaStore external mini server is included for testing. See
  mini_hms.[h|cc].

- The Kudu Metastore plugin is compiled from CMake as a standalone jar
  for loading into the HMS mini server.

Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
---
M CMakeLists.txt
M build-support/dist_test.py
M build-support/run_dist_test.py
A cmake_modules/FindJavaHome.cmake
A cmake_modules/FindThrift.cmake
A src/kudu/hms/CMakeLists.txt
A src/kudu/hms/hive_metastore.thrift
A src/kudu/hms/hms_client-test.cc
A src/kudu/hms/hms_client.cc
A src/kudu/hms/hms_client.h
A src/kudu/hms/mini_hms.cc
A src/kudu/hms/mini_hms.h
M thirdparty/LICENSE.txt
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
M thirdparty/download-thirdparty.sh
M thirdparty/vars.sh
17 files changed, 2,911 insertions(+), 5 deletions(-)


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

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

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-18 Thread Adar Dembo (Code Review)

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


Patch Set 24:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/7053/22/src/kudu/hms/CMakeLists.txt
File src/kudu/hms/CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/7053/22/src/kudu/hms/CMakeLists.txt@31
PS22, Line 31:   kudu_common
> Done
Not done? I still see kudu_common in HMS_DEPS.


http://gerrit.cloudera.org:8080/#/c/7053/22/src/kudu/hms/CMakeLists.txt@57
PS22, Line 57:   INCLUDE_JARS ${DEPENDENCY_JARS}
> From the CMake docs:
Oh, add_jar() is smart about tracking dependencies on individual source files? 
Okay then, this makes sense.


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

http://gerrit.cloudera.org:8080/#/c/7053/22/src/kudu/hms/mini_hms.cc@125
PS22, Line 125:   // The schema options allow Hive to startup and run without 
first running the
> Done
Could you also add a comment for hive.metastore.event.db.listener.timetolive? 
The rest are somewhat self-explanatory, I guess.


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

http://gerrit.cloudera.org:8080/#/c/7053/24/src/kudu/hms/mini_hms.cc@125
PS24, Line 125: The schema options
Which options is this referring to? datanucleus.schema.autoCreateAll and 
hive.metastore.schema.verification? Could you call them out specifically to 
make this more clear?


http://gerrit.cloudera.org:8080/#/c/7053/22/thirdparty/build-definitions.sh
File thirdparty/build-definitions.sh:

http://gerrit.cloudera.org:8080/#/c/7053/22/thirdparty/build-definitions.sh@730
PS22, Line 730:   # Normally we would only remove cached CMake files, but the 
Thrift build is
> Comment added.  This was changed in R17 as a result of failed builds on CI.
OK, but every single rebuild of thirdparty is going to have to rebuild thrift 
as a result. How long does thrift take to build with a cold ccache?


http://gerrit.cloudera.org:8080/#/c/7053/22/thirdparty/vars.sh
File thirdparty/vars.sh:

http://gerrit.cloudera.org:8080/#/c/7053/22/thirdparty/vars.sh@212
PS22, Line 212: # TODO(dan): bump to a release version once HIVE-17747 is 
published.
> Yes it was sourced from the apache hive git repository.  We should not be d
OK but could you at least leave a breadcrumb here pointing to the git 
repository? Would build instructions ala SPARSEHASH or SPARSEPP work?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 24
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 18 Oct 2017 21:34:31 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-17 Thread Dan Burkert (Code Review)

Dan Burkert has removed a vote on this change.

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 24
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-17 Thread Dan Burkert (Code Review)

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


Patch Set 24: Code-Review+1

I can't make heads or tails of the remaining IWYU suggestions.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 24
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 18 Oct 2017 02:00:16 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-17 Thread Dan Burkert (Code Review)

Dan Burkert has removed Tidy Bot from this change.  ( 
http://gerrit.cloudera.org:8080/7053 )

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


Removed reviewer Tidy Bot.
--
To view, visit http://gerrit.cloudera.org:8080/7053
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 24
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-17 Thread Dan Burkert (Code Review)

Hello Tidy Bot, Alexey Serbin, Jean-Daniel Cryans, Kudu Jenkins, Adar Dembo, 
Todd Lipcon,

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

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

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..

KUDU-2191 (2/n): Hive Metastore client

This patch lays the groundwork for integrating the Kudu catalog with the
Hive MetaStore.

The focus of this patch is a Kudu-specific C++ HMS client
(hms_client.[h|cc]) in a new hms module. This client provides bindings
for the Hive Metastore APIs that Kudu will use in follow-up commits.

- Thrift has been added as a dependency, and a mechanism for performing
  Thrift codegen at compile time has been added (see FindThrift.cmake,
  based on FindProtobuf.cmake)

- Bison has been added as a build-time dependency, because the system
  bison version on RHEL 6 is not sufficiently new enough for Thrift 0.10.

- Hive and Hadoop have been added to thirdparty as test-only dependencies.

- A Hive MetaStore external mini server is included for testing. See
  mini_hms.[h|cc].

- The Kudu Metastore plugin is compiled from CMake as a standalone jar
  for loading into the HMS mini server.

Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
---
M CMakeLists.txt
M build-support/dist_test.py
M build-support/run_dist_test.py
A cmake_modules/FindJavaHome.cmake
A cmake_modules/FindThrift.cmake
A src/kudu/hms/CMakeLists.txt
A src/kudu/hms/hive_metastore.thrift
A src/kudu/hms/hms_client-test.cc
A src/kudu/hms/hms_client.cc
A src/kudu/hms/hms_client.h
A src/kudu/hms/mini_hms.cc
A src/kudu/hms/mini_hms.h
M thirdparty/LICENSE.txt
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
M thirdparty/download-thirdparty.sh
M thirdparty/vars.sh
17 files changed, 2,914 insertions(+), 5 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 24
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-17 Thread Dan Burkert (Code Review)

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


Patch Set 23:

(17 comments)

http://gerrit.cloudera.org:8080/#/c/7053/22/CMakeLists.txt
File CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/7053/22/CMakeLists.txt@870
PS22, Line 870:   find_package(Java 1.7 REQUIRED)
> Could you add a comment that the version argument is >=, not == ?
Done


http://gerrit.cloudera.org:8080/#/c/7053/22/CMakeLists.txt@871
PS22, Line 871:   # Defines the add_jar() CMake command.
> Can you add a comment saying that this allows us to call add_jar()?
Done


http://gerrit.cloudera.org:8080/#/c/7053/22/cmake_modules/FindJavaHome.cmake
File cmake_modules/FindJavaHome.cmake:

http://gerrit.cloudera.org:8080/#/c/7053/22/cmake_modules/FindJavaHome.cmake@74
PS22, Line 74: set(JAVA_HOME_FOUND true)
> Nit: I think it'd be clearer if this were inverted. That is, if this were u
Done


http://gerrit.cloudera.org:8080/#/c/7053/22/src/kudu/hms/CMakeLists.txt
File src/kudu/hms/CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/7053/22/src/kudu/hms/CMakeLists.txt@31
PS22, Line 31:   kudu_common
> Why is this needed?
Done


http://gerrit.cloudera.org:8080/#/c/7053/22/src/kudu/hms/CMakeLists.txt@57
PS22, Line 57:   INCLUDE_JARS ${DEPENDENCY_JARS}
>From the CMake docs:

> We do not recommend using GLOB to collect a list of source files from your 
> source tree. If no CMakeLists.txt file changes when a source is added or 
> removed then the generated build system cannot know when to ask CMake to 
> regenerate.


http://gerrit.cloudera.org:8080/#/c/7053/22/src/kudu/hms/CMakeLists.txt@59
PS22, Line 59:
> What does EXECUTABLE_OUTPUT_PATH resolve to? Where does the JAR end up?
in build/latest/bin/


http://gerrit.cloudera.org:8080/#/c/7053/22/src/kudu/hms/CMakeLists.txt@64
PS22, Line 64: target_link_libraries(mini_hms
> If this is to be used by the minicluster CLI tool, it can't be conditioned
Done


http://gerrit.cloudera.org:8080/#/c/7053/22/src/kudu/hms/CMakeLists.txt@73
PS22, Line 73: kudu_hms
> FWIW, 'hms' and 'mini-hms' side-by-side suggest that both are metastore imp
renamed to 'kudu_hms' to match the rest of our namespace -> lib name mappings.


http://gerrit.cloudera.org:8080/#/c/7053/22/src/kudu/hms/hive_metastore.thrift
File src/kudu/hms/hive_metastore.thrift:

http://gerrit.cloudera.org:8080/#/c/7053/22/src/kudu/hms/hive_metastore.thrift@22
PS22, Line 22: # 
https://raw.githubusercontent.com/apache/hive/rel/release-2.3.0/metastore/if/hive_metastore.thrift
> Could you add a note in vars.sh next to Hive that if we bump the version, w
Done


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

http://gerrit.cloudera.org:8080/#/c/7053/22/src/kudu/hms/mini_hms.cc@125
PS22, Line 125:   // The schema options allow Hive to startup and run without 
first running the
> Would be nice to sprinkle comments here explaining the choice for each non-
Done


http://gerrit.cloudera.org:8080/#/c/7053/22/thirdparty/LICENSE.txt
File thirdparty/LICENSE.txt:

PS22:
> What about hadoop and hive?
Done


http://gerrit.cloudera.org:8080/#/c/7053/22/thirdparty/build-definitions.sh
File thirdparty/build-definitions.sh:

http://gerrit.cloudera.org:8080/#/c/7053/22/thirdparty/build-definitions.sh@726
PS22, Line 726: build_thrift() {
> Should mention that this depends on bison. See build_glog for inspiration.
Done


http://gerrit.cloudera.org:8080/#/c/7053/22/thirdparty/build-definitions.sh@730
PS22, Line 730:   # Normally we would only remove cached CMake files, but the 
Thrift build is
> Why is this necessary? None of our other dependency builds do this.
Comment added.  This was changed in R17 as a result of failed builds on CI.  
The logs are gone and I don't recall exactly what the symptoms were.


http://gerrit.cloudera.org:8080/#/c/7053/22/thirdparty/build-definitions.sh@737
PS22, Line 737:   # Thrift requires C++11 when compiled on Linux against libc++ 
(see cxxfunctional.h).
> Add EXTRA_CMAKE_FLAGS and then use ${NINJA:-make} instead of make.
Done


http://gerrit.cloudera.org:8080/#/c/7053/22/thirdparty/download-thirdparty.sh
File thirdparty/download-thirdparty.sh:

http://gerrit.cloudera.org:8080/#/c/7053/22/thirdparty/download-thirdparty.sh@343
PS22, Line 343: THRIFT_PATCHLEVEL=0
> Apparently we should now be setting PATCHLEVEL=0 on new deps to ease future
Done


http://gerrit.cloudera.org:8080/#/c/7053/22/thirdparty/download-thirdparty.sh@349
PS22, Line 349: fi
  :
> Can you add a little more detail? Do you remember which platform failed?
Done


http://gerrit.cloudera.org:8080/#/c/7053/22/thirdparty/vars.sh
File thirdparty/vars.sh:

http://gerrit.cloudera.org:8080/#/c/7053/22/thirdparty/vars.sh@212
PS22, Line 212: # TODO(dan): bump to a release version once HIVE-17747 is 
published.
> How was this built? Is it sourced

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-17 Thread Dan Burkert (Code Review)

Hello Tidy Bot, Alexey Serbin, Jean-Daniel Cryans, Kudu Jenkins, Adar Dembo, 
Todd Lipcon,

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

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

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..

KUDU-2191 (2/n): Hive Metastore client

This patch lays the groundwork for integrating the Kudu catalog with the
Hive MetaStore.

The focus of this patch is a Kudu-specific C++ HMS client
(hms_client.[h|cc]) in a new hms module. This client provides bindings
for the Hive Metastore APIs that Kudu will use in follow-up commits.

- Thrift has been added as a dependency, and a mechanism for performing
  Thrift codegen at compile time has been added (see FindThrift.cmake,
  based on FindProtobuf.cmake)

- Bison has been added as a build-time dependency, because the system
  bison version on RHEL 6 is not sufficiently new enough for Thrift 0.10.

- Hive and Hadoop have been added to thirdparty as test-only dependencies.

- A Hive MetaStore external mini server is included for testing. See
  mini_hms.[h|cc].

- The Kudu Metastore plugin is compiled from CMake as a standalone jar
  for loading into the HMS mini server.

Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
---
M CMakeLists.txt
M build-support/dist_test.py
M build-support/run_dist_test.py
A cmake_modules/FindJavaHome.cmake
A cmake_modules/FindThrift.cmake
A src/kudu/hms/CMakeLists.txt
A src/kudu/hms/hive_metastore.thrift
A src/kudu/hms/hms_client-test.cc
A src/kudu/hms/hms_client.cc
A src/kudu/hms/hms_client.h
A src/kudu/hms/mini_hms.cc
A src/kudu/hms/mini_hms.h
M thirdparty/LICENSE.txt
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
M thirdparty/download-thirdparty.sh
M thirdparty/vars.sh
17 files changed, 2,917 insertions(+), 5 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 23
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-17 Thread Adar Dembo (Code Review)

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


Patch Set 22:

(17 comments)

I focused mostly on the glue, since I think I (and Todd) already reviewed the 
client.

http://gerrit.cloudera.org:8080/#/c/7053/22/CMakeLists.txt
File CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/7053/22/CMakeLists.txt@870
PS22, Line 870:   find_package(Java 1.7 REQUIRED)
Could you add a comment that the version argument is >=, not == ?


http://gerrit.cloudera.org:8080/#/c/7053/22/CMakeLists.txt@871
PS22, Line 871:   include(UseJava)
Can you add a comment saying that this allows us to call add_jar()?


http://gerrit.cloudera.org:8080/#/c/7053/22/cmake_modules/FindJavaHome.cmake
File cmake_modules/FindJavaHome.cmake:

http://gerrit.cloudera.org:8080/#/c/7053/22/cmake_modules/FindJavaHome.cmake@74
PS22, Line 74:   # Use the 'java_home' finder on macOS.
Nit: I think it'd be clearer if this were inverted. That is, if this were under 
a if (APPLE) case.


http://gerrit.cloudera.org:8080/#/c/7053/22/src/kudu/hms/CMakeLists.txt
File src/kudu/hms/CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/7053/22/src/kudu/hms/CMakeLists.txt@31
PS22, Line 31:   kudu_common
Why is this needed?


http://gerrit.cloudera.org:8080/#/c/7053/22/src/kudu/hms/CMakeLists.txt@57
PS22, Line 57: 
"${CMAKE_SOURCE_DIR}/java/kudu-hive/src/main/java/org/apache/kudu/hive/metastore/KuduMetastorePlugin.java"
Can you use SOURCES and then glob this directory?


http://gerrit.cloudera.org:8080/#/c/7053/22/src/kudu/hms/CMakeLists.txt@59
PS22, Line 59: OUTPUT_DIR "${EXECUTABLE_OUTPUT_PATH}")
What does EXECUTABLE_OUTPUT_PATH resolve to? Where does the JAR end up?


http://gerrit.cloudera.org:8080/#/c/7053/22/src/kudu/hms/CMakeLists.txt@64
PS22, Line 64:   add_library(mini_hms ${MINI_HMS_SRCS})
If this is to be used by the minicluster CLI tool, it can't be conditioned on 
NOT NO_TESTS.


http://gerrit.cloudera.org:8080/#/c/7053/22/src/kudu/hms/CMakeLists.txt@73
PS22, Line 73: hms
FWIW, 'hms' and 'mini-hms' side-by-side suggest that both are metastore 
implementations. Perhaps rename 'hms' to something like 'hms_util' or 
'hms_client'?


http://gerrit.cloudera.org:8080/#/c/7053/22/src/kudu/hms/hive_metastore.thrift
File src/kudu/hms/hive_metastore.thrift:

http://gerrit.cloudera.org:8080/#/c/7053/22/src/kudu/hms/hive_metastore.thrift@22
PS22, Line 22: # 
https://raw.githubusercontent.com/apache/hive/rel/release-2.3.0/metastore/if/hive_metastore.thrift
Could you add a note in vars.sh next to Hive that if we bump the version, we 
ought to replace this file?


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

http://gerrit.cloudera.org:8080/#/c/7053/22/src/kudu/hms/mini_hms.cc@125
PS22, Line 125:   static const string kFileTemplate = R"(
Would be nice to sprinkle comments here explaining the choice for each 
non-default option.


http://gerrit.cloudera.org:8080/#/c/7053/22/thirdparty/LICENSE.txt
File thirdparty/LICENSE.txt:

PS22:
What about hadoop and hive?


http://gerrit.cloudera.org:8080/#/c/7053/22/thirdparty/build-definitions.sh
File thirdparty/build-definitions.sh:

http://gerrit.cloudera.org:8080/#/c/7053/22/thirdparty/build-definitions.sh@726
PS22, Line 726: build_thrift() {
Should mention that this depends on bison. See build_glog for inspiration.


http://gerrit.cloudera.org:8080/#/c/7053/22/thirdparty/build-definitions.sh@730
PS22, Line 730:   rm -Rf *
Why is this necessary? None of our other dependency builds do this.


http://gerrit.cloudera.org:8080/#/c/7053/22/thirdparty/build-definitions.sh@737
PS22, Line 737: cmake \
Add EXTRA_CMAKE_FLAGS and then use ${NINJA:-make} instead of make.


http://gerrit.cloudera.org:8080/#/c/7053/22/thirdparty/download-thirdparty.sh
File thirdparty/download-thirdparty.sh:

http://gerrit.cloudera.org:8080/#/c/7053/22/thirdparty/download-thirdparty.sh@343
PS22, Line 343: if [ ! -d "$THRIFT_SOURCE" ]; then
Apparently we should now be setting PATCHLEVEL=0 on new deps to ease future 
patches.


http://gerrit.cloudera.org:8080/#/c/7053/22/thirdparty/download-thirdparty.sh@349
PS22, Line 349:   # This would normally call autoreconf, but it does not 
succeed on every
  :   # platform.
Can you add a little more detail? Do you remember which platform failed?


http://gerrit.cloudera.org:8080/#/c/7053/22/thirdparty/vars.sh
File thirdparty/vars.sh:

http://gerrit.cloudera.org:8080/#/c/7053/22/thirdparty/vars.sh@212
PS22, Line 212: HIVE_VERSION=3fb4649fa847cfec33f701f6c884f12087680cf0
How was this built? Is it sourced from github? Could you include instructions?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id:

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-17 Thread Dan Burkert (Code Review)

Hello Tidy Bot, Alexey Serbin, Jean-Daniel Cryans, Kudu Jenkins, Adar Dembo, 
Todd Lipcon,

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

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

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..

KUDU-2191 (2/n): Hive Metastore client

This patch lays the groundwork for integrating the Kudu catalog with the
Hive MetaStore.

The focus of this patch is a Kudu-specific C++ HMS client
(hms_client.[h|cc]) in a new hms module. This client provides bindings
for the Hive Metastore APIs that Kudu will use in follow-up commits.

- Thrift has been added as a dependency, and a mechanism for performing
  Thrift codegen at compile time has been added (see FindThrift.cmake,
  based on FindProtobuf.cmake)

- Bison has been added as a build-time dependency, because the system
  bison version on RHEL 6 is not sufficiently new enough for Thrift 0.10.

- Hive and Hadoop have been added to thirdparty as test-only dependencies.

- A Hive MetaStore external mini server is included for testing. See
  mini_hms.[h|cc].

- The Kudu Metastore plugin is compiled from CMake as a standalone jar
  for loading into the HMS mini server.

Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
---
M CMakeLists.txt
M build-support/dist_test.py
M build-support/run_dist_test.py
A cmake_modules/FindJavaHome.cmake
A cmake_modules/FindThrift.cmake
A src/kudu/hms/CMakeLists.txt
A src/kudu/hms/hive_metastore.thrift
A src/kudu/hms/hms_client-test.cc
A src/kudu/hms/hms_client.cc
A src/kudu/hms/hms_client.h
A src/kudu/hms/mini_hms.cc
A src/kudu/hms/mini_hms.h
M thirdparty/LICENSE.txt
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
M thirdparty/download-thirdparty.sh
M thirdparty/vars.sh
17 files changed, 2,881 insertions(+), 5 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 22
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-17 Thread Dan Burkert (Code Review)

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


Patch Set 19:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7053/21/src/kudu/hms/CMakeLists.txt
File src/kudu/hms/CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/7053/21/src/kudu/hms/CMakeLists.txt@38
PS21, Line 38: # hms-test
this should be mini_hms


http://gerrit.cloudera.org:8080/#/c/7053/19/src/kudu/hms/hms_client.cc
File src/kudu/hms/hms_client.cc:

http://gerrit.cloudera.org:8080/#/c/7053/19/src/kudu/hms/hms_client.cc@34
PS19, Line 34: #include "kudu/hms/hive_metastore_constants.h"
> warning: #includes are not sorted properly [llvm-include-order]
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 19
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 17 Oct 2017 20:36:15 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

2017-10-17 Thread Dan Burkert (Code Review)

Hello Tidy Bot, Alexey Serbin, Jean-Daniel Cryans, Kudu Jenkins, Adar Dembo, 
Todd Lipcon,

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

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

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..

KUDU-2191 (2/n): Hive Metastore client

This patch lays the groundwork for integrating the Kudu catalog with the
Hive MetaStore.

The focus of this patch is a Kudu-specific C++ HMS client
(hms_client.[h|cc]) in a new hms module. This client provides bindings
for the Hive Metastore APIs that Kudu will use in follow-up commits.

- Thrift has been added as a dependency, and a mechanism for performing
  Thrift codegen at compile time has been added (see FindThrift.cmake,
  based on FindProtobuf.cmake)

- Bison has been added as a build-time dependency, because the system
  bison version on RHEL 6 is not sufficiently new enough for Thrift 0.10.

- Hive and Hadoop have been added to thirdparty as test-only dependencies.

- A Hive MetaStore external mini server is included for testing. See
  mini_hms.[h|cc].

- The Kudu Metastore plugin is compiled from CMake as a standalone jar
  for loading into the HMS mini server.

Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
---
M CMakeLists.txt
M build-support/dist_test.py
M build-support/run_dist_test.py
A cmake_modules/FindJavaHome.cmake
A cmake_modules/FindThrift.cmake
A src/kudu/hms/CMakeLists.txt
A src/kudu/hms/hive_metastore.thrift
A src/kudu/hms/hms_client-test.cc
A src/kudu/hms/hms_client.cc
A src/kudu/hms/hms_client.h
A src/kudu/hms/mini_hms.cc
A src/kudu/hms/mini_hms.h
M thirdparty/LICENSE.txt
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
M thirdparty/download-thirdparty.sh
M thirdparty/vars.sh
17 files changed, 2,881 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/7053/21
--
To view, visit http://gerrit.cloudera.org:8080/7053
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 21
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

57 matches

Mail list logo