You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Zach Amsden (Code Review)" <ge...@cloudera.org> on 2017/07/01 03:46:22 UTC
[Impala-ASF-CR] Complete guide to important environment variables for build, test, and mini-cluster operations.
Zach Amsden has uploaded a new change for review.
http://gerrit.cloudera.org:8080/7350
Change subject: Complete guide to important environment variables for build, test, and mini-cluster operations.
......................................................................
Complete guide to important environment variables for
build, test, and mini-cluster operations.
Change-Id: I16d34cb4fa0c60c5ad6d9c8764cc0ec21c5cb368
---
M bin/impala-config.sh
A docs/Developer-Guide/Build-Environment.txt
2 files changed, 265 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/50/7350/1
--
To view, visit http://gerrit.cloudera.org:8080/7350
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I16d34cb4fa0c60c5ad6d9c8764cc0ec21c5cb368
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
[Impala-ASF-CR] IMPALA-5768: Better developer documentation
Posted by "Zach Amsden (Code Review)" <ge...@cloudera.org>.
Zach Amsden has posted comments on this change.
Change subject: IMPALA-5768: Better developer documentation
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7350/2/bin/impala-config.sh
File bin/impala-config.sh:
Line 456: LIBHDFS_OPTS="${LIBHDFS_OPTS}:${IMPALA_HOME}/be/build/debug/service"
> I agree that it's wrong, and /latest/ is the correct way to point it at tha
I got what appears to be a clean and functional build out of it - pretty sure all this stuff that keeps mentioning libbackend.so is obsolete.
--
To view, visit http://gerrit.cloudera.org:8080/7350
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I16d34cb4fa0c60c5ad6d9c8764cc0ec21c5cb368
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] Guide to important environment variables for build, test, and mini-cluster operations.
Posted by "Zach Amsden (Code Review)" <ge...@cloudera.org>.
Zach Amsden has uploaded a new patch set (#2).
Change subject: Guide to important environment variables for build, test, and mini-cluster operations.
......................................................................
Guide to important environment variables for
build, test, and mini-cluster operations.
Change-Id: I16d34cb4fa0c60c5ad6d9c8764cc0ec21c5cb368
---
M README.md
M bin/impala-config.sh
2 files changed, 72 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/50/7350/2
--
To view, visit http://gerrit.cloudera.org:8080/7350
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I16d34cb4fa0c60c5ad6d9c8764cc0ec21c5cb368
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>
[Impala-ASF-CR] IMPALA-5768: Better developer documentation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-5768: Better developer documentation
......................................................................
Patch Set 6:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1027/
--
To view, visit http://gerrit.cloudera.org:8080/7350
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I16d34cb4fa0c60c5ad6d9c8764cc0ec21c5cb368
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5768: Better developer documentation
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-5768: Better developer documentation
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/7350
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I16d34cb4fa0c60c5ad6d9c8764cc0ec21c5cb368
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] Guide to important environment variables for build, test, and mini-cluster operations.
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: Guide to important environment variables for build, test, and mini-cluster operations.
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/7350
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I16d34cb4fa0c60c5ad6d9c8764cc0ec21c5cb368
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5768: Better developer documentation
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has submitted this change and it was merged.
Change subject: IMPALA-5768: Better developer documentation
......................................................................
IMPALA-5768: Better developer documentation
Guide to important environment variables for
build, impala paths and config cleanup.
Change-Id: I16d34cb4fa0c60c5ad6d9c8764cc0ec21c5cb368
Reviewed-on: http://gerrit.cloudera.org:8080/7350
Tested-by: Impala Public Jenkins
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
---
M README.md
M bin/impala-config.sh
2 files changed, 75 insertions(+), 25 deletions(-)
Approvals:
Impala Public Jenkins: Verified
Tim Armstrong: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/7350
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I16d34cb4fa0c60c5ad6d9c8764cc0ec21c5cb368
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>
[Impala-ASF-CR] Complete guide to important environment variables for build, test, and mini-cluster operations.
Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.
Change subject: Complete guide to important environment variables for build, test, and mini-cluster operations.
......................................................................
Patch Set 1:
> (2 comments)
>
> > > I personally abhor Google docs as they just seem to get lost in
> > the
> > > Wiki-sea over time. If we are going to take time to document
> the
> > > various set of environment variables controlling important
> > aspects
> > > of the build, why not make that accessible to both the internal
> > and
> > > external developers by including it in the codebase.
> > >
> > > This is my first draft of the standard build settings.
> >
> > What about the Wiki, like this page: https://cwiki.apache.org/confluence/display/IMPALA/Bootstrapping+an+Impala+Development+Environment+From+Scratch
>
> Our wikis are pretty nice, and for things like ASAN, totally worth
> documenting separately, but for non-major features, keeping this up
> to date still requires updating things in two places. In
> experience, this isn't something people will do reliably unless
> forced. I think we could force at least minimal documentation by
> keeping a white-list of exported build variables and enforcing it
> with a pre-checkin lint rule.
Do you plan to add that lint rule to this patch?
--
To view, visit http://gerrit.cloudera.org:8080/7350
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I16d34cb4fa0c60c5ad6d9c8764cc0ec21c5cb368
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5768: Better developer documentation
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-5768: Better developer documentation
......................................................................
Patch Set 4: Code-Review+1
(1 comment)
LGTM. I'll give other reviewers a chance to weigh in.
http://gerrit.cloudera.org:8080/#/c/7350/4/README.md
File README.md:
Line 105: | SENTRY_HOME | "${CDH_COMPONENTS_HOME}/toolchain/" | |
Should this be /sentry-?? or something?
--
To view, visit http://gerrit.cloudera.org:8080/7350
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I16d34cb4fa0c60c5ad6d9c8764cc0ec21c5cb368
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5768: Better developer documentation
Posted by "Zach Amsden (Code Review)" <ge...@cloudera.org>.
Hello Tim Armstrong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7350
to look at the new patch set (#5).
Change subject: IMPALA-5768: Better developer documentation
......................................................................
IMPALA-5768: Better developer documentation
Guide to important environment variables for
build, impala paths and config cleanup.
Change-Id: I16d34cb4fa0c60c5ad6d9c8764cc0ec21c5cb368
---
M README.md
M bin/impala-config.sh
2 files changed, 75 insertions(+), 25 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/50/7350/5
--
To view, visit http://gerrit.cloudera.org:8080/7350
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I16d34cb4fa0c60c5ad6d9c8764cc0ec21c5cb368
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>
[Impala-ASF-CR] Complete guide to important environment variables for build, test, and mini-cluster operations.
Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
David Knupp has posted comments on this change.
Change subject: Complete guide to important environment variables for build, test, and mini-cluster operations.
......................................................................
Patch Set 1:
(2 comments)
Whether it's a .txt file in the code base, or a wiki entry, how can we make sure that this doc stays in sync with impala-config.sh (and/or any other place where env vars get set)? We'd have multiple sources of truth, wouldn't we?
http://gerrit.cloudera.org:8080/#/c/7350/1/bin/impala-config.sh
File bin/impala-config.sh:
Line 440
Speaking confidently as the only team member with a fine arts education, I'm shocked that I did not know this setting existed. :-)
http://gerrit.cloudera.org:8080/#/c/7350/1/docs/Developer-Guide/Build-Environment.txt
File docs/Developer-Guide/Build-Environment.txt:
Line 1: Environment Variables:
How many of these are set in impala-config.sh, and how many are set by other means?
--
To view, visit http://gerrit.cloudera.org:8080/7350
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I16d34cb4fa0c60c5ad6d9c8764cc0ec21c5cb368
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] Complete guide to important environment variables for build, test, and mini-cluster operations.
Posted by "Zach Amsden (Code Review)" <ge...@cloudera.org>.
Zach Amsden has posted comments on this change.
Change subject: Complete guide to important environment variables for build, test, and mini-cluster operations.
......................................................................
Patch Set 1:
(2 comments)
> > I personally abhor Google docs as they just seem to get lost in
> the
> > Wiki-sea over time. If we are going to take time to document the
> > various set of environment variables controlling important
> aspects
> > of the build, why not make that accessible to both the internal
> and
> > external developers by including it in the codebase.
> >
> > This is my first draft of the standard build settings.
>
> What about the Wiki, like this page: https://cwiki.apache.org/confluence/display/IMPALA/Bootstrapping+an+Impala+Development+Environment+From+Scratch
Our wikis are pretty nice, and for things like ASAN, totally worth documenting separately, but for non-major features, keeping this up to date still requires updating things in two places. In experience, this isn't something people will do reliably unless forced. I think we could force at least minimal documentation by keeping a white-list of exported build variables and enforcing it with a pre-checkin lint rule.
http://gerrit.cloudera.org:8080/#/c/7350/1/bin/impala-config.sh
File bin/impala-config.sh:
Line 440
> Speaking confidently as the only team member with a fine arts education, I'
Glad to know that my alternative career as the artist Deth Leprikon remains a well kept secret.
http://gerrit.cloudera.org:8080/#/c/7350/1/docs/Developer-Guide/Build-Environment.txt
File docs/Developer-Guide/Build-Environment.txt:
Line 1: Environment Variables:
> How many of these are set in impala-config.sh, and how many are set by othe
Additional docs will be forthcoming
--
To view, visit http://gerrit.cloudera.org:8080/7350
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I16d34cb4fa0c60c5ad6d9c8764cc0ec21c5cb368
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] Complete guide to important environment variables for build, test, and mini-cluster operations.
Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.
Change subject: Complete guide to important environment variables for build, test, and mini-cluster operations.
......................................................................
Patch Set 1:
> I personally abhor Google docs as they just seem to get lost in the
> Wiki-sea over time. If we are going to take time to document the
> various set of environment variables controlling important aspects
> of the build, why not make that accessible to both the internal and
> external developers by including it in the codebase.
>
> This is my first draft of the standard build settings.
What about the Wiki, like this page: https://cwiki.apache.org/confluence/display/IMPALA/Bootstrapping+an+Impala+Development+Environment+From+Scratch
--
To view, visit http://gerrit.cloudera.org:8080/7350
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I16d34cb4fa0c60c5ad6d9c8764cc0ec21c5cb368
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] Guide to important environment variables for build, test, and mini-cluster operations.
Posted by "Zach Amsden (Code Review)" <ge...@cloudera.org>.
Zach Amsden has posted comments on this change.
Change subject: Guide to important environment variables for build, test, and mini-cluster operations.
......................................................................
Patch Set 2:
I cut this back in scope somewhat and made it use README.md so that it shows up nicely formatted. I'll go on and document the mini-cluster separately and file a JIRA for this if everyone is happy with the format.
--
To view, visit http://gerrit.cloudera.org:8080/7350
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I16d34cb4fa0c60c5ad6d9c8764cc0ec21c5cb368
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] Guide to important environment variables for build, test, and mini-cluster operations.
Posted by "Zach Amsden (Code Review)" <ge...@cloudera.org>.
Zach Amsden has posted comments on this change.
Change subject: Guide to important environment variables for build, test, and mini-cluster operations.
......................................................................
Patch Set 2:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/7350/2/bin/impala-config.sh
File bin/impala-config.sh:
Line 343: if type nproc >/dev/null 2>&1; then
> I used "getconf _NPROCESSORS_ONLN" in infra/python/bootstrap_virtualenv.py.
Done
PS2, Line 433: nproc
> Hmm, should this use $CORES?
No, I think we only want to reduce concurrency for tests, which might actually start doing multi-threaded things, but for the build, full steam ahead.
Line 456: LIBHDFS_OPTS="${LIBHDFS_OPTS}:${IMPALA_HOME}/be/build/debug/service"
> Pretty sure we don't need this, since everything works on a release build.
mabe safer to put be/build/latest/service ?
Line 473: LD_LIBRARY_PATH="${LD_LIBRARY_PATH}:${IMPALA_HOME}/be/build/debug/service"
> Same here. Pretty sure that we don't need to add this to LD_LIBRARY_PATH.
Agree, but unsure about the combinations of shared / static builds needed to test this hypothesis.
Line 491: alias gerrit-verify-only="${IMPALA_AUX_TEST_HOME}/jenkins/gerrit-verify-only.sh"
> We can remove this while we're here too - it's referring to a Cloudera-inte
Done
--
To view, visit http://gerrit.cloudera.org:8080/7350
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I16d34cb4fa0c60c5ad6d9c8764cc0ec21c5cb368
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5768: Better developer documentation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-5768: Better developer documentation
......................................................................
Patch Set 6: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/7350
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I16d34cb4fa0c60c5ad6d9c8764cc0ec21c5cb368
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] Guide to important environment variables for build, test, and mini-cluster operations.
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: Guide to important environment variables for build, test, and mini-cluster operations.
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/7350/2/bin/impala-config.sh
File bin/impala-config.sh:
Line 456: LIBHDFS_OPTS="${LIBHDFS_OPTS}:${IMPALA_HOME}/be/build/debug/service"
> mabe safer to put be/build/latest/service ?
I agree that it's wrong, and /latest/ is the correct way to point it at that directly, but I was thinking that the wrongness provided evidence that it wasn't actually needed.
You're right that we should probably validate it before changing it, so it's a bad idea to tack it onto this change.
Line 473: LD_LIBRARY_PATH="${LD_LIBRARY_PATH}:${IMPALA_HOME}/be/build/debug/service"
> Agree, but unsure about the combinations of shared / static builds needed t
Yeah probably best not to tack it on if we're not confident.
--
To view, visit http://gerrit.cloudera.org:8080/7350
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I16d34cb4fa0c60c5ad6d9c8764cc0ec21c5cb368
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] Complete guide to important environment variables for build, test, and mini-cluster operations.
Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
David Knupp has posted comments on this change.
Change subject: Complete guide to important environment variables for build, test, and mini-cluster operations.
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7350/1/docs/Developer-Guide/Build-Environment.txt
File docs/Developer-Guide/Build-Environment.txt:
Line 1: Environment Variables:
> Additional docs will be forthcoming
I may be missing something, but there seem to be a few missing settings, e.g., these from testdata/bin/cluster/admin
export HDFS_WEBUI_PORT=5070 # changed from 50070 so it is not ephemeral
export YARN_WEBUI_PORT=8088 # same as default
export KMS_WEBUI_PORT=9600 # changed to make room for non-ephemeral HBase ports
# (HADOOP-12811)
export KUDU_WEBUI_PORT=8051 # same as default
--
To view, visit http://gerrit.cloudera.org:8080/7350
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I16d34cb4fa0c60c5ad6d9c8764cc0ec21c5cb368
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] Guide to important environment variables for build, test, and mini-cluster operations.
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: Guide to important environment variables for build, test, and mini-cluster operations.
......................................................................
Patch Set 2:
(7 comments)
Looks good to me. I noticed a few more things we could clean up but we don't necessarily need to bundle them in this change.
http://gerrit.cloudera.org:8080/#/c/7350/2/bin/impala-config.sh
File bin/impala-config.sh:
PS2, Line 337:
Nice catch
Line 460
Thank you for removing this.
Line 343: if type nproc >/dev/null 2>&1; then
I used "getconf _NPROCESSORS_ONLN" in infra/python/bootstrap_virtualenv.py. My understanding is that it works pretty much anywhere so we could avoid this branch.. Tangential but might as well simplify this script while we're here.
PS2, Line 433: nproc
Hmm, should this use $CORES?
Line 456: LIBHDFS_OPTS="${LIBHDFS_OPTS}:${IMPALA_HOME}/be/build/debug/service"
Pretty sure we don't need this, since everything works on a release build. It's kind of tangential but while we're here...
Line 473: LD_LIBRARY_PATH="${LD_LIBRARY_PATH}:${IMPALA_HOME}/be/build/debug/service"
Same here. Pretty sure that we don't need to add this to LD_LIBRARY_PATH.
Line 491: alias gerrit-verify-only="${IMPALA_AUX_TEST_HOME}/jenkins/gerrit-verify-only.sh"
We can remove this while we're here too - it's referring to a Cloudera-internal script which a) doesn't belong in the ASF repo and b) no longer works since we moved to jenkins.impala.io
--
To view, visit http://gerrit.cloudera.org:8080/7350
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I16d34cb4fa0c60c5ad6d9c8764cc0ec21c5cb368
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] Complete guide to important environment variables for build, test, and mini-cluster operations.
Posted by "Zach Amsden (Code Review)" <ge...@cloudera.org>.
Zach Amsden has posted comments on this change.
Change subject: Complete guide to important environment variables for build, test, and mini-cluster operations.
......................................................................
Patch Set 1:
I personally abhor Google docs as they just seem to get lost in the Wiki-sea over time. If we are going to take time to document the various set of environment variables controlling important aspects of the build, why not make that accessible to both the internal and external developers by including it in the codebase.
This is my first draft of the standard build settings.
--
To view, visit http://gerrit.cloudera.org:8080/7350
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I16d34cb4fa0c60c5ad6d9c8764cc0ec21c5cb368
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5768: Better developer documentation
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-5768: Better developer documentation
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/7350
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I16d34cb4fa0c60c5ad6d9c8764cc0ec21c5cb368
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5768
Posted by "Zach Amsden (Code Review)" <ge...@cloudera.org>.
Hello Tim Armstrong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7350
to look at the new patch set (#3).
Change subject: IMPALA-5768
......................................................................
IMPALA-5768
Guide to important environment variables for
build, impala paths and config cleanup.
Change-Id: I16d34cb4fa0c60c5ad6d9c8764cc0ec21c5cb368
---
M README.md
M bin/impala-config.sh
2 files changed, 75 insertions(+), 25 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/50/7350/3
--
To view, visit http://gerrit.cloudera.org:8080/7350
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I16d34cb4fa0c60c5ad6d9c8764cc0ec21c5cb368
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>
[Impala-ASF-CR] IMPALA-5768: Better developer documentation
Posted by "Zach Amsden (Code Review)" <ge...@cloudera.org>.
Hello Tim Armstrong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7350
to look at the new patch set (#4).
Change subject: IMPALA-5768: Better developer documentation
......................................................................
IMPALA-5768: Better developer documentation
Guide to important environment variables for
build, impala paths and config cleanup.
Change-Id: I16d34cb4fa0c60c5ad6d9c8764cc0ec21c5cb368
---
M README.md
M bin/impala-config.sh
2 files changed, 75 insertions(+), 25 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/50/7350/4
--
To view, visit http://gerrit.cloudera.org:8080/7350
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I16d34cb4fa0c60c5ad6d9c8764cc0ec21c5cb368
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>