You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Mike Percy (Code Review)" <ge...@cloudera.org> on 2018/08/31 23:56:03 UTC
[kudu-CR] mini cluster: Add scripts to build binaries for testing use
Mike Percy has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11377
Change subject: mini cluster: Add scripts to build binaries for testing use
......................................................................
mini cluster: Add scripts to build binaries for testing use
This script will build a jar file containing "relocatable" binaries for
use with a minicluster. The idea is that the resulting jar will be
uploaded to Maven Central whenver there is a release. There should be
one artifact built on Linux (preferably an old OS, such as RHEL 6) and
one on macOS, so that people can run integration tests against Kudu on
either platform.
The other missing pieces on top of this would be a component that
depends on and unpacks this JAR file, and a refactor of the MiniCluster
code to use that resulting bits to start up a Kudu cluster as part of a
Java integration test.
Very basic testing done on both Linux and macOS.
Change-Id: I8b8f90cfe80f6830177bf6ea9e0711eb0d034f75
---
A build-support/build_mini_cluster_binaries.sh
A build-support/relocate_binaries_for_mini_cluster.py
2 files changed, 465 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/11377/1
--
To view, visit http://gerrit.cloudera.org:8080/11377
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8b8f90cfe80f6830177bf6ea9e0711eb0d034f75
Gerrit-Change-Number: 11377
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy <mp...@apache.org>
[kudu-CR] KUDU-2411. Add scripts to build binaries for testing use
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/11377 )
Change subject: KUDU-2411. Add scripts to build binaries for testing use
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/build_mini_cluster_binaries.sh
File build-support/build_mini_cluster_binaries.sh:
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/build_mini_cluster_binaries.sh@53
PS1, Line 53: ding with MacPorts.
> SGTM: I like the idea of adding a TODO note.
Thanks!
--
To view, visit http://gerrit.cloudera.org:8080/11377
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b8f90cfe80f6830177bf6ea9e0711eb0d034f75
Gerrit-Change-Number: 11377
Gerrit-PatchSet: 4
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Fri, 04 Jan 2019 22:21:59 +0000
Gerrit-HasComments: Yes
[kudu-CR] mini cluster: Add scripts to build binaries for testing use
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11377 )
Change subject: mini cluster: Add scripts to build binaries for testing use
......................................................................
Patch Set 1:
(3 comments)
Cool!
Did you try to verify that binaries built on older OS X run on a newer one?
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/build_mini_cluster_binaries.sh
File build-support/build_mini_cluster_binaries.sh:
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/build_mini_cluster_binaries.sh@46
PS1, Line 46: # TODO(mpercy): What's a better way to detect macOS?
Another way of doing it is relying on $OSTYPE, like it's done in build-thirdparty.sh
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/build_mini_cluster_binaries.sh@53
PS1, Line 53: /usr/local/opt/openssl
That makes sense only in case of Homebrew, but it will not work in case of MacPorts or others. To make it generic, it possible to use pkg-config, and have a hard-coded path for Homebrew where pkg-config is broken. An example of how to do that is at:
https://github.com/apache/kudu/blob/master/thirdparty/build-thirdparty.sh#L174
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/relocate_binaries_for_mini_cluster.py
File build-support/relocate_binaries_for_mini_cluster.py:
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/relocate_binaries_for_mini_cluster.py@284
PS1, Line 284: Kerberos|libSystem|libncurses
What about libc++ and others? It's not excluded?
--
To view, visit http://gerrit.cloudera.org:8080/11377
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b8f90cfe80f6830177bf6ea9e0711eb0d034f75
Gerrit-Change-Number: 11377
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 03 Sep 2018 16:34:57 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2411. Add scripts to build binaries for testing use
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11377
to look at the new patch set (#2).
Change subject: KUDU-2411. Add scripts to build binaries for testing use
......................................................................
KUDU-2411. Add scripts to build binaries for testing use
This script will build a jar file containing "relocatable" binaries for
use with a minicluster. The idea is that the resulting jar will be
uploaded to Maven Central whenver there is a release. There should be
one artifact built on Linux (preferably an old OS, such as RHEL 6) and
one on macOS, so that people can run integration tests against Kudu on
either platform.
Other pieces not included in this patch are code to depend on, locate,
and extract the resulting JAR file, as well as a refactor of the
MiniCluster code to use the resulting bits to start up a Kudu cluster as
part of a Java integration test. Some of that work is underway or has
already been merged as part of KUDU-2411.
Very basic testing of this patch has been done on both Linux and macOS.
We will need to do more testing and validation as the integration work
progresses.
Change-Id: I8b8f90cfe80f6830177bf6ea9e0711eb0d034f75
---
A build-support/build_mini_cluster_binaries.sh
A build-support/relocate_binaries_for_mini_cluster.py
2 files changed, 446 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/11377/2
--
To view, visit http://gerrit.cloudera.org:8080/11377
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8b8f90cfe80f6830177bf6ea9e0711eb0d034f75
Gerrit-Change-Number: 11377
Gerrit-PatchSet: 2
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
[kudu-CR] KUDU-2411. Add scripts to build binaries for testing use
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11377 )
Change subject: KUDU-2411. Add scripts to build binaries for testing use
......................................................................
Patch Set 4: Code-Review+1
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/build_mini_cluster_binaries.sh
File build-support/build_mini_cluster_binaries.sh:
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/build_mini_cluster_binaries.sh@53
PS1, Line 53: ding with MacPorts.
> I'm not Alexey but it would be good to at least note this limitation in a T
SGTM: I like the idea of adding a TODO note.
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/relocate_binaries_for_mini_cluster.py
File build-support/relocate_binaries_for_mini_cluster.py:
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/relocate_binaries_for_mini_cluster.py@284
PS1, Line 284: runtime dependencies.
> I added a TODO because I'm not exactly sure either way, but we can always t
SGTM
--
To view, visit http://gerrit.cloudera.org:8080/11377
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b8f90cfe80f6830177bf6ea9e0711eb0d034f75
Gerrit-Change-Number: 11377
Gerrit-PatchSet: 4
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Fri, 04 Jan 2019 22:21:12 +0000
Gerrit-HasComments: Yes
[kudu-CR] mini cluster: Add scripts to build binaries for testing use
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/11377 )
Change subject: mini cluster: Add scripts to build binaries for testing use
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/build_mini_cluster_binaries.sh
File build-support/build_mini_cluster_binaries.sh:
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/build_mini_cluster_binaries.sh@31
PS1, Line 31: TARGETS="kudu-tserver kudu-master"
We also need the kudu target for the minicluster stuff to work.
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/relocate_binaries_for_mini_cluster.py
File build-support/relocate_binaries_for_mini_cluster.py:
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/relocate_binaries_for_mini_cluster.py@55
PS1, Line 55: TARGETS = ["kudu-tserver", "kudu-master"]
These targets are defined it both scripts. One central place would be nice.
--
To view, visit http://gerrit.cloudera.org:8080/11377
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b8f90cfe80f6830177bf6ea9e0711eb0d034f75
Gerrit-Change-Number: 11377
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Thu, 18 Oct 2018 20:07:22 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2411. Add scripts to build binaries for testing use
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11377 )
Change subject: KUDU-2411. Add scripts to build binaries for testing use
......................................................................
Patch Set 3:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/11377/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11377/1//COMMIT_MSG@11
PS1, Line 11: whenver
> whenever
I think you missed this one.
http://gerrit.cloudera.org:8080/#/c/11377/1//COMMIT_MSG@12
PS1, Line 12: preferably an old OS, such as RHEL 6
> I tested building on EL6 and running on Ubuntu 16.04.
Could you note the rationale for dynamic linking over static linking in the commit message?
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/build_mini_cluster_binaries.sh
File build-support/build_mini_cluster_binaries.sh:
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/build_mini_cluster_binaries.sh@53
PS1, Line 53: /usr/local/opt/openssl
> I am only an occasional macOS user and my Mac dev box recently crashed hard
I'm not Alexey but it would be good to at least note this limitation in a TODO if you're going to merge the code as-is.
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/relocate_binaries_for_mini_cluster.py
File build-support/relocate_binaries_for_mini_cluster.py:
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/relocate_binaries_for_mini_cluster.py@35
PS1, Line 35: import sys
> That lib doesn't look like it's part of the default Python distribution and
Agreed; I didn't realize sh wasn't a standard module.
http://gerrit.cloudera.org:8080/#/c/11377/3/build-support/relocate_binaries_for_mini_cluster.py
File build-support/relocate_binaries_for_mini_cluster.py:
http://gerrit.cloudera.org:8080/#/c/11377/3/build-support/relocate_binaries_for_mini_cluster.py@217
PS3, Line 217: # Make sure we have the chrpath command available.
: check_for_command('chrpath')
:
We're going to call chrpath() a lot; no need to do this with each invocation. Seems like this is better done up-front, before we do any real work.
http://gerrit.cloudera.org:8080/#/c/11377/3/build-support/relocate_binaries_for_mini_cluster.py@238
PS3, Line 238: target_src = os.path.join(config[BUILD_BIN_DIR], target)
: target_dst = os.path.join(config[ARTIFACT_BIN_DIR], target)
relocate_deps() already did this; perhaps you could feed in these by argument instead?
http://gerrit.cloudera.org:8080/#/c/11377/3/build-support/relocate_binaries_for_mini_cluster.py@245
PS3, Line 245: PAT_LINUX_LIB_EXCLUDE = re.compile('(libpthread|libc|libstdc\+\+|librt|libdl|libgcc.*)\.so')
This deserves to be globally defined to raise its visibility. Also, since it may evolve over time, it'd be good to put each library on its own line to minimize the diff when the list changes.
--
To view, visit http://gerrit.cloudera.org:8080/11377
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b8f90cfe80f6830177bf6ea9e0711eb0d034f75
Gerrit-Change-Number: 11377
Gerrit-PatchSet: 3
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Fri, 04 Jan 2019 19:11:02 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2411. Add scripts to build binaries for testing use
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11377 )
Change subject: KUDU-2411. Add scripts to build binaries for testing use
......................................................................
KUDU-2411. Add scripts to build binaries for testing use
This script will build a jar file containing "relocatable" binaries for
use with a minicluster. The idea is that the resulting jar will be
uploaded to Maven Central whenever there is a release. There should be
one artifact built on Linux (preferably an old OS, such as RHEL 6) and
one on macOS, so that people can run integration tests against Kudu on
either platform.
The current approach is to dynamically link the dependencies instead of
statically linking them to optimize for the total size of the resulting
test artifact. With kudu-tserver, kudu-master, and kudu admin tool
binaries all linking against the same shared objects, the size of the
jar file to download and unpack is significantly reduced. There are a
bunch of shared objects we have to ship regardless, such as dependencies
from thirdparty and security libraries from the system.
Other pieces not included in this patch are code to depend on, locate,
and extract the resulting JAR file, as well as a refactor of the
MiniCluster code to use the resulting bits to start up a Kudu cluster as
part of a Java integration test. Some of that work is underway or has
already been merged as part of KUDU-2411.
Very basic testing of this patch has been done on both Linux and macOS.
We will need to do more testing and validation as the integration work
progresses.
Change-Id: I8b8f90cfe80f6830177bf6ea9e0711eb0d034f75
Reviewed-on: http://gerrit.cloudera.org:8080/11377
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
---
A build-support/build_mini_cluster_binaries.sh
A build-support/relocate_binaries_for_mini_cluster.py
2 files changed, 470 insertions(+), 0 deletions(-)
Approvals:
Alexey Serbin: Looks good to me, but someone else must approve
Adar Dembo: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/11377
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I8b8f90cfe80f6830177bf6ea9e0711eb0d034f75
Gerrit-Change-Number: 11377
Gerrit-PatchSet: 5
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
[kudu-CR] mini cluster: Add scripts to build binaries for testing use
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11377 )
Change subject: mini cluster: Add scripts to build binaries for testing use
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11377/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11377/1//COMMIT_MSG@12
PS1, Line 12: preferably an old OS, such as RHEL 6
> This makes sense; el6.6 would be the natural choice.
Answering my own question: the build system's openssl is coming along for the ride as it is copied into the archive. That's probably OK as I think by building against a system with a very old glibc, we'll be able to keep using these libraries even when on a system with a new glibc. Have you tested an extreme example, like building on el6.6 and running on Ubuntu 18?
By the way, why not statically link all the Kudu dependencies? Wouldn't that make for a simpler archive (i.e. less moving parts)?
--
To view, visit http://gerrit.cloudera.org:8080/11377
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b8f90cfe80f6830177bf6ea9e0711eb0d034f75
Gerrit-Change-Number: 11377
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Sat, 01 Sep 2018 18:01:20 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2411. Add scripts to build binaries for testing use
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11377
to look at the new patch set (#4).
Change subject: KUDU-2411. Add scripts to build binaries for testing use
......................................................................
KUDU-2411. Add scripts to build binaries for testing use
This script will build a jar file containing "relocatable" binaries for
use with a minicluster. The idea is that the resulting jar will be
uploaded to Maven Central whenever there is a release. There should be
one artifact built on Linux (preferably an old OS, such as RHEL 6) and
one on macOS, so that people can run integration tests against Kudu on
either platform.
The current approach is to dynamically link the dependencies instead of
statically linking them to optimize for the total size of the resulting
test artifact. With kudu-tserver, kudu-master, and kudu admin tool
binaries all linking against the same shared objects, the size of the
jar file to download and unpack is significantly reduced. There are a
bunch of shared objects we have to ship regardless, such as dependencies
from thirdparty and security libraries from the system.
Other pieces not included in this patch are code to depend on, locate,
and extract the resulting JAR file, as well as a refactor of the
MiniCluster code to use the resulting bits to start up a Kudu cluster as
part of a Java integration test. Some of that work is underway or has
already been merged as part of KUDU-2411.
Very basic testing of this patch has been done on both Linux and macOS.
We will need to do more testing and validation as the integration work
progresses.
Change-Id: I8b8f90cfe80f6830177bf6ea9e0711eb0d034f75
---
A build-support/build_mini_cluster_binaries.sh
A build-support/relocate_binaries_for_mini_cluster.py
2 files changed, 470 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/11377/4
--
To view, visit http://gerrit.cloudera.org:8080/11377
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8b8f90cfe80f6830177bf6ea9e0711eb0d034f75
Gerrit-Change-Number: 11377
Gerrit-PatchSet: 4
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
[kudu-CR] KUDU-2411. Add scripts to build binaries for testing use
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11377 )
Change subject: KUDU-2411. Add scripts to build binaries for testing use
......................................................................
Patch Set 4: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11377/3/build-support/relocate_binaries_for_mini_cluster.py
File build-support/relocate_binaries_for_mini_cluster.py:
http://gerrit.cloudera.org:8080/#/c/11377/3/build-support/relocate_binaries_for_mini_cluster.py@217
PS3, Line 217:
: def copy_file(src, dest):
:
> I'm not really concerned about this from a performance perspective because
To me it wasn't so much about performance but about more user-friendly flow of the script (I alluded to this in the second sentence in my suggestion). As a user I like working with scripts that work like this:
1. Check a bunch of stuff up front without mutating any on-disk state, erroring out if something is missing.
2. Do all the work.
This gives me confidence that if I see an error in step 1, none of my on-disk state has changed. In this script, that would mean moving check_for_chrpath() to just before prep_artifact_dirs().
All that said, I appreciate your point about keeping it close to where chrpath is actually used, and I don't feel strongly about it.
--
To view, visit http://gerrit.cloudera.org:8080/11377
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b8f90cfe80f6830177bf6ea9e0711eb0d034f75
Gerrit-Change-Number: 11377
Gerrit-PatchSet: 4
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Fri, 04 Jan 2019 22:31:03 +0000
Gerrit-HasComments: Yes
[kudu-CR] mini cluster: Add scripts to build binaries for testing use
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11377 )
Change subject: mini cluster: Add scripts to build binaries for testing use
......................................................................
Patch Set 1:
(11 comments)
Very cool. Do you intend to use this in the next Kudu release? Would be good to update the releasing document to explain what needs to happen.
http://gerrit.cloudera.org:8080/#/c/11377/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11377/1//COMMIT_MSG@11
PS1, Line 11: whenver
whenever
http://gerrit.cloudera.org:8080/#/c/11377/1//COMMIT_MSG@12
PS1, Line 12: preferably an old OS, such as RHEL 6
This makes sense; el6.6 would be the natural choice.
The gcc5 ABI change isn't an issue here because these are executables, not shared libraries, so you can't link against them with the wrong ABI. But what about openssl, which broke backwards compatibility in a big way between 1.0 and 1.1?
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/build_mini_cluster_binaries.sh
File build-support/build_mini_cluster_binaries.sh:
PS1:
Curious why not do this in Python, and have one large script?
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/build_mini_cluster_binaries.sh@63
PS1, Line 63: bit
What does 'bit' mean in this context?
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/build_mini_cluster_binaries.sh@76
PS1, Line 76: 's/\///'
This will be more readable if you use different symbols for delimiting the pattern, like #.
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/relocate_binaries_for_mini_cluster.py
File build-support/relocate_binaries_for_mini_cluster.py:
PS1:
I think we tend towards 2-spaces per indentation in Kudu python scripts.
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/relocate_binaries_for_mini_cluster.py@35
PS1, Line 35: import subprocess
You might find the 'sh' module to be an easier way of calling small Unix commands: https://amoffat.github.io/sh/
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/relocate_binaries_for_mini_cluster.py@171
PS1, Line 171: def get_dep_library_paths_linux(binary_path):
Can you refactor and reuse ldd_deps() from dist_test.py here?
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/relocate_binaries_for_mini_cluster.py@231
PS1, Line 231: def copy_executable(target, config):
Why not use an shutil function here? Is it because they don't copy all metadata? If so, could you note that?
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/relocate_binaries_for_mini_cluster.py@251
PS1, Line 251: See relocate_deps(). Linux implementation.
This refers to something that doesn't exist?
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/relocate_binaries_for_mini_cluster.py@279
PS1, Line 279: See relocate_deps(). macOS implementation.
Same comment.
--
To view, visit http://gerrit.cloudera.org:8080/11377
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b8f90cfe80f6830177bf6ea9e0711eb0d034f75
Gerrit-Change-Number: 11377
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Sat, 01 Sep 2018 17:59:15 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2411. Add scripts to build binaries for testing use
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/11377 )
Change subject: KUDU-2411. Add scripts to build binaries for testing use
......................................................................
Patch Set 3:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/11377/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11377/1//COMMIT_MSG@11
PS1, Line 11: whenver
> I think you missed this one.
Oops, yep I didn't notice this comment.
http://gerrit.cloudera.org:8080/#/c/11377/1//COMMIT_MSG@12
PS1, Line 12: preferably an old OS, such as RHEL 6
> Could you note the rationale for dynamic linking over static linking in the
Done
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/build_mini_cluster_binaries.sh
File build-support/build_mini_cluster_binaries.sh:
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/build_mini_cluster_binaries.sh@53
PS1, Line 53: /usr/local/opt/openssl
> I'm not Alexey but it would be good to at least note this limitation in a T
Done
http://gerrit.cloudera.org:8080/#/c/11377/3/build-support/relocate_binaries_for_mini_cluster.py
File build-support/relocate_binaries_for_mini_cluster.py:
http://gerrit.cloudera.org:8080/#/c/11377/3/build-support/relocate_binaries_for_mini_cluster.py@217
PS3, Line 217: # Make sure we have the chrpath command available.
: check_for_command('chrpath')
:
> We're going to call chrpath() a lot; no need to do this with each invocatio
I'm not really concerned about this from a performance perspective because this script runs in a couple of seconds, but to reduce the number of unnecessary invocations of `which` I moved this up a level to the Linux-specific call site. I think it's better to keep the check close to the call site in this case because of the lack of a significant performance or fail-fast concern. Let me know if you think there's a larger benefit to putting it at the top of the file behind an if (LINUX) flag.
http://gerrit.cloudera.org:8080/#/c/11377/3/build-support/relocate_binaries_for_mini_cluster.py@238
PS3, Line 238: target_src = os.path.join(config[BUILD_BIN_DIR], target)
: target_dst = os.path.join(config[ARTIFACT_BIN_DIR], target)
> relocate_deps() already did this; perhaps you could feed in these by argume
Done
http://gerrit.cloudera.org:8080/#/c/11377/3/build-support/relocate_binaries_for_mini_cluster.py@245
PS3, Line 245: PAT_LINUX_LIB_EXCLUDE = re.compile('(libpthread|libc|libstdc\+\+|librt|libdl|libgcc.*)\.so')
> This deserves to be globally defined to raise its visibility. Also, since i
I had considered putting it up above but wasn't sure if being more visible was outweighed by being close to the usage. I think it's reasonable to put it at the top, though, so it's one of the first things one sees when opening the file since it may get modified periodically. I also like your idea about making it an extended-syntax regex. Done.
--
To view, visit http://gerrit.cloudera.org:8080/11377
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b8f90cfe80f6830177bf6ea9e0711eb0d034f75
Gerrit-Change-Number: 11377
Gerrit-PatchSet: 3
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Fri, 04 Jan 2019 22:12:57 +0000
Gerrit-HasComments: Yes
[kudu-CR] mini cluster: Add scripts to build binaries for testing use
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/11377 )
Change subject: mini cluster: Add scripts to build binaries for testing use
......................................................................
Patch Set 1:
(14 comments)
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/build_mini_cluster_binaries.sh
File build-support/build_mini_cluster_binaries.sh:
PS1:
> Curious why not do this in Python, and have one large script?
I wouldn't be against porting the whole thing over to Python in the future but would prefer to keep them separated for the initial commit. The reason I wrote this in two languages is because the portion of the build process encoded in this shell script is tersely written and easily understood as a simple series of commands. The process to make the binaries relocatable requires a bunch of regex parsing and path munging, making Python the better choice for expressing that part of it.
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/build_mini_cluster_binaries.sh@31
PS1, Line 31: TARGETS="kudu-tserver kudu-master"
> We also need the kudu target for the minicluster stuff to work.
Done
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/build_mini_cluster_binaries.sh@46
PS1, Line 46: # TODO(mpercy): What's a better way to detect macOS?
> Another way of doing it is relying on $OSTYPE, like it's done in build-thir
My Mac recently crashed, so I'm inclined to leave this as a TODO for the initial commit because it worked the last time I tried it and I don't have a convenient way to experiment with a better way of doing this.
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/build_mini_cluster_binaries.sh@53
PS1, Line 53: /usr/local/opt/openssl
> Thanks for the pointer, I'll take a look at that implementation.
I am only an occasional macOS user and my Mac dev box recently crashed hard (seems like a hardware problem or possibly an alien invasion). Mind if we merge this part as-is and maybe you can help get it working in a more general way later?
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/build_mini_cluster_binaries.sh@63
PS1, Line 63: bit
> What does 'bit' mean in this context?
I meant shared objects... I'll update this.
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/build_mini_cluster_binaries.sh@76
PS1, Line 76: 's/\///'
> This will be more readable if you use different symbols for delimiting the
Done
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/relocate_binaries_for_mini_cluster.py
File build-support/relocate_binaries_for_mini_cluster.py:
PS1:
> I think we tend towards 2-spaces per indentation in Kudu python scripts.
Done
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/relocate_binaries_for_mini_cluster.py@35
PS1, Line 35: import subprocess
> Thanks for the suggestion, I haven't used that lib before. I'll look into i
That lib doesn't look like it's part of the default Python distribution and it would be some hassle to add pip / virtualenv support to the build-support scripts. The benefit in terseness doesn't seem worth those drawbacks at the moment.
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/relocate_binaries_for_mini_cluster.py@55
PS1, Line 55: TARGETS = ["kudu-tserver", "kudu-master"]
> These targets are defined it both scripts. One central place would be nice.
Done, targets are now passed into this script on the command line.
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/relocate_binaries_for_mini_cluster.py@171
PS1, Line 171: def get_dep_library_paths_linux(binary_path):
> I'll take a look at that.
This is done, but only for the Linux implementation at the moment. I'd like to take a look at porting the macOS implementation over later, but my Mac gave up the ghost so I think it's better to merge this tested version for now.
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/relocate_binaries_for_mini_cluster.py@231
PS1, Line 231: def copy_executable(target, config):
> Why not use an shutil function here? Is it because they don't copy all meta
Thanks for the suggestion, done.
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/relocate_binaries_for_mini_cluster.py@251
PS1, Line 251: See relocate_deps(). Linux implementation.
> Ah, should be relocate_target()
I added a relocate_deps() function to delegate to this one.
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/relocate_binaries_for_mini_cluster.py@279
PS1, Line 279: See relocate_deps(). macOS implementation.
> Same comment.
Done
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/relocate_binaries_for_mini_cluster.py@284
PS1, Line 284: Kerberos|libSystem|libncurses
> I thought we might want to include libc++ in case we use our own LLVM versi
I added a TODO because I'm not exactly sure either way, but we can always tweak this list later.
--
To view, visit http://gerrit.cloudera.org:8080/11377
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b8f90cfe80f6830177bf6ea9e0711eb0d034f75
Gerrit-Change-Number: 11377
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Fri, 04 Jan 2019 02:26:24 +0000
Gerrit-HasComments: Yes
[kudu-CR] mini cluster: Add scripts to build binaries for testing use
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/11377 )
Change subject: mini cluster: Add scripts to build binaries for testing use
......................................................................
Patch Set 1:
(6 comments)
> Did you try to verify that binaries built on older OS X run on a newer one?
No, I only have tried to build this way on High Sierra with Homebrew. On the bright side, Mac users tend to upgrade the OS pretty regularly.
http://gerrit.cloudera.org:8080/#/c/11377/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11377/1//COMMIT_MSG@12
PS1, Line 12: preferably an old OS, such as RHEL 6
> Answering my own question: the build system's openssl is coming along for t
Statically linking results in a larger artifact because we have to ship both kudu-master and kudu-tserver
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/build_mini_cluster_binaries.sh
File build-support/build_mini_cluster_binaries.sh:
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/build_mini_cluster_binaries.sh@53
PS1, Line 53: /usr/local/opt/openssl
> That makes sense only in case of Homebrew, but it will not work in case of
Thanks for the pointer, I'll take a look at that implementation.
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/relocate_binaries_for_mini_cluster.py
File build-support/relocate_binaries_for_mini_cluster.py:
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/relocate_binaries_for_mini_cluster.py@35
PS1, Line 35: import subprocess
> You might find the 'sh' module to be an easier way of calling small Unix co
Thanks for the suggestion, I haven't used that lib before. I'll look into it.
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/relocate_binaries_for_mini_cluster.py@171
PS1, Line 171: def get_dep_library_paths_linux(binary_path):
> Can you refactor and reuse ldd_deps() from dist_test.py here?
I'll take a look at that.
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/relocate_binaries_for_mini_cluster.py@251
PS1, Line 251: See relocate_deps(). Linux implementation.
> This refers to something that doesn't exist?
Ah, should be relocate_target()
http://gerrit.cloudera.org:8080/#/c/11377/1/build-support/relocate_binaries_for_mini_cluster.py@284
PS1, Line 284: Kerberos|libSystem|libncurses
> What about libc++ and others? It's not excluded?
I thought we might want to include libc++ in case we use our own LLVM version.
--
To view, visit http://gerrit.cloudera.org:8080/11377
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b8f90cfe80f6830177bf6ea9e0711eb0d034f75
Gerrit-Change-Number: 11377
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Thu, 06 Sep 2018 04:33:07 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2411. Add scripts to build binaries for testing use
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/11377 )
Change subject: KUDU-2411. Add scripts to build binaries for testing use
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11377/3/build-support/relocate_binaries_for_mini_cluster.py
File build-support/relocate_binaries_for_mini_cluster.py:
http://gerrit.cloudera.org:8080/#/c/11377/3/build-support/relocate_binaries_for_mini_cluster.py@217
PS3, Line 217:
: def copy_file(src, dest):
:
> To me it wasn't so much about performance but about more user-friendly flow
Thanks for clarifying your thought process. Your point about erroring out before making any changes on disk is fair, but since in this case that would only entail creating a few output directories it's not very serious. Since you don't feel strongly about it then let's leave it as-is for now, because I'm not motivated enough by the tradeoffs (slight user friendliness improvement vs slight maintainability edge) to feel like it's worth another revision of this patch.
--
To view, visit http://gerrit.cloudera.org:8080/11377
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b8f90cfe80f6830177bf6ea9e0711eb0d034f75
Gerrit-Change-Number: 11377
Gerrit-PatchSet: 4
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Fri, 04 Jan 2019 23:14:05 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2411. Add scripts to build binaries for testing use
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11377
to look at the new patch set (#3).
Change subject: KUDU-2411. Add scripts to build binaries for testing use
......................................................................
KUDU-2411. Add scripts to build binaries for testing use
This script will build a jar file containing "relocatable" binaries for
use with a minicluster. The idea is that the resulting jar will be
uploaded to Maven Central whenver there is a release. There should be
one artifact built on Linux (preferably an old OS, such as RHEL 6) and
one on macOS, so that people can run integration tests against Kudu on
either platform.
Other pieces not included in this patch are code to depend on, locate,
and extract the resulting JAR file, as well as a refactor of the
MiniCluster code to use the resulting bits to start up a Kudu cluster as
part of a Java integration test. Some of that work is underway or has
already been merged as part of KUDU-2411.
Very basic testing of this patch has been done on both Linux and macOS.
We will need to do more testing and validation as the integration work
progresses.
Change-Id: I8b8f90cfe80f6830177bf6ea9e0711eb0d034f75
---
A build-support/build_mini_cluster_binaries.sh
A build-support/relocate_binaries_for_mini_cluster.py
2 files changed, 468 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/11377/3
--
To view, visit http://gerrit.cloudera.org:8080/11377
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8b8f90cfe80f6830177bf6ea9e0711eb0d034f75
Gerrit-Change-Number: 11377
Gerrit-PatchSet: 3
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
[kudu-CR] mini cluster: Add scripts to build binaries for testing use
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/11377 )
Change subject: mini cluster: Add scripts to build binaries for testing use
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11377/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11377/1//COMMIT_MSG@12
PS1, Line 12: preferably an old OS, such as RHEL 6
> Statically linking results in a larger artifact because we have to ship bot
I tested building on EL6 and running on Ubuntu 16.04.
--
To view, visit http://gerrit.cloudera.org:8080/11377
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b8f90cfe80f6830177bf6ea9e0711eb0d034f75
Gerrit-Change-Number: 11377
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Thu, 06 Sep 2018 04:34:08 +0000
Gerrit-HasComments: Yes