You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Thomas Marshall (Code Review)" <ge...@cloudera.org> on 2018/08/30 20:07:45 UTC
[Impala-ASF-CR] IMPALA-7499: build against CDH Kudu
Thomas Marshall has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11363
Change subject: IMPALA-7499: build against CDH Kudu
......................................................................
IMPALA-7499: build against CDH Kudu
This patch transitions from pulling in Kudu (libkudu_client.so and the
minicluster tarballs) from the toolchain to instead pull Kudu in with
the other CDH components.
For OSes where the CDH binaries are not provided but the toolchain
binaries are (only Ubuntu 14), we set USE_CDH_KUDU to false to
continue to download the toolchain binaries. We also continue
to use the toolchain binaries to build the client stub for OSes
where KUDU_IS_SUPPORTED is false.
This patch also fixes an issue in bootstrap_toolchain.py where we were
using the wrong g++ to compile the Kudu stub.
Testing:
- Verified building and running Impala works as expected for supported
combinations of KUDU_IS_SUPPORTED/USE_CDH_KUDU
Change-Id: If6e1048438b6d09a1b38c58371d6212bb6dcc06c
---
M CMakeLists.txt
M bin/bootstrap_toolchain.py
M bin/impala-config.sh
M infra/python/bootstrap_virtualenv.py
M testdata/cluster/node_templates/common/etc/init.d/kudu-common
5 files changed, 165 insertions(+), 101 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/63/11363/1
--
To view, visit http://gerrit.cloudera.org:8080/11363
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: If6e1048438b6d09a1b38c58371d6212bb6dcc06c
Gerrit-Change-Number: 11363
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Marshall <th...@cmu.edu>
[Impala-ASF-CR] IMPALA-7499: build against CDH Kudu
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11363 )
Change subject: IMPALA-7499: build against CDH Kudu
......................................................................
Patch Set 4:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3141/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/11363
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If6e1048438b6d09a1b38c58371d6212bb6dcc06c
Gerrit-Change-Number: 11363
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Mon, 10 Sep 2018 21:48:41 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7499: build against CDH Kudu
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11363 )
Change subject: IMPALA-7499: build against CDH Kudu
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11363
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If6e1048438b6d09a1b38c58371d6212bb6dcc06c
Gerrit-Change-Number: 11363
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Mon, 10 Sep 2018 21:48:40 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7499: build against CDH Kudu
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11363 )
Change subject: IMPALA-7499: build against CDH Kudu
......................................................................
Patch Set 3:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3120/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/11363
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If6e1048438b6d09a1b38c58371d6212bb6dcc06c
Gerrit-Change-Number: 11363
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Thu, 06 Sep 2018 00:46:27 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7499: build against CDH Kudu
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11363 )
Change subject: IMPALA-7499: build against CDH Kudu
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/11363
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If6e1048438b6d09a1b38c58371d6212bb6dcc06c
Gerrit-Change-Number: 11363
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Thu, 06 Sep 2018 04:09:35 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7499: build against CDH Kudu
Posted by "Thomas Marshall (Code Review)" <ge...@cloudera.org>.
Hello Philip Zeyliger, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11363
to look at the new patch set (#2).
Change subject: IMPALA-7499: build against CDH Kudu
......................................................................
IMPALA-7499: build against CDH Kudu
This patch transitions from pulling in Kudu (libkudu_client.so and the
minicluster tarballs) from the toolchain to instead pull Kudu in with
the other CDH components.
For OSes where the CDH binaries are not provided but the toolchain
binaries are (only Ubuntu 14), we set USE_CDH_KUDU to false to
continue to download the toolchain binaries. We also continue
to use the toolchain binaries to build the client stub for OSes
where KUDU_IS_SUPPORTED is false.
This patch also fixes an issue in bootstrap_toolchain.py where we were
using the wrong g++ to compile the Kudu stub.
Testing:
- Verified building and running Impala works as expected for supported
combinations of KUDU_IS_SUPPORTED/USE_CDH_KUDU
Change-Id: If6e1048438b6d09a1b38c58371d6212bb6dcc06c
---
M CMakeLists.txt
M bin/bootstrap_toolchain.py
M bin/impala-config.sh
M infra/python/bootstrap_virtualenv.py
M testdata/cluster/node_templates/common/etc/init.d/kudu-common
5 files changed, 151 insertions(+), 102 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/63/11363/2
--
To view, visit http://gerrit.cloudera.org:8080/11363
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If6e1048438b6d09a1b38c58371d6212bb6dcc06c
Gerrit-Change-Number: 11363
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
[Impala-ASF-CR] IMPALA-7499: build against CDH Kudu
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11363 )
Change subject: IMPALA-7499: build against CDH Kudu
......................................................................
Patch Set 2:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/544/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/11363
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If6e1048438b6d09a1b38c58371d6212bb6dcc06c
Gerrit-Change-Number: 11363
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Thu, 30 Aug 2018 23:25:57 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7499: build against CDH Kudu
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11363 )
Change subject: IMPALA-7499: build against CDH Kudu
......................................................................
Patch Set 1:
(16 comments)
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py
File bin/bootstrap_toolchain.py:
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@72
PS1, Line 72:
flake8: E203 whitespace before ':'
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@73
PS1, Line 73:
flake8: E203 whitespace before ':'
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@74
PS1, Line 74:
flake8: E203 whitespace before ':'
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@74
PS1, Line 74:
flake8: E241 multiple spaces after ':'
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@75
PS1, Line 75:
flake8: E203 whitespace before ':'
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@75
PS1, Line 75:
flake8: E241 multiple spaces after ':'
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@76
PS1, Line 76:
flake8: E203 whitespace before ':'
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@79
PS1, Line 79:
flake8: E203 whitespace before ':'
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@106
PS1, Line 106: lsb_release_cache = None
flake8: E305 expected 2 blank lines after class or function definition, found 1
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@107
PS1, Line 107: def get_os_version():
flake8: E302 expected 2 blank lines, found 0
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@121
PS1, Line 121: def try_get_toolchain_platform_label():
flake8: E302 expected 2 blank lines, found 1
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@129
PS1, Line 129: def try_get_cdh_platform_label():
flake8: E302 expected 2 blank lines, found 1
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@134
PS1, Line 134: e
flake8: E722 do not use bare except'
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@137
PS1, Line 137: def get_toolchain_platform_label(release=None):
flake8: E302 expected 2 blank lines, found 1
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@150
PS1, Line 150: def get_cdh_platform_label():
flake8: E302 expected 2 blank lines, found 1
http://gerrit.cloudera.org:8080/#/c/11363/1/infra/python/bootstrap_virtualenv.py
File infra/python/bootstrap_virtualenv.py:
http://gerrit.cloudera.org:8080/#/c/11363/1/infra/python/bootstrap_virtualenv.py@277
PS1, Line 277: k
flake8: F821 undefined name 'kudu_base_dir'
--
To view, visit http://gerrit.cloudera.org:8080/11363
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If6e1048438b6d09a1b38c58371d6212bb6dcc06c
Gerrit-Change-Number: 11363
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 30 Aug 2018 20:11:56 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7499: build against CDH Kudu
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11363 )
Change subject: IMPALA-7499: build against CDH Kudu
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/582/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/11363
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If6e1048438b6d09a1b38c58371d6212bb6dcc06c
Gerrit-Change-Number: 11363
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Wed, 05 Sep 2018 03:19:26 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7499: build against CDH Kudu
Posted by "Thomas Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/11363 )
Change subject: IMPALA-7499: build against CDH Kudu
......................................................................
Patch Set 2:
(22 comments)
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py
File bin/bootstrap_toolchain.py:
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@19
PS1, Line 19: # The purpose of this script is to download prebuilt binaries and jar files to satisfy the
: # third-party dependencies for Impala. The script checks for the presence of IMPALA_HOME
: # and IMPALA_TOOLCHAIN. IMPALA_HOME indicates that the environment is correctly setup and
: # that we can deduce the version settings of the dependencies from the environment.
: # IMPALA_TOOLCHAIN indicates the location where the prebuilt artifacts should be extracted
: # to. If DOWNLOAD_CDH_COMPONENTS is set to true, this script will also download and
: # extract the CDH components (i.e. Hadoop, Hive, HBase and Sentry) into
: # CDH_COMPONENTS_HOME.
> Could you update this to include KUDU_IS_SUPPORTED and USE_CDH_KUDU as they
Done
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@71
PS1, Line 71: OsMapping("suselinux11", "ec2-package-sles-11", None),
> add a doc here about what this maps to and from?
Done
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@72
PS1, Line 72: (
> flake8: E203 whitespace before ':'
Done
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@73
PS1, Line 73: (
> flake8: E203 whitespace before ':'
Done
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@74
PS1, Line 74: "
> flake8: E203 whitespace before ':'
Done
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@74
PS1, Line 74: c
> flake8: E241 multiple spaces after ':'
Done
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@75
PS1, Line 75: c
> flake8: E241 multiple spaces after ':'
Done
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@75
PS1, Line 75: "
> flake8: E203 whitespace before ':'
Done
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@76
PS1, Line 76: (
> flake8: E203 whitespace before ':'
Done
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@79
PS1, Line 79:
> flake8: E203 whitespace before ':'
Done
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@106
PS1, Line 106: 'toolchain' and 'cdh' labels. Return None if not found.
> flake8: E305 expected 2 blank lines after class or function definition, fou
Done
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@107
PS1, Line 107: """
> Please add pydoc here?
Done
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@107
PS1, Line 107: """
> flake8: E302 expected 2 blank lines, found 0
Done
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@121
PS1, Line 121: 'release' can be provided to override the underlying OS version.
> flake8: E302 expected 2 blank lines, found 1
Done
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@129
PS1, Line 129: # Only need to check against the major release if RHEL or CentOS
> flake8: E302 expected 2 blank lines, found 1
Done
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@134
PS1, Line 134:
> flake8: E722 do not use bare except'
Done
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@137
PS1, Line 137: return mapping
> flake8: E302 expected 2 blank lines, found 1
Done
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@150
PS1, Line 150: try:
> flake8: E302 expected 2 blank lines, found 1
Done
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@154
PS1, Line 154: if attempt == NUM_ATTEMPTS:
: raise
: logging.error("Download failed; retrying after sleep: " + str(e))
: time.sle
> This code is identical to lines 141-145. I think it probably makes sense to
Done
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@475
PS1, Line 475:
> This is just: if ...: packages += Package["kudu"]
Done
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@501
PS1, Line 501:
> This is just:
Done
http://gerrit.cloudera.org:8080/#/c/11363/1/infra/python/bootstrap_virtualenv.py
File infra/python/bootstrap_virtualenv.py:
http://gerrit.cloudera.org:8080/#/c/11363/1/infra/python/bootstrap_virtualenv.py@277
PS1, Line 277:
> flake8: F821 undefined name 'kudu_base_dir'
Done
--
To view, visit http://gerrit.cloudera.org:8080/11363
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If6e1048438b6d09a1b38c58371d6212bb6dcc06c
Gerrit-Change-Number: 11363
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Thu, 30 Aug 2018 22:40:37 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7499: build against CDH Kudu
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11363 )
Change subject: IMPALA-7499: build against CDH Kudu
......................................................................
Patch Set 4: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/11363
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If6e1048438b6d09a1b38c58371d6212bb6dcc06c
Gerrit-Change-Number: 11363
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Tue, 11 Sep 2018 01:01:04 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7499: build against CDH Kudu
Posted by "Philip Zeyliger (Code Review)" <ge...@cloudera.org>.
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/11363 )
Change subject: IMPALA-7499: build against CDH Kudu
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11363
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If6e1048438b6d09a1b38c58371d6212bb6dcc06c
Gerrit-Change-Number: 11363
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Mon, 10 Sep 2018 21:46:41 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7499: build against CDH Kudu
Posted by "Philip Zeyliger (Code Review)" <ge...@cloudera.org>.
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/11363 )
Change subject: IMPALA-7499: build against CDH Kudu
......................................................................
Patch Set 1:
(6 comments)
Thanks! Looks all reasonable. See comments below.
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py
File bin/bootstrap_toolchain.py:
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@19
PS1, Line 19: # The purpose of this script is to download prebuilt binaries and jar files to satisfy the
: # third-party dependencies for Impala. The script checks for the presence of IMPALA_HOME
: # and IMPALA_TOOLCHAIN. IMPALA_HOME indicates that the environment is correctly setup and
: # that we can deduce the version settings of the dependencies from the environment.
: # IMPALA_TOOLCHAIN indicates the location where the prebuilt artifacts should be extracted
: # to. If DOWNLOAD_CDH_COMPONENTS is set to true, this script will also download and extract
: # the CDH components (i.e. Hadoop, Hive, HBase and Sentry) into
: # CDH_COMPONENTS_HOME.
Could you update this to include KUDU_IS_SUPPORTED and USE_CDH_KUDU as they're now inputs to this script.
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@71
PS1, Line 71: CDH_OS_MAPPING = {
add a doc here about what this maps to and from?
The keys here (and in TOOLCHAIN_OS_MAPPING) are munged versions of "lsb_release -irs". You could choose to make the value a namedtuple (or other object) that includes both "toolchain" and "cdh" values, and make it a little bit more obvious that the two mappings have to be consistent.
I insist on the doc. I don't insist on merging the maps.
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@107
PS1, Line 107: def get_os_version():
> flake8: E302 expected 2 blank lines, found 0
Please add pydoc here?
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@154
PS1, Line 154: release = get_os_version()
: for k, v in CDH_OS_MAPPING.iteritems():
: if re.search(k, release):
: return v
This code is identical to lines 141-145. I think it probably makes sense to write a helper function that manages that.
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@475
PS1, Line 475: if os.getenv("USE_CDH_KUDU") == "false": packages += map(Package, ["kudu"])
This is just: if ...: packages += Package["kudu"]
http://gerrit.cloudera.org:8080/#/c/11363/1/bin/bootstrap_toolchain.py@501
PS1, Line 501: cdh_components += map(Package, ["kudu"])
This is just:
cdh_components += Package(["kudu"])
--
To view, visit http://gerrit.cloudera.org:8080/11363
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If6e1048438b6d09a1b38c58371d6212bb6dcc06c
Gerrit-Change-Number: 11363
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Comment-Date: Thu, 30 Aug 2018 20:31:20 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7499: build against CDH Kudu
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11363 )
Change subject: IMPALA-7499: build against CDH Kudu
......................................................................
IMPALA-7499: build against CDH Kudu
This patch transitions from pulling in Kudu (libkudu_client.so and the
minicluster tarballs) from the toolchain to instead pull Kudu in with
the other CDH components.
For OSes where the CDH binaries are not provided but the toolchain
binaries are (only Ubuntu 14), we set USE_CDH_KUDU to false to
continue to download the toolchain binaries. We also continue
to use the toolchain binaries to build the client stub for OSes
where KUDU_IS_SUPPORTED is false.
This patch also fixes an issue in bootstrap_toolchain.py where we were
using the wrong g++ to compile the Kudu stub.
Testing:
- Verified building and running Impala works as expected for supported
combinations of KUDU_IS_SUPPORTED/USE_CDH_KUDU
Change-Id: If6e1048438b6d09a1b38c58371d6212bb6dcc06c
Reviewed-on: http://gerrit.cloudera.org:8080/11363
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M CMakeLists.txt
M bin/bootstrap_toolchain.py
M bin/impala-config.sh
M infra/python/bootstrap_virtualenv.py
M testdata/cluster/node_templates/common/etc/init.d/kudu-common
5 files changed, 151 insertions(+), 102 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/11363
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: If6e1048438b6d09a1b38c58371d6212bb6dcc06c
Gerrit-Change-Number: 11363
Gerrit-PatchSet: 5
Gerrit-Owner: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
[Impala-ASF-CR] IMPALA-7499: build against CDH Kudu
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11363 )
Change subject: IMPALA-7499: build against CDH Kudu
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/538/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/11363
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If6e1048438b6d09a1b38c58371d6212bb6dcc06c
Gerrit-Change-Number: 11363
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Comment-Date: Thu, 30 Aug 2018 20:51:19 +0000
Gerrit-HasComments: No