Re: Unknown clang-tidy failure in GVO

[Michael Brown]

At the bottom of
https://jenkins.impala.io/job/clang-tidy-ub1604/78/consoleFull I see:

*16:05:31* + bin/run_clang_tidy.sh*16:13:18* + grep ']'
/home/ubuntu/tidylog.txt*16:13:18*
/home/ubuntu/Impala/be/src/rpc/thrift-server-test.cc:105:26: warning:
extra ';' after member function definition
[clang-diagnostic-extra-semi]*16:13:18*
/home/ubuntu/Impala/be/src/rpc/thrift-server-test.cc:106:29: warning:
extra ';' after member function definition
[clang-diagnostic-extra-semi]*16:13:18*
/home/ubuntu/Impala/be/src/rpc/thrift-server-test.cc:149:7: warning:
ignoring return value of function declared with 'nodiscard' attribute
[clang-diagnostic-unused-result]


On Thu, Oct 26, 2017 at 12:57 PM, Sailesh Mukil 
wrote:

> Does anyone know the cause for this failure in the clang-tidy run in GVO?
> It's something to do with hadoop-lzo.
>
> https://jenkins.impala.io/job/clang-tidy-ub1604/78/consoleFull
>
>
> *15:52:44*   [javadoc] Generating
> /home/ubuntu/hadoop-lzo/build/docs/api/help-doc.html...*15:52:44*
> [javadoc] 1 error*15:52:44*   [javadoc] 1 warning*15:52:44*
> [javadoc] javadoc: error - Error while reading file
> /home/ubuntu/hadoop-lzo/src/java/overview.html*15:52:44* *15:52:44*
> package:*15:52:44* [mkdir] Created dir:
> /home/ubuntu/hadoop-lzo/build/hadoop-lzo-0.4.15*15:52:44* [mkdir]
> Created dir: /home/ubuntu/hadoop-lzo/build/hadoop-lzo-0.4.15/lib*15:52:44*
> [mkdir] Created dir:
> /home/ubuntu/hadoop-lzo/build/hadoop-lzo-0.4.15/docs*15:52:44*
> [mkdir] Created dir: /home/ubuntu/hadoop-lzo/lib*15:52:44*  [copy]
> Copying 56 files to
> /home/ubuntu/hadoop-lzo/build/hadoop-lzo-0.4.15/lib*15:52:44*
> [copy] Copying 1 file to
> /home/ubuntu/hadoop-lzo/build/hadoop-lzo-0.4.15*15:52:44*  [exec]
> Created /home/ubuntu/hadoop-lzo/build/hadoop-lzo-0.4.15/lib/native/
> docs*15:52:44*
>  [exec] Copying libraries in /docs to
> /home/ubuntu/hadoop-lzo/build/hadoop-lzo-0.4.15/lib/native/docs/*15:52:44*
>  [exec] Created
> /home/ubuntu/hadoop-lzo/build/hadoop-lzo-0.4.15/lib/native/
> hadoop-lzo-0.4.15.jar*15:52:44*
>  [exec] Copying libraries in /hadoop-lzo-0.4.15.jar to
> /home/ubuntu/hadoop-lzo/build/hadoop-lzo-0.4.15/lib/native/
> hadoop-lzo-0.4.15.jar/*15:52:44*
>  [exec] Created
> /home/ubuntu/hadoop-lzo/build/hadoop-lzo-0.4.15/lib/native/lib*15:52:44*
>  [exec] Copying libraries in /lib to
> /home/ubuntu/hadoop-lzo/build/hadoop-lzo-0.4.15/lib/native/lib/*15:52:44*
>  [exec] Created
> /home/ubuntu/hadoop-lzo/build/hadoop-lzo-0.4.15/lib/native/
> Linux-amd64-64*15:52:44*
>  [exec] Copying libraries in
> /home/ubuntu/hadoop-lzo/build/native/Linux-amd64-64/lib to
> /home/ubuntu/hadoop-lzo/build/hadoop-lzo-0.4.15/lib/native/
> Linux-amd64-64/*15:52:44*
>  [exec] /home/ubuntu/hadoop-lzo/src/native/packageNativeHadoop.sh:
> 44: cd: can't cd to /docs/*15:52:44*  [exec] tar:
> *gplcompression*: Cannot stat: No such file or directory*15:52:44*
>  [exec] tar: Exiting with failure status due to previous
> errors*15:52:44*  [exec]
> /home/ubuntu/hadoop-lzo/src/native/packageNativeHadoop.sh: 44: cd:
> can't cd to /hadoop-lzo-0.4.15.jar/*15:52:44*  [exec] tar:
> *gplcompression*: Cannot stat: No such file or directory*15:52:44*
>  [exec] tar: Exiting with failure status due to previous
> errors*15:52:44*  [exec] tar: *gplcompression*: Cannot stat: No
> such file or directory*15:52:44*  [exec] tar: Exiting with failure
> status due to previous errors*15:52:44*  [copy] Copying 77 files
> to /home/ubuntu/hadoop-lzo/build/hadoop-lzo-0.4.15/docs*15:52:44*
> [copy] Copying 1 file to
> /home/ubuntu/hadoop-lzo/build/hadoop-lzo-0.4.15*15:52:44*  [copy]
> Copying 4 files to
> /home/ubuntu/hadoop-lzo/build/hadoop-lzo-0.4.15/ivy*15:52:44*
> [copy] Copying 1 file to
> /home/ubuntu/hadoop-lzo/build/hadoop-lzo-0.4.15*15:52:44*  [copy]
> Copying 64 files to
> /home/ubuntu/hadoop-lzo/build/hadoop-lzo-0.4.15/src*15:52:44*
> [copy] Copying 1 file to
> /home/ubuntu/hadoop-lzo/build/hadoop-lzo-0.4.15
>

Re: [VOTE] Graduate to a TLP

[Michael Brown]

+1

On Tue, Oct 17, 2017 at 7:06 PM, Jim Apple  wrote:

> Following our discussion
> https://lists.apache.org/thread.html/2f5db4788aff9b0557354b9106c032
> 8a29c1f90c1a74a228163949d2@%3Cdev.impala.apache.org%3E
> , I propose that we graduate to a TLP. According to
> https://incubator.apache.org/guides/graduation.html#
> community_graduation_vote
> this is not required, and https://impala.apache.org/bylaws.html does not
> say whose votes are "binding" in a graduation vote, so all community
> members are welcome to vote.
>
> This will remain open 72 hours. I will be notifying general@incubator it
> is
> occurring.
>
> This is my +1.
>

Re: Time for graduation?

[Michael Brown]

I am in favor of graduation.

On Thu, Oct 12, 2017 at 2:17 PM, Todd Lipcon  wrote:

> Hey Impala community,
>
> It's been a while that all of the Impala infrastructure has been moved
> over, and the community appears to be functioning healthily, generating new
> releases on a regular cadence as well as adding new committers and PPMC
> members. All of the branding stuff seems great, and the user mailing list
> has a healthy amount of traffic and a good track record of answering
> questions when they come up.
>
> As a mentor I think it's probably time to discuss graduation. The project
> is already functioning in the same way as your typical Apache TLP and it
> seems like it's time to become one.
>
> Any thoughts? If everyone is on board, the next step would be:
>
> 1. Pick the initial PMC chair for the TLP. According to the published
> Impala Bylaws it seems that this is meant to rotate annually, so no need to
> stress too much about it.
>
> A couple obvious choices here would be Marcel (as the original founder of
> the project) or perhaps Jim (who has done yeoman's work on a lot of the
> incubation process, podling reports, etc). Others could certainly volunteer
> or be nominated as well.
>
> 2. Draft a Resolution for the PPMC and IPMC to vote upon.
> -- the resolution would include the above-decided chair as well as the list
> of initial PMC, etc.
> -- the Initial PMC could be just the current list of PPMC, or you could
> consider adding others at this point as well.
>
>
> I can help with the above process but figured I'd solicit opinions first on
> whether the communit feels it's ready to graduate.
>
> Thanks
> Todd
>

Re: upcoming Jira downtime

[Michael Brown]

Good news, everyone!

Hello,

On Friday, October 13, 2017 at 12:45:00 AM UTC, Jira will be taken down for
a restart. This is to apply a potential fix for the intermittent timeout
issue we have been experiencing. The downtime should be approximately 10
minutes or less.

-Chris
ASF Infra


On Tue, Oct 3, 2017 at 8:49 AM, Michael Brown  wrote:

> This was sent to committ...@apache.org et al:
>
> Please be advised:
>
>
> Sun, 08 Oct 2017 00:45:00 GMT - Sun, 08 Oct 2017 06:45:00 GMT:
>
> Jira - issues.apache.org will be down for approximately 6 hours while the
> database is exported and reloaded to correct a database collation/encoding
> issue. This is necessary in preparation for the 7.x upgrade (to be
> scheduled soon.)
>
> -Chris
> ASF Infra
>

Re: Jenkins down briefly to take quiesced snapshot

[Michael Brown]

It's back up.

On Thu, Oct 12, 2017 at 7:14 AM, Michael Brown  wrote:

> Sorry for late notice, should only take a few minutes. No jobs are
> currently running.
>
> This is in preparation for an upgrade in the next few days.
>

Jenkins down briefly to take quiesced snapshot

[Michael Brown]

Sorry for late notice, should only take a few minutes. No jobs are
currently running.

This is in preparation for an upgrade in the next few days.

Re: Build Impala Rpm

[Michael Brown]

Hi sky,

Unfortunately we don't provide this upstream yet (contributions welcome!),
but the summary is that you would craft a specfile. In the specfile, you
would

1. Define package dependencies. bin/bootstrap_development.sh in Impala
source is a good way to know what dependencies Impala has.

2. Define how to build Impala. (buildall.sh in Impala source)

3. Define what artifacts from buildall.sh you want to put in your package
(look in be/build) and where they should reside when a user installs your
RPM (e.g., /opt/impala/bin/impalad)

4. You would have to create and bundle more items yourself, like initd or
systemd startup files and any conf files

http://rpm.org/documentation.html seems like a good start for a reference.

Good luck!


On Wed, Oct 11, 2017 at 4:15 AM, sky  wrote:

> Hi all,
> How to compile impala into rpm package？

upcoming Jira downtime

[Michael Brown]

This was sent to committ...@apache.org et al:

Please be advised:


Sun, 08 Oct 2017 00:45:00 GMT - Sun, 08 Oct 2017 06:45:00 GMT:

Jira - issues.apache.org will be down for approximately 6 hours while the
database is exported and reloaded to correct a database collation/encoding
issue. This is necessary in preparation for the 7.x upgrade (to be
scheduled soon.)

-Chris
ASF Infra

"tests for tests" in gerrit-verify-dryrun?

[Michael Brown]

Hello,

I'm about to start working on Impala's random query generator, a testing
tool to help find test gaps in Impala's functional tests.

The random query generator and infra code around it has some functional and
pure unit tests [0] that are not part of GVD, but it wouldn't be hard to
fold them into GVD's execution. As part of the upcoming work, I plan to add
even more tests: we need quick unit or functional tests to ensure a test
tool is working as expected.

What are people's thoughts on having these "tests for tests", or infra
tests, be part of GVD?

Pros:
1. Helps prevent regression in tools and infra

2. Verification procedure is the same as with the rest of Impala: run
gerrit-verify-dryrun

3. Automatic Apache RAT verification

Cons:
1. Patches to the random query generator tend to be self-contained. Ought
we spend more AWS cycles and time building Impala and running these tests
in order to run some ostensible (but growing) infra tests?

2. Flaky tests and failing builds can block test tool progress

Other solutions if the cons win:
1. Separate Jenkins job for these tests (there's a separate job for
submitting and verifying docs, for instance). A con of this is that this
can lead to a proliferation of Jenkins jobs and confusion with contributors
on which jobs apply where. Also, if there is ever a patch where Impala
proper and query generator are both updated, which job wins?

2. Status quo and set Verified+1/Submitted by hand. This is much easier for
committers than non-committers. I'm OK with status quo, but in the past,
there have been requests to improve this situation [1]

For a data point, I can cd to "tests/comparison/tests", run
"impala-py.test", and 71 tests take about 10 seconds to run.

Thanks for any feedback.

[0]
https://git-wip-us.apache.org/repos/asf?p=incubator-impala.git;a=tree;f=tests/comparison/tests;h=49e3b5d7d9a6f5f716c135bda36292e05fb0e0d3;hb=HEAD

[1] https://issues.apache.org/jira/browse/IMPALA-4756

reminder: Jenkins jobs available to all contributors

[Michael Brown]

For all contributors (this includes contributors who are not project
committers), the following Jenkins jobs are available to all Impala
contributors for testing patches they submit to Gerrit.

https://jenkins.impala.io/job/pre-review-test/

Use this to compile Impala and run, if desired, tests or test subsets.
bin/run-all-tests.sh in the source tree is the best place to understand the
meanings to the options of this job.

https://jenkins.impala.io/job/gerrit-verify-dryrun-external/ (new September
2017)

Use this to run the full set of verifications, including multiple
compilations of Impala using various build tools, and execution of all
backend tests, frontend tests, core end-to-end tests, and Apache RAT
verification. This is a wrapper around what a committer will ultimately run
for you to verify and submit your patch (gerrit-verify-dryrun).

Neither of these jobs write a comment or Verified bit or attempt to Submit
a Gerrit patch set. A submitter ought to monitor his or her job as it
progresses. A submitter is encouraged to manually link a successful job in
a Gerrit review comment as demonstration of success.

This language is also here:

https://cwiki.apache.org/confluence/display/IMPALA/Using+Gerrit+to+submit+and+review+patches#UsingGerrittosubmitandreviewpatches-Verifyingapatch(opentoallImpalacontributors)

Re: [VOTE] 2.10.0 release candidate 2 (RC2)

[Michael Brown]

+1 (binding)

- downloaded and verified tarballs
- compiled and ran core tests
- connected and ran a few queries


On Wed, Aug 30, 2017 at 11:35 PM, Bharath Vissapragada <
bhara...@cloudera.com> wrote:

> This is a vote to release Impala 2.10.0.
>
> - The artefacts for testing can be downloaded from <
> https://dist.apache.org/repos/dist/dev/incubator/impala/2.10.0/RC2/>
>
> - The git tag for this release candidate is 2.10.0-rc2 and treehash is
> visible at
> <
> https://git-wip-us.apache.org/repos/asf?p=incubator-impala.git;a=tree;hb=
> 23d79462da5d0108709e8b1399c97606f4ebdf92
> >
>
> Please vote +1 or -1. -1 votes should be accompanied by an explanation of
> the reason. Only PPMC members and mentors have binding votes, but other
> community members are encouraged to cast non-binding votes. This vote will
> pass if there are 3 binding +1 votes and more binding +1 votes than -1
> votes.
>
> This wiki page describes how to check the release before you vote:
> *https://cwiki.apache.org/confluence/display/IMPALA/How+
> to+Release#HowtoRelease-HowtoVoteonaReleaseCandidate
>  to+Release#HowtoRelease-HowtoVoteonaReleaseCandidate>*
>
> The vote will be open until the end of day, September 5th, Pacific time
> zone (UTC-08:00).
> Once the vote passes the Impala PPMC vote, it still must pass the incubator
> PMC vote before a release is made.
>

Re: jenkins.impala.io pre-existing workspace

[Michael Brown]

Looks like someone has done this.

On Wed, Aug 23, 2017 at 8:16 PM, Alexander Behm 
wrote:

> Yes, let's please add the post-build action for sanity and consistency with
> our other jobs.
>
> On Wed, Aug 23, 2017 at 7:42 PM, Tim Armstrong 
> wrote:
>
> > Maybe the workspace just got left in a weird state - I think in most
> cases
> > "git init" followed by checking out a branch and doing a clean would
> work.
> >
> > Should we add the delete workspace post-build action?
> >
> > On Wed, Aug 23, 2017 at 5:32 PM, Michael Brown 
> wrote:
> >
> > > Not a known issue. I noticed ubuntu-16.04-from-scratch is not set to
> > clean
> > > up its workspace, and its config has not been touched since Aug 11. It
> > > seems strange we only saw this now
> > >
> > > On Wed, Aug 23, 2017 at 5:25 PM, Tim Armstrong <
> tarmstr...@cloudera.com>
> > > wrote:
> > >
> > > > Is this a known problem? My job failed because the Impala repo
> already
> > > > existed on the machine:
> > > >
> > > > https://jenkins.impala.io/job/ubuntu-16.04-from-scratch/164/
> > > >
> > > > *23:00:24* + /usr/bin/git init /home/ubuntu/Impala*23:00:24*
> > > > Reinitialized existing Git repository in /home/ubuntu/Impala/.git/
> > > > 
> > > > *23:02:18* + for ITER in '$(seq 1 10)'*23:02:18* + echo 'ATTEMPT:
> > > > 1'*23:02:18* ATTEMPT: 1*23:02:18* + /usr/bin/git checkout
> > > > FETCH_HEAD*23:02:18* + cat
> > > > /home/ubuntu/Impala/tmp.3tYBn0GUga*23:02:18* 23:02:18.712300
> git.c:344
> > > >   trace: built-in: git 'checkout' 'FETCH_HEAD'*23:02:18*
> > > > error: The following untracked working tree files would be
> overwritten
> > > > by checkout:*23:02:18*  .clang-format*23:02:18*
> > > >  .clang-tidy*23:02:18*
> > > > .gitignore*23:02:18*CMakeLists.txt*23:02:18*
> > > > DISCLAIMER*23:02:18*
> > > > EXPORT_CONTROL.md*23:02:18* LICENSE.txt*23:02:18*
> > > >  LOGS.md*23:02:18*
> > > > NOTICE.txt*23:02:18*README.md*23:02:18*
> > > >  be/.gitignore*23:02:18*
> > > > be/.impala.doxy*23:02:18*   be/CMakeLists.txt*23:02:18*
> > > > be/src/benchmarks/CMakeLists.txt*23:02:18*
> > > > be/src/benchmarks/atod-benchmark.cc*23:02:18*
> > > > be/src/benchmarks/atof-benchmark.cc*23:02:18*
> > > > be/src/benchmarks/atoi-benchmark.cc*23:02:18*
> > > > be/src/benchmarks/bit-packing-benchmark.cc*23:02:18*
> > > > be/src/benchmarks/bitmap-benchmark.cc
> > > > ...
> > > >
> > >
> >
>

Re: jenkins.impala.io pre-existing workspace

[Michael Brown]

Not a known issue. I noticed ubuntu-16.04-from-scratch is not set to clean
up its workspace, and its config has not been touched since Aug 11. It
seems strange we only saw this now

On Wed, Aug 23, 2017 at 5:25 PM, Tim Armstrong 
wrote:

> Is this a known problem? My job failed because the Impala repo already
> existed on the machine:
>
> https://jenkins.impala.io/job/ubuntu-16.04-from-scratch/164/
>
> *23:00:24* + /usr/bin/git init /home/ubuntu/Impala*23:00:24*
> Reinitialized existing Git repository in /home/ubuntu/Impala/.git/
> 
> *23:02:18* + for ITER in '$(seq 1 10)'*23:02:18* + echo 'ATTEMPT:
> 1'*23:02:18* ATTEMPT: 1*23:02:18* + /usr/bin/git checkout
> FETCH_HEAD*23:02:18* + cat
> /home/ubuntu/Impala/tmp.3tYBn0GUga*23:02:18* 23:02:18.712300 git.c:344
>   trace: built-in: git 'checkout' 'FETCH_HEAD'*23:02:18*
> error: The following untracked working tree files would be overwritten
> by checkout:*23:02:18*  .clang-format*23:02:18*
>  .clang-tidy*23:02:18*
> .gitignore*23:02:18*CMakeLists.txt*23:02:18*
> DISCLAIMER*23:02:18*
> EXPORT_CONTROL.md*23:02:18* LICENSE.txt*23:02:18*
>  LOGS.md*23:02:18*
> NOTICE.txt*23:02:18*README.md*23:02:18*
>  be/.gitignore*23:02:18*
> be/.impala.doxy*23:02:18*   be/CMakeLists.txt*23:02:18*
> be/src/benchmarks/CMakeLists.txt*23:02:18*
> be/src/benchmarks/atod-benchmark.cc*23:02:18*
> be/src/benchmarks/atof-benchmark.cc*23:02:18*
> be/src/benchmarks/atoi-benchmark.cc*23:02:18*
> be/src/benchmarks/bit-packing-benchmark.cc*23:02:18*
> be/src/benchmarks/bitmap-benchmark.cc
> ...
>

Re: jenkins.impala.io outage tomorrow for instance size upgrade

[Michael Brown]

jenkins.impala.io is back up and a test GVD has gotten far enough along
that everything seems to be in order.. Please let us know if you have any
problems.

We now have more free memory than in use, which was absolutely not the case
before.

On Wed, Aug 9, 2017 at 7:42 AM, Michael Brown  wrote:

> Reminder: this is happening soon.
>
> On Tue, Aug 8, 2017 at 9:09 AM, Michael Brown  wrote:
>
>> Hello,
>>
>> In response to the incident last week in which jenkins.impala.io ran out
>> of memory, we will be increasing the instance size. This requires downtime:
>> Amazon recommends quiesced instances when snapshotting root volumes.
>>
>> jenkins.impala.io will go down at 8 A.M. Pacific tomorrow for this
>> purpose. We are hoping downtime of no more than 2 hours.
>>
>
>

Re: jenkins.impala.io outage tomorrow for instance size upgrade

[Michael Brown]

Reminder: this is happening soon.

On Tue, Aug 8, 2017 at 9:09 AM, Michael Brown  wrote:

> Hello,
>
> In response to the incident last week in which jenkins.impala.io ran out
> of memory, we will be increasing the instance size. This requires downtime:
> Amazon recommends quiesced instances when snapshotting root volumes.
>
> jenkins.impala.io will go down at 8 A.M. Pacific tomorrow for this
> purpose. We are hoping downtime of no more than 2 hours.
>

jenkins.impala.io outage tomorrow for instance size upgrade

[Michael Brown]

Hello,

In response to the incident last week in which jenkins.impala.io ran out of
memory, we will be increasing the instance size. This requires downtime:
Amazon recommends quiesced instances when snapshotting root volumes.

jenkins.impala.io will go down at 8 A.M. Pacific tomorrow for this purpose.
We are hoping downtime of no more than 2 hours.

Re: jenkins.impala.io outage

[Michael Brown]

Update: I've restarted a GVD for https://gerrit.cloudera.org/#/c/7573/
because it fixes a broken build blocker. I have to do things to get ready
for my day now, but I'll check back in a few hours.

On Thu, Aug 3, 2017 at 5:13 AM, Michael Brown  wrote:

> Hello,
>
> About 6 hours ago the Jenkins server on jenkins.impala.io ran out of heap
> space.
>
> I've restarted Jenkins again, and several of us will get together to see
> what happened. I think it will probably result in a larger instance size.
>

jenkins.impala.io outage

[Michael Brown]

Hello,

About 6 hours ago the Jenkins server on jenkins.impala.io ran out of heap
space.

I've restarted Jenkins again, and several of us will get together to see
what happened. I think it will probably result in a larger instance size.

Re: GVO machines dependencies

[Michael Brown]

"from-scratch" is a misnomer: only the first build to run on a given node
is truly from-scratch, because running impala-setup takes effect
system-wide, and is run once, when the node is created.

For http://jenkins.impala.io:8080/job/ubuntu-14.04-from-scratch/1371/ , the
worker's build history suggests it has been up roughly 17 hours.
http://jenkins.impala.io:8080/computer/ub1404-c4.4xl-gp2%20(i-0d76efbc8de26926c)/builds
This means your recent change hasn't taken effect on "older" workers.

Any new workers that get created should get the change.





On Wed, May 24, 2017 at 4:34 PM, Sailesh Mukil  wrote:

> Thanks Michael and Jim,
>
> It looks like adding to bin/bootstrap_build.sh makes the ub14-build-only
> job work fine, however, updating the impala-setup still doesn't seem to
> work. I submitted a pull request that was merged by Dimitris here:
> https://github.com/awleblang/impala-setup/commits/master
>
> Also, the impala-setup chef logs don't seem to get printed in the jenkins
> console output. So I'm not sure if it's actually being run or not.
>
> On Wed, May 24, 2017 at 3:37 PM, Michael Brown  wrote:
>
> > Thanks Jim; I had forgotten bin/bootstrap_build.sh.
> >
> > On Wed, May 24, 2017 at 3:14 PM, Jim Apple  wrote:
> >
> > > I'd recommend against #2 using the node setup to install Impala
> > > dependencies. The only reason it installs openjdk is that Jenkins
> > > needs Java to talk to it. All of the other Impala dependencies are
> > > installed in the jobs themselves.
> > >
> > > On Wed, May 24, 2017 at 3:11 PM, Michael Brown 
> > wrote:
> > > >> Is there a way to add a new dependency to these machines?
> > > >
> > > > Our builds use two worker labels:
> > > > 1. ubuntu14.04-c4.4xlarge-gp2
> > > > 2. ub14-build-only
> > > >
> > > > 1 uses https://github.com/awleblang/impala-setup, and I think you
> > should
> > > > add libffi to there. It's something you should do anyway so new users
> > get
> > > > set up with the right dependencies.
> > > >
> > > > 2 is a different story, but I see that at
> > > > http://jenkins.impala.io:8080/configure that worker is already
> > > installing a
> > > > JDK via apt-get. You could use a similar pattern to install libffi.
> > >
> >
>

Re: GVO machines dependencies

[Michael Brown]

Thanks Jim; I had forgotten bin/bootstrap_build.sh.

On Wed, May 24, 2017 at 3:14 PM, Jim Apple  wrote:

> I'd recommend against #2 using the node setup to install Impala
> dependencies. The only reason it installs openjdk is that Jenkins
> needs Java to talk to it. All of the other Impala dependencies are
> installed in the jobs themselves.
>
> On Wed, May 24, 2017 at 3:11 PM, Michael Brown  wrote:
> >> Is there a way to add a new dependency to these machines?
> >
> > Our builds use two worker labels:
> > 1. ubuntu14.04-c4.4xlarge-gp2
> > 2. ub14-build-only
> >
> > 1 uses https://github.com/awleblang/impala-setup, and I think you should
> > add libffi to there. It's something you should do anyway so new users get
> > set up with the right dependencies.
> >
> > 2 is a different story, but I see that at
> > http://jenkins.impala.io:8080/configure that worker is already
> installing a
> > JDK via apt-get. You could use a similar pattern to install libffi.
>

Re: GVO machines dependencies

[Michael Brown]

> Is there a way to add a new dependency to these machines?

Our builds use two worker labels:
1. ubuntu14.04-c4.4xlarge-gp2
2. ub14-build-only

1 uses https://github.com/awleblang/impala-setup, and I think you should
add libffi to there. It's something you should do anyway so new users get
set up with the right dependencies.

2 is a different story, but I see that at
http://jenkins.impala.io:8080/configure that worker is already installing a
JDK via apt-get. You could use a similar pattern to install libffi.

Re: Need access to Impala wiki to update a link

[Michael Brown]

Done. Thanks for volunteering!

On Mon, May 22, 2017 at 4:17 PM, Laurel Hale  wrote:

> Hi Michael,
> My user name is "laurel" Thanks!
> --Laurel
>
> On Mon, May 22, 2017 at 3:05 PM, Michael Brown  wrote:
>
>> OK. So please create a new user and tell me the username.
>> https://cwiki.apache.org/confluence/signup.action
>>
>> On Mon, May 22, 2017 at 2:23 PM, Laurel Hale  wrote:
>>
>>> I tried that, but it is not associated with my email address so it must
>>> be someone else's account (if that's possible)?
>>>
>>> On Mon, May 22, 2017 at 2:17 PM, Michael Brown 
>>> wrote:
>>>
>>>> I can help you.
>>>>
>>>> I see an account called 'lhale' already exists. Should I add
>>>> permissions to edit our wiki to that account? If you need to reset your
>>>> password, try https://cwiki.apache.org/confluence/forgotuserpassword.a
>>>> ction
>>>>
>>>> If you need to create a new account, use https://cwiki.apache.org/c
>>>> onfluence/signup.action
>>>>
>>>> Once you either reset your lhale password or create a new account,
>>>> please reply with the username to which I should grant permissions, and
>>>> I'll do so.
>>>>
>>>> Thanks!
>>>>
>>>>
>>>> On Mon, May 22, 2017 at 1:55 PM, Laurel Hale 
>>>> wrote:
>>>>
>>>>> Hi all,
>>>>> I've been asked to edit a link on this page of the Impala wiki:
>>>>>
>>>>> https://cwiki.apache.org/confluence/display/IMPALA/Documentation
>>>>>
>>>>> Can someone create an account for me?
>>>>>
>>>>> Thanks,
>>>>> Laurel
>>>>>
>>>>
>>>>
>>>
>>
>

Re: Need access to Impala wiki to update a link

[Michael Brown]

OK. So please create a new user and tell me the username.
https://cwiki.apache.org/confluence/signup.action

On Mon, May 22, 2017 at 2:23 PM, Laurel Hale  wrote:

> I tried that, but it is not associated with my email address so it must be
> someone else's account (if that's possible)?
>
> On Mon, May 22, 2017 at 2:17 PM, Michael Brown  wrote:
>
>> I can help you.
>>
>> I see an account called 'lhale' already exists. Should I add permissions
>> to edit our wiki to that account? If you need to reset your password, try
>> https://cwiki.apache.org/confluence/forgotuserpassword.action
>>
>> If you need to create a new account, use https://cwiki.apache.org/c
>> onfluence/signup.action
>>
>> Once you either reset your lhale password or create a new account, please
>> reply with the username to which I should grant permissions, and I'll do so.
>>
>> Thanks!
>>
>>
>> On Mon, May 22, 2017 at 1:55 PM, Laurel Hale  wrote:
>>
>>> Hi all,
>>> I've been asked to edit a link on this page of the Impala wiki:
>>>
>>> https://cwiki.apache.org/confluence/display/IMPALA/Documentation
>>>
>>> Can someone create an account for me?
>>>
>>> Thanks,
>>> Laurel
>>>
>>
>>
>

Re: Need access to Impala wiki to update a link

[Michael Brown]

I can help you.

I see an account called 'lhale' already exists. Should I add permissions to
edit our wiki to that account? If you need to reset your password, try
https://cwiki.apache.org/confluence/forgotuserpassword.action

If you need to create a new account, use
https://cwiki.apache.org/confluence/signup.action

Once you either reset your lhale password or create a new account, please
reply with the username to which I should grant permissions, and I'll do so.

Thanks!


On Mon, May 22, 2017 at 1:55 PM, Laurel Hale  wrote:

> Hi all,
> I've been asked to edit a link on this page of the Impala wiki:
>
> https://cwiki.apache.org/confluence/display/IMPALA/Documentation
>
> Can someone create an account for me?
>
> Thanks,
> Laurel
>

Re: error in documentation - timestamp_literals

[Michael Brown]

[Explicitly adding jiri.troup in case sender is not on dev@ list]

Actually, that docs error is in upstream documentation [0], and that's
where this should be fixed. Cloudera should then incorporate the change
from upstream.

https://issues.apache.org/jira/browse/IMPALA-5348 filed to address the
problem.

[0] http://impala.apache.org/docs/build/html/topics/impala_literals.html

On Mon, May 22, 2017 at 8:36 AM, Vincent Tran  wrote:

> Hi Jiri,
>
>
>
> Thanks for reporting. Can you please open a support case for this issue so
> that we can track it more effectively?
>
>
>
> http://support.cloudera.com
>
>
>
> Cheers,
>
>
>
> Vincent
>
>
>
>
>
> From: "Troup, Jiri" 
> Reply-To: 
> Date: Monday, May 22, 2017 at 5:57 AM
> To: "dev@impala.incubator.apache.org" 
> Subject: error in documentation - timestamp_literals
>
>
>
> I have found issue in your documentation
>
>
>
> I’m using
>
>
>
> impalad version 2.7.0-cdh5.9.0 RELEASE (build
> 4b4cf1936bd6cdf34fda5e2f32827e7d60c07a9c) Built on Fri Oct 21 01:07:22
> PDT 2016
>
>
>
> In documentation
>
>
>
> https://www.cloudera.com/documentation/enterprise/5-9-
> x/topics/impala_literals.html#timestamp_literals
>
>
>
> you say :
>
>
>
> You can also use INTERVAL expressions to add or subtract from timestamp
> literal values, such as '1966-07-30' + INTERVAL 5 YEARS + INTERVAL 3 DAYS.
> See TIMESTAMP Data Type for details.
>
>
>
> When I run query
>
>
>
> select  '1966-07-30'   + interval 1 year; --doesn’t work
> !!! but according your doc. this should work.
>
> select cast('1966-07-30' as timestamp) + interval 1 year; --have to
> manually cast to timestamp
>
>
>
>
>
>   Jiří Troup
> Consultant
>
>
>
> Adastra
> Nile House
> Karolinská 654/2
> 186 00 Praha 8
> Czech Republic
>
>
>
> jiri.tr...@adastragrp.com
> www.adastra.cz
> www.adastragrp.com
>
> Follow Adastra Czech Republic on LinkedIn, Facebook and YouTube.
>
>
>
>
>
>

Re: Could someone kick off a GVO for this?

[Michael Brown]

I saw that you rebased, so I carried the +2 for you and started GVD.

On Mon, May 15, 2017 at 2:39 PM, Michael Brown  wrote:

> It looks sort of old. Want to rebase first to make sure the latest tests
> run?
>
> On Mon, May 15, 2017 at 2:34 PM, Zachary Amsden 
> wrote:
>
>> https://gerrit.cloudera.org/#/c/6335/
>>
>
>

Re: Could someone kick off a GVO for this?

[Michael Brown]

It looks sort of old. Want to rebase first to make sure the latest tests
run?

On Mon, May 15, 2017 at 2:34 PM, Zachary Amsden 
wrote:

> https://gerrit.cloudera.org/#/c/6335/
>

Re: Should we change tests so they don't use single letter table names?

[Michael Brown]

Why not alter the frontend test to drop t if exists? Tests should generally
strive to set themselves up. Is there some trait of the frontend tests that
prevents that?

On Fri, May 12, 2017 at 4:38 AM, Lars Volker  wrote:

> Hi All,
>
> I frequently create test tables on my local system with names like "t" or
> "p". A couple of frontend tests use the same names and then fail with
> "Table already exists".
>
> Does anyone else hit this from time to time? Can we change the table names
> in the tests to avoid single letter names? If there are no objections, I'll
> open a JIRA.
>
> Thanks, Lars
>

impala.apache.org now hosting documentation

[Michael Brown]

With the resolution of IMPALA-3398, I'm pleased to announce that the Apache
Impala (incubating) Web site now self-hosts its documentation, here
http://impala.apache.org/impala-docs.html . Self-hosting documentation
should help grow the Impala community.

Many in our community helped with this effort, but I'd like to acknowledge
the following members who put in a lot of their time to make this happen:

Ambreen Kazi
Jim Apple
John Russell
Laurel Hale

Thanks to all!

wiki edit permissions request

[Michael Brown]

Hello,

Could someone give me permissions to edit the IMPALA space on
cwiki.apache.org?

Looks like my username for cwiki.apache.org is mikeb.

Thanks.

Re: Impala JIRA migration to https://issues.apache.org/jira is complete

[Michael Brown]

I expect such redirection to be a little difficult to configure. But it
seems like the same (or similar) JQL should work on issues.apache.org For
example:

https://issues.apache.org/jira/issues/?jql=type%20%3D%20bug%20and%20project%20%3D%20IMPALA%20AND%20resolution%20%3D%20fixed%20AND%20affectedVersion%20!%3D%20%22Impala%202.8.0%22%20AND%20fixVersion%20%3D%20%22Impala%202.8.0%22%20and%20not%20labels%20%3D%20broken-build%20order%20by%20priority%20desc


On Tue, Mar 21, 2017 at 12:47 PM, John Russell 
wrote:

> Just confirming: if I have a JIRA report URL, for example
>
> https://issues.cloudera.org/issues/?jql=type%20%3D%20bug%
> 20and%20project%20%3D%20IMPALA%20AND%20resolution%20%3D%20fixed%20AND%
> 20affectedVersion%20!%3D%20%22Impala%202.8.0%22%20AND%
> 20fixVersion%20%3D%20%22Impala%202.8.0%22%20and%
> 20not%20labels%20%3D%20broken-build%20order%20by%20priority%20desc
>
> should I expect that to redirect over to issues.apache.org also?
> Currently such a report stays on issues.cloudera.org.
>
> There are a bunch of such report links in the Impala release notes.
>
> Thanks,
> John
>
> > On Mar 13, 2017, at 2:24 PM, Jim Apple  wrote:
> >
> > If you read or write Impala JIRAs, this message is for you.
> >
> > Impala's JIRA is now at https://issues.apache.org/jira/browse/IMPALA/.
> Links like https://issues.cloudera.org/browse/IMPALA-3224 should now
> redirect to https://issues.apache.org/jira/browse/IMPALA-3224.
> >
> > Permissions on the Apache JIRA are much more strict. We have tried to
> make them almost as permissive for the IMPALA project as they were on the
> IMPALA project on issues.cloudera.org. If you need permissions you don't
> have, please email d...@impala.apache.org.
> >
> > Other than that, we don't know of any major issues at this point,
> however we have found and fixed some unexpected problems so far, and others
> may exist. If you find something that is impeding your work, please file a
> subtask of https://issues.apache.org/jira/browse/IMPALA-5064.
> >
> > We tried to translate usernames as accurately as possible. In many cases
> this worked just fine. In some cases this was not possible, and you will
> see the issues that you participated on as $OLDUSERNAME now list you as
> $OLDUSERNAME_impala_1de3, with some random-looking hex digits after your
> name. You can claim that username by resetting your password:
> >
> > 1. Go to https://issues.apache.org/jira/secure/ForgotLoginDetails.jspa
> > 2. Forgot Password, enter the username that looks like
> $OLDUSERNAME_impala_1ead
> > 3. Wait for email
> > 4. Reset password
> >
> > If you have problems with that process, please follow
> https://www.apache.org/dev/infra-contact and contact Apache
> infrastructure. The Impala community does not administer the JIRA server.
>
>

Re: jenkins.impala.io job queues disabled, restarting to apply security updates

[Michael Brown]

Jenkins is back up.

On Tue, Mar 21, 2017 at 8:30 AM, Michael Brown  wrote:

> There are a few security updates issued for plugins we use on
> jenkins.impala.io.
>
> I've disabled the ability to start new jobs. No jobs are running
> currently. I'll reply to this message when the update is complete.
>

jenkins.impala.io job queues disabled, restarting to apply security updates

[Michael Brown]

There are a few security updates issued for plugins we use on
jenkins.impala.io.

I've disabled the ability to start new jobs. No jobs are running currently.
I'll reply to this message when the update is complete.

jenkins.impala.io password reset request, thanks

[Michael Brown]

Re: test_ddl_stress likely not getting regular, automated runs

[Michael Brown]

I took a look at TestSpillStress and found a bunch of problems with
it. There's enough here that it merits a separate bug:

https://issues.cloudera.org/browse/IMPALA-4914

On Wed, Feb 8, 2017 at 3:45 PM, Tim Armstrong  wrote:
> For what it's worth there's a similar problem with TestSpillStress that I
> came across too. I was surprised when I realised too.
>
> On Wed, Feb 8, 2017 at 3:38 PM, Michael Brown  wrote:
>
>> Hello,
>>
>> Background: I am conducting an audit of metadata tests.
>>
>> A particular manifestation of IMPALA-3947 [0] is that the test module
>> test_ddl_stress.py isn't runnable via any buildall.sh option. I expect
>> this means test_ddl_stress.py isn't being run regularly except by
>> conscientious developers running it locally. Note that this test was
>> not disabled as part of the commit "IMPALA-2605: Omit the sort and
>> mini stress tests".
>>
>> I'm interested to know any history around this.
>>
>> Could any community members who have been around for several years,
>> especially those who work on the Catalog, shed light on this? Were you
>> assuming test_ddl_stress.py was being run regularly? Or do you know to
>> run it by hand? Do you know it to be reliable and useful, and it
>> should be included in regular runs, or do you know it to be flaky and
>> needing work?
>>
>> It's fine if no one knows any such answers, but they would lend
>> context to getting test_ddl_stress running regularly.
>>
>> Thanks.
>>
>> [0] https://issues.cloudera.org/browse/IMPALA-3947
>>

Re: test_ddl_stress likely not getting regular, automated runs

[Michael Brown]

Thanks for letting me know Alex!

On Thu, Feb 9, 2017 at 4:58 PM, Alex Behm  wrote:
> I think we expected this test to run regularly, at least in exhaustive mode.
>
> Looks like I was unaware of this workload core/exhaustive mess and disabled
> that test in this commit: 992c261d
>
> Certainly was not on purpose.
>
> On Wed, Feb 8, 2017 at 3:45 PM, Tim Armstrong 
> wrote:
>
>> For what it's worth there's a similar problem with TestSpillStress that I
>> came across too. I was surprised when I realised too.
>>
>> On Wed, Feb 8, 2017 at 3:38 PM, Michael Brown  wrote:
>>
>> > Hello,
>> >
>> > Background: I am conducting an audit of metadata tests.
>> >
>> > A particular manifestation of IMPALA-3947 [0] is that the test module
>> > test_ddl_stress.py isn't runnable via any buildall.sh option. I expect
>> > this means test_ddl_stress.py isn't being run regularly except by
>> > conscientious developers running it locally. Note that this test was
>> > not disabled as part of the commit "IMPALA-2605: Omit the sort and
>> > mini stress tests".
>> >
>> > I'm interested to know any history around this.
>> >
>> > Could any community members who have been around for several years,
>> > especially those who work on the Catalog, shed light on this? Were you
>> > assuming test_ddl_stress.py was being run regularly? Or do you know to
>> > run it by hand? Do you know it to be reliable and useful, and it
>> > should be included in regular runs, or do you know it to be flaky and
>> > needing work?
>> >
>> > It's fine if no one knows any such answers, but they would lend
>> > context to getting test_ddl_stress running regularly.
>> >
>> > Thanks.
>> >
>> > [0] https://issues.cloudera.org/browse/IMPALA-3947
>> >
>>

test_ddl_stress likely not getting regular, automated runs

[Michael Brown]

Hello,

Background: I am conducting an audit of metadata tests.

A particular manifestation of IMPALA-3947 [0] is that the test module
test_ddl_stress.py isn't runnable via any buildall.sh option. I expect
this means test_ddl_stress.py isn't being run regularly except by
conscientious developers running it locally. Note that this test was
not disabled as part of the commit "IMPALA-2605: Omit the sort and
mini stress tests".

I'm interested to know any history around this.

Could any community members who have been around for several years,
especially those who work on the Catalog, shed light on this? Were you
assuming test_ddl_stress.py was being run regularly? Or do you know to
run it by hand? Do you know it to be reliable and useful, and it
should be included in regular runs, or do you know it to be flaky and
needing work?

It's fine if no one knows any such answers, but they would lend
context to getting test_ddl_stress running regularly.

Thanks.

[0] https://issues.cloudera.org/browse/IMPALA-3947

carry + 2 / GVO request

[Michael Brown]

https://gerrit.cloudera.org/#/c/5795/

Thanks.

Re: Python string formatting

[Michael Brown]

I agree.

On Tue, Jan 31, 2017 at 10:44 AM, Lars Volker  wrote:
> Thanks for the feedback here and in the review.
>
> I agree that we should aim for a style as close to PEP8 as possible.
> However, I also didn't want to overshoot and my first goal was to get some
> useful tooling set up, so that I don't have to constantly worry about
> formatting. Once I had figured out some tooling, I thought I might as well
> share it and solicit feedback.
>
> Regarding the next steps, I'm open for anything really. I didn't know about
> the --diff switch of flake8, that looks very useful. Even better of course
> would be, if all python code could be converted to PEP8.
>
> Here is a list of all PEP8 violations and their count, obtained with
> "pycodestyle --statistics -qq tests":
>
> 9017E111 indentation is not a multiple of four
> 902 E114 indentation is not a multiple of four (comment)
> 2   E116 unexpected indentation (comment)
> 24  E122 continuation line missing indentation or outdented
> 5   E124 closing bracket does not match visual indentation
> 105 E125 continuation line with same indent as next logical line
> 43  E127 continuation line over-indented for visual indent
> 1038E128 continuation line under-indented for visual indent
> 7   E131 continuation line unaligned for hanging indent
> 13  E201 whitespace after '('
> 8   E202 whitespace before ']'
> 55  E203 whitespace before ':'
> 5   E211 whitespace before '['
> 5   E221 multiple spaces before operator
> 7   E222 multiple spaces after operator
> 9   E225 missing whitespace around operator
> 1   E227 missing whitespace around bitwise or shift operator
> 127 E231 missing whitespace after ':'
> 157 E251 unexpected spaces around keyword / parameter equals
> 20  E261 at least two spaces before inline comment
> 21  E265 block comment should start with '# '
> 1   E266 too many leading '#' for block comment
> 1   E271 multiple spaces after keyword
> 4   E301 expected 1 blank line, found 0
> 313 E302 expected 2 blank lines, found 1
> 16  E303 too many blank lines (2)
> 13  E305 expected 2 blank lines after class or function definition,
> found 1
> 6   E306 expected 1 blank line before a nested definition, found 0
> 7   E402 module level import not at top of file
> 3800E501 line too long (80 > 79 characters)
> 278 E502 the backslash is redundant between brackets
> 87  E701 multiple statements on one line (colon)
> 74  E703 statement ends with a semicolon
> 12  E711 comparison to None should be 'if cond is None:'
> 9   E712 comparison to False should be 'if cond is False:' or 'if not
> cond:'
> 2   E713 test for membership should be 'not in'
> 2   E741 ambiguous variable name 'l'
> 1   W292 no newline at end of file
> 9   W391 blank line at end of file
> 2   W601 .has_key() is deprecated, use 'in'
> 19  W602 deprecated form of raising exception
>
> If we take out the well known ones (indent, line width), it does not look
> too far fetched to me to change it all to PEP8.
>
> Thoughts?
>
>
>
> On Tue, Jan 31, 2017 at 5:59 PM, Michael Brown  wrote:
>
>> Thanks. I made some comments on the review, but I see now I should
>> probably share my general view here.
>>
>> My general view is, if we are going to codify our Python style guide,
>> I would rather we codify style conventions that are closer to standard
>> Python style conventions, rather than codify what is currently done. I
>> am willing to keep 2-space indents and 90-char lines, but I don't
>> think anything else should be part of the conventions when those
>> conventions involves ignoring PEP-008. My instinct tells me the Python
>> conventions weren't conventions at all, but came up organically
>> without regard to actually reading conventions or using tooling.
>> Otherwise, we'd have already had a Python style guide, right?
>>
>> If the concern is "But there are too many noisy errors if I am editing
>> an existing, large file, so we should ignore these anyway", something
>> like this is possible:
>>
>> git diff | flake8 --diff
>>
>> This will only show PEP-008 problems on changed code, not whole files.
>>
>>
>>
>> On Mon, Jan 30, 2017 at 3:20 PM, Lars Volker  wrote:
>> > Cool, thanks Michael for the reply. I added a section on Python to the
>> Impala
>> > Style Guide
>> > <https://cwiki.apa

Re: Python string formatting

[Michael Brown]

Thanks. I made some comments on the review, but I see now I should
probably share my general view here.

My general view is, if we are going to codify our Python style guide,
I would rather we codify style conventions that are closer to standard
Python style conventions, rather than codify what is currently done. I
am willing to keep 2-space indents and 90-char lines, but I don't
think anything else should be part of the conventions when those
conventions involves ignoring PEP-008. My instinct tells me the Python
conventions weren't conventions at all, but came up organically
without regard to actually reading conventions or using tooling.
Otherwise, we'd have already had a Python style guide, right?

If the concern is "But there are too many noisy errors if I am editing
an existing, large file, so we should ignore these anyway", something
like this is possible:

git diff | flake8 --diff

This will only show PEP-008 problems on changed code, not whole files.



On Mon, Jan 30, 2017 at 3:20 PM, Lars Volker  wrote:
> Cool, thanks Michael for the reply. I added a section on Python to the Impala
> Style Guide
> <https://cwiki.apache.org/confluence/display/IMPALA/Impala+Style+Guide>.
> Please feel free to edit it or let me know if I should make changes. I will
> also send out a review to add a .pep8rc file to the repository.
>
> On Fri, Jan 27, 2017 at 11:56 PM, Michael Brown  wrote:
>
>> I prefer str.format() over the % operator, because:
>>
>> https://docs.python.org/2.7/library/stdtypes.html#str.format
>>
>> "This method of string formatting is the new standard in Python 3, and
>> should be preferred to the % formatting described in String Formatting
>> Operations in new code."
>>
>> Without an Impala Python style guide, I tend to use what I see on
>> docs.python.org, modulo our 2-space indent and 90-char line policy.
>>
>>
>> On Fri, Jan 27, 2017 at 2:44 PM, Lars Volker  wrote:
>> > Hi All,
>> >
>> > do we have a strong preference for either old style or new style string
>> > formatting in Python?
>> >
>> > "Hello %s!" % ("world") *vs* "Hello {0}!".format("world")
>> >
>> > The Impala Style Guide
>> > <https://cwiki.apache.org/confluence/display/IMPALA/Impala+Style+Guide>
>> doesn't
>> > mention Python at all.
>> >
>> > Thanks, Lars
>>

Re: Standards for committers and PPMC members

[Michael Brown]

I apologize for dropping the ball on this.

> Would it help to have examples of candidates L, M, N, O, and P who focus on 
> testing tools and infrastructure?

Yes.

On Wed, Jan 11, 2017 at 1:50 PM, Jim Apple  wrote:
> Do you have any thoughts about what specific type or format of
> feedback would help make it less of a black box? Would it help to have
> examples of candidates L, M, N, O, and P who focus on testing tools
> and infrastructure?
>
> On Wed, Jan 11, 2017 at 1:14 PM, Michael Brown  wrote:
>>> What do you think, Michael?
>>
>> Thanks to you, Tim, and Todd for your thoughts. It still feels like a black
>> box, especially for those of us who tend to concentrate on testing tools
>> and infrastructure for Impala. Any feedback is appreciated.
>>
>> On Fri, Jan 6, 2017 at 8:53 AM, Jim Apple  wrote:
>>
>>> My feeling is similar to Tim's:
>>>
>>> It's the PPMC's responsibility, but a contributor is welcome to plead
>>> their case, ask for a mentor, and so on. I think we shouldn't consider
>>> it rude or pushy or aggressive to request committership. It is a
>>> compliment to Impala and the Impala community that the contributor
>>> want to be more involved.
>>>
>>> What do you think, Michael?
>>>
>>> On Fri, Jan 6, 2017 at 8:36 AM, Tim Armstrong 
>>> wrote:
>>> > Hi Michael,
>>> >   My two cents is that the PMC should be proactive about identifying
>>> > potential committers and working with them to address any gaps. We
>>> haven't
>>> > done a good job of that so far but we've started up some discussions on
>>> the
>>> > private list to get better at that.
>>> >
>>> > You should feel free to ask anyone on the PMC about any of the above
>>> > questions. Ideally that wouldn't be necessary, but in practice it may
>>> help
>>> > move things along, particularly if you have someone who will advocate for
>>> > you and wrangle the PMC to come to a consensus. It's definitely on us to
>>> > communicate to you what gaps (if any) there are - it shouldn't really be
>>> a
>>> > black box.
>>> >
>>> > - Tim
>>> >
>>> > On Fri, Jan 6, 2017 at 8:24 AM, Michael Brown 
>>> wrote:
>>> >
>>> >> You've done a great job highlighting some example scenarios. Here are
>>> some
>>> >> questions that aren't addressed in your writeup.
>>> >>
>>> >> What are contributors' responsibilities to move toward committership? In
>>> >> particular, I'm talking about process, not the nuts and bolts of
>>> >> contributions (including patches, bugs, reviews).  For example:
>>> >>
>>> >> Should a contributor who wants to be a committer find a "mentor"?
>>> >>
>>> >> Should a contributor who wants to be a committer be lobbying for
>>> >> committership to someone who has reviewed his patches, or dealt with
>>> bugs
>>> >> he's filed, or otherwise interacted with?
>>> >>
>>> >> Should a contributor nominate himself on this list? Must he cite
>>> examples
>>> >> of his contributions?
>>> >>
>>> >> How can a contributor who wants to be a committer receive good feedback
>>> for
>>> >> areas of improvement if his committership is rejected?
>>> >>
>>> >>
>>> >> On Thu, Jan 5, 2017 at 1:41 PM, Jim Apple  wrote:
>>> >>
>>> >> > I think it would be helpful to non-committer contributors (and non-PMC
>>> >> > committers (just me right now)) if PPMC members would muse a bit about
>>> >> > what they believe the bar is for committership or PPMC membership.
>>> >> >
>>> >> > I am not suggesting that the PPMC write a document with so much detail
>>> >> > that you are hamstrung when looking at contributors in the future and
>>> >> > decising if they did 6 hard code reviews and 5 medium or 7 hard code
>>> >> > reviews and 4 medium ones.
>>> >> >
>>> >> > However, multiple people have pinged me asking how to become a
>>> >> > committer, asking what work products are sufficient.
>>> >> >
>>> >> > I don't have a foolproof way of describing the possible bars, so let
>

Re: Python string formatting

[Michael Brown]

I prefer str.format() over the % operator, because:

https://docs.python.org/2.7/library/stdtypes.html#str.format

"This method of string formatting is the new standard in Python 3, and
should be preferred to the % formatting described in String Formatting
Operations in new code."

Without an Impala Python style guide, I tend to use what I see on
docs.python.org, modulo our 2-space indent and 90-char line policy.


On Fri, Jan 27, 2017 at 2:44 PM, Lars Volker  wrote:
> Hi All,
>
> do we have a strong preference for either old style or new style string
> formatting in Python?
>
> "Hello %s!" % ("world") *vs* "Hello {0}!".format("world")
>
> The Impala Style Guide
>  
> doesn't
> mention Python at all.
>
> Thanks, Lars

Re: Need help pushing my first gerrit review

[Michael Brown]

Make sure your git remote URL contains your Github username. "git
remote show asf-gerrit" should show:

ssh://your_github_username_goes_h...@gerrit.cloudera.org:29418/Impala-ASF

If you hadn't done this before, an export on the command line won't
fix the remote. You would need something like "git remote set-url" to
fix that (man git-remote for details).

Make sure the SSH private key corresponding to the public key you have
uploaded to Gerrit has been added to your SSH agent, or that OS X
keychain thing maybe? (I don't know how the latter works.) In the
ssh-agent case, "ssh-add -l" should print at least one entry, with a
file name corresponding to your private SSH key as I described above.
Man ssh-add for details.

It's weird to me that you say your known_hosts entry corresponds to
the key you uploaded to Gerrit. known_hosts contains mappings of
hostnames/IP addresses and SSH server public keys. That shouldn't be
the public key that you upload to Gerrit. This list quashes images, so
we can't see the image you included. If it's neither of the above
things, is there a chance you uploaded the wrong thing to Gerrit?


On Wed, Jan 18, 2017 at 8:58 PM, Laurel Hale  wrote:
> I just tried that and received the same error:
> ---
> [laurel@laurel-MBP incubator-impala](remove_cm_from_some_titles)$ git status
> On branch remove_cm_from_some_titles
> nothing to commit, working tree clean
> [laurel@laurel-MBP incubator-impala](remove_cm_from_some_titles)$ export
> GERRIT_USER=laurelh
> [laurel@laurel-MBP incubator-impala](remove_cm_from_some_titles)$ git push
> --no-thin asf-gerrit HEAD:refs/for/master
> Permission denied (publickey).
> fatal: Could not read from remote repository.
>
> Please make sure you have the correct access rights and the repository
> exists.
>
> 
>
> Do you think that it might have something to do with the fact that I don't
> have permissions to commit to master? Or do you think I failed to complete
> all of my gerrit setup? (The process was very fragmented and I could have
> left out installing the gerrit pre-commit hook. Could that be it?)
>
>
> Thank you very much,
>
> Laurel
>
> On Wed, Jan 18, 2017 at 8:47 PM, Alex Behm  wrote:
>
>> Maybe your GERRIT_USER is from your OS user? You might have to "export
>> GERRIT_USER=your_gerrir_username" before you try to push.
>>
>> On Wed, Jan 18, 2017 at 8:44 PM, Laurel Hale  wrote:
>>
>> > Hi,
>> > I finally got time to push a review to gerrit today and got the following
>> > error:
>> > 
>> > [laurel@laurel-MBP incubator-impala](remove_cm_from_some_titles)$ git
>> > status
>> > On branch remove_cm_from_some_titles
>> > nothing to commit, working tree clean
>> > [laurel@laurel-MBP incubator-impala](remove_cm_from_some_titles)$ git
>> > push --no-thin asf-gerrit HEAD:refs/for/master
>> > Permission denied (publickey).
>> > fatal: Could not read from remote repository.
>> >
>> > Please make sure you have the correct access rights
>> > and the repository exists.
>> > --
>> >
>> > I checked my ~/.ssh/known_hosts file and it seems to include a pointer to
>> > gerrit:
>> >
>> > [gerrit.cloudera.org]:29418,[75.101.130.251]:29418 ssh-rsa
>> > B3NzaC1yc2EDAQABAAABAQDVszOqZdQgRfqhii0gkwQnJJPtLmkc
>> > VRXDr553L/xOGrjrCrF5sT0oM+UGqXg1ykxmMv7oEUZbf3AiBonyTQ4CQ71l
>> > +nxbTCm97r+bo+AprhN8ybi0/gEsj5RtyDxNQk9M+wBZI8gje5Zj6f24sbPg
>> > PsmR1LOG5rWhz8s8VHL/ZMAad9veB2xEDGrDzhc4OD6nVBn4XbFA2XEPt9qf
>> > pvBgctKNcukfVeoe4d/o5OieRhcDZwmBhou+medqUVuEdg0DiORjMvHHQtpM
>> > svRBt/JSIehctCVgzrEKA1dD3XzL27cuR7v7QlYx97zGtPx4l5dhSc+Z19g5BxD74P8GNvA3
>> >
>> > which corresponds to the public key I uploaded to gerrit:
>> >
>> >
>> >
>> >
>> > Any ideas of what I am doing wrong?
>> >
>> > Thanks,
>> >
>> > Laurel
>> >
>>

Re: need verify/submit from a committer

[Michael Brown]

The first has been taken care of. The second (
https://gerrit.cloudera.org/#/c/5486/) has just been rebased.

Could a committer:

1. carry the +2
2. start GVO

Thanks!


On Wed, Jan 11, 2017 at 8:33 AM, Michael Brown  wrote:

> Hello,
>
> These two reviews have been approved, but the changes wouldn't be
> exercised by running GVO. Could a committer please cherry-pick them?
>
> https://gerrit.cloudera.org/#/c/5387/
> https://gerrit.cloudera.org/#/c/5486/
>
> The second is based on the first (the second's parent commit is the first
> patch set). I believe when the first is cherry-picked, the second will no
> longer say "cannot merge". If something looks suspicious with the second
> after the first is cherry-picked, let me know and I'll rebase it to clean
> it up.
>
> Thank you.
>

Re: Standards for committers and PPMC members

[Michael Brown]

> What do you think, Michael?

Thanks to you, Tim, and Todd for your thoughts. It still feels like a black
box, especially for those of us who tend to concentrate on testing tools
and infrastructure for Impala. Any feedback is appreciated.

On Fri, Jan 6, 2017 at 8:53 AM, Jim Apple  wrote:

> My feeling is similar to Tim's:
>
> It's the PPMC's responsibility, but a contributor is welcome to plead
> their case, ask for a mentor, and so on. I think we shouldn't consider
> it rude or pushy or aggressive to request committership. It is a
> compliment to Impala and the Impala community that the contributor
> want to be more involved.
>
> What do you think, Michael?
>
> On Fri, Jan 6, 2017 at 8:36 AM, Tim Armstrong 
> wrote:
> > Hi Michael,
> >   My two cents is that the PMC should be proactive about identifying
> > potential committers and working with them to address any gaps. We
> haven't
> > done a good job of that so far but we've started up some discussions on
> the
> > private list to get better at that.
> >
> > You should feel free to ask anyone on the PMC about any of the above
> > questions. Ideally that wouldn't be necessary, but in practice it may
> help
> > move things along, particularly if you have someone who will advocate for
> > you and wrangle the PMC to come to a consensus. It's definitely on us to
> > communicate to you what gaps (if any) there are - it shouldn't really be
> a
> > black box.
> >
> > - Tim
> >
> > On Fri, Jan 6, 2017 at 8:24 AM, Michael Brown 
> wrote:
> >
> >> You've done a great job highlighting some example scenarios. Here are
> some
> >> questions that aren't addressed in your writeup.
> >>
> >> What are contributors' responsibilities to move toward committership? In
> >> particular, I'm talking about process, not the nuts and bolts of
> >> contributions (including patches, bugs, reviews).  For example:
> >>
> >> Should a contributor who wants to be a committer find a "mentor"?
> >>
> >> Should a contributor who wants to be a committer be lobbying for
> >> committership to someone who has reviewed his patches, or dealt with
> bugs
> >> he's filed, or otherwise interacted with?
> >>
> >> Should a contributor nominate himself on this list? Must he cite
> examples
> >> of his contributions?
> >>
> >> How can a contributor who wants to be a committer receive good feedback
> for
> >> areas of improvement if his committership is rejected?
> >>
> >>
> >> On Thu, Jan 5, 2017 at 1:41 PM, Jim Apple  wrote:
> >>
> >> > I think it would be helpful to non-committer contributors (and non-PMC
> >> > committers (just me right now)) if PPMC members would muse a bit about
> >> > what they believe the bar is for committership or PPMC membership.
> >> >
> >> > I am not suggesting that the PPMC write a document with so much detail
> >> > that you are hamstrung when looking at contributors in the future and
> >> > decising if they did 6 hard code reviews and 5 medium or 7 hard code
> >> > reviews and 4 medium ones.
> >> >
> >> > However, multiple people have pinged me asking how to become a
> >> > committer, asking what work products are sufficient.
> >> >
> >> > I don't have a foolproof way of describing the possible bars, so let
> >> > me give a few examples for feedback from the PPMC.
> >> >
> >> > +
> >> > Potential committers:
> >> >
> >> > Alice started contributing 4 months ago. She fixes at least one style
> >> > issue or typo every weekend.
> >> >
> >> > Bob started contributing a year ago. We uses Impala to organize his
> >> > VHS collection, and he regularly reports scaling bugs as his
> >> > collection grows to more and more impalad nodes. His reports are often
> >> > out of date, since he runs an old Impala, but some are still bugs in
> >> > the latest version. His bug reports are of very high quality.
> >> >
> >> > Carol started contributing six months ago. She helped design one
> >> > tricky feature. It took her six months and 27 revisions to get the
> >> > patch in. She also helps other users a lot with their issues.
> >> >
> >> > Dave has been contributing for 18 months. He helped design a tricky
&g

Re: need verify/submit from a committer

[Michael Brown]

Correct, it's 1 and 2. To be clear, I'm not proposing to cherry-pick those
reviews without having done any testing. The commit message shows the
testing involved.

Are we interested in adding "tests-for-tests" into our regular runs?

On Wed, Jan 11, 2017 at 9:21 AM, Jim Apple  wrote:

> If these don't get exercised by the pre-merge test, why not run them
> through there anyway?
>
> I can think of a couple of reasons, but I'm wondering if there's
> something else other than (1) resource use (2) flaky tests cause
> spurious failures sometimes?
>
> On Wed, Jan 11, 2017 at 8:33 AM, Michael Brown  wrote:
> > Hello,
> >
> > These two reviews have been approved, but the changes wouldn't be
> exercised
> > by running GVO. Could a committer please cherry-pick them?
> >
> > https://gerrit.cloudera.org/#/c/5387/
> > https://gerrit.cloudera.org/#/c/5486/
> >
> > The second is based on the first (the second's parent commit is the first
> > patch set). I believe when the first is cherry-picked, the second will no
> > longer say "cannot merge". If something looks suspicious with the second
> > after the first is cherry-picked, let me know and I'll rebase it to clean
> > it up.
> >
> > Thank you.
>

need verify/submit from a committer

[Michael Brown]

Hello,

These two reviews have been approved, but the changes wouldn't be exercised
by running GVO. Could a committer please cherry-pick them?

https://gerrit.cloudera.org/#/c/5387/
https://gerrit.cloudera.org/#/c/5486/

The second is based on the first (the second's parent commit is the first
patch set). I believe when the first is cherry-picked, the second will no
longer say "cannot merge". If something looks suspicious with the second
after the first is cherry-picked, let me know and I'll rebase it to clean
it up.

Thank you.

Re: Standards for committers and PPMC members

[Michael Brown]

You've done a great job highlighting some example scenarios. Here are some
questions that aren't addressed in your writeup.

What are contributors' responsibilities to move toward committership? In
particular, I'm talking about process, not the nuts and bolts of
contributions (including patches, bugs, reviews).  For example:

Should a contributor who wants to be a committer find a "mentor"?

Should a contributor who wants to be a committer be lobbying for
committership to someone who has reviewed his patches, or dealt with bugs
he's filed, or otherwise interacted with?

Should a contributor nominate himself on this list? Must he cite examples
of his contributions?

How can a contributor who wants to be a committer receive good feedback for
areas of improvement if his committership is rejected?


On Thu, Jan 5, 2017 at 1:41 PM, Jim Apple  wrote:

> I think it would be helpful to non-committer contributors (and non-PMC
> committers (just me right now)) if PPMC members would muse a bit about
> what they believe the bar is for committership or PPMC membership.
>
> I am not suggesting that the PPMC write a document with so much detail
> that you are hamstrung when looking at contributors in the future and
> decising if they did 6 hard code reviews and 5 medium or 7 hard code
> reviews and 4 medium ones.
>
> However, multiple people have pinged me asking how to become a
> committer, asking what work products are sufficient.
>
> I don't have a foolproof way of describing the possible bars, so let
> me give a few examples for feedback from the PPMC.
>
> +
> Potential committers:
>
> Alice started contributing 4 months ago. She fixes at least one style
> issue or typo every weekend.
>
> Bob started contributing a year ago. We uses Impala to organize his
> VHS collection, and he regularly reports scaling bugs as his
> collection grows to more and more impalad nodes. His reports are often
> out of date, since he runs an old Impala, but some are still bugs in
> the latest version. His bug reports are of very high quality.
>
> Carol started contributing six months ago. She helped design one
> tricky feature. It took her six months and 27 revisions to get the
> patch in. She also helps other users a lot with their issues.
>
> Dave has been contributing for 18 months. He helped design a tricky
> feature, too, but his code was not high quality enough to check in. He
> did document the feature while a PPMC member wrote the code. Since
> then, he's been helping users on the mailing lists and filing UI bugs,
> especially with the REPL.
>
> Eve used to contribute before Impala was with Apache, and she was not
> listed as a committer/PPMC member when incubation started. Since then,
> she does code reviews, only commenting on style issues. She does 3 or
> 4 a month.
>
> Frank has been contributing for three months. He writes 3-4 patches
> every weekend. They are all tests, query generation, or
> impala-shell.sh work, and they are almost uniformly high-quality.
>
> My personal feelings: Yes on Bob, Carol, Eve, and Frank. Alice is not
> on track. Dave is on track but should do more design work and doc
> writing.
>
> +
> Potential PPMC members, all of which are already committers.
>
> Gertrude has been a contributor for 18 months. She spends most of her
> efforts on backend performance in-the-small - a few microops saved per
> row per patch. She helps review patches in this area. She doesn't
> participate much on governance.
>
> Harold has been a contributor for a 30 months. He works exclusively on
> performance, but he writes very little code. All of his effort is
> devoted to understanding Impala performance issues, which he writes
> and and files as high quality bug reports. He does not review code and
> he does not write code or documentation. He participates in discussion
> and consensus-building on design.
>
> Imelda has been a contributor for 12 months. She also does not write
> code. She is focused only on community outreach, writing blog posts
> and doing the simplest code reviews for her recruits to the project.
> She posts or gets a new contributor once a month.
>
> Jules has been a contributor for 40 months. He only reviews code, but
> he gives outstanding reviews of both design and style. He managed two
> releases last year.
>
> Kim has been a contributor for 55 months. She used to write a lot of
> code but now she is focused on keeping infrastructure ship-shape,
> mainly flaky test fixing and Jenkins wrangling. She rarely votes.
>
> My personal feelings: No on Gertrude and Kim, yes on Harold, Imelda,
> and Jules. G+K may be outstanding committers and members, but are not
> on track for PPMC membership. However, they could get on track very
> easily by focusing some small part of their effort on governance work.
>
> +++
>
> BTW, if you don't know if you already

Re: random query generator patch submit request

[Michael Brown]

Thanks!

While you're at it, asf/master needs to be brought up to date with
asf_gerrit/master:

* 585ed5a - (asf_gerrit/master) IMPALA-4450: qgen: use string concatenation
operator for postgres queries (7 minutes ago) 
* 0f62bf3 - IMPALA-4550: Fix CastExpr analysis for substituted slots (5
hours ago) 
* 9f497ba - IMPALA-2890: Support ALTER TABLE statements for Kudu tables (12
hours ago) 
* 90bf40d - IMPALA-4553: ntpd must be synchronized for kudu to start. (16
hours ago) 
* 2680580 - (asf/master) IMPALA-4512: Add a script that builds Impala on
stock Ubuntu 14.04. (18 hours ago) 


On Wed, Nov 30, 2016 at 8:23 AM, Sailesh Mukil  wrote:

> Done.
>
> On Wed, Nov 30, 2016 at 8:07 AM, Michael Brown  wrote:
>
> > Hello,
> >
> > This is a patch to the random query generator, not affected by GVO:
> >
> > https://gerrit.cloudera.org/#/c/5034/
> >
> > Since GVO doesn't touch this code path, and Jenkins executors are
> precious,
> > could a Committer please verify/submit this change directly?
> >
> > Thanks
> >
>

random query generator patch submit request

[Michael Brown]

Hello,

This is a patch to the random query generator, not affected by GVO:

https://gerrit.cloudera.org/#/c/5034/

Since GVO doesn't touch this code path, and Jenkins executors are precious,
could a Committer please verify/submit this change directly?

Thanks

Impala CONCAT() behavior vs. other databases

[Michael Brown]

Hello,

While using the Impala random query generator to compare Impala query
results with PostgreSQL query results, I noticed a discrepancy between
the CONCAT() functions. I'm looking for information from the community
whether the discrepancy I found is intentional (in which case I will
fix the random query generator) or unintentional (in which case I will
file an Impala defect Jira). I couldn't find bugs about this, though I
did see IMPALA-452, but IMPALA-452 doesn't explicitly call out
behavior discrepancies or preferences.

If Impala CONCAT() has an argument that evaluates to NULL, then Impala
CONCAT() returns NULL.

If PostgreSQL CONCAT() has an argument that evaluates to NULL, then
PostgreSQL will ignore it, and treat it as an empty string. Even
CONCAT(NULL) evaluates to the empty string.

PostgreSQL has a || operator that behaves like Impala CONCAT(): if a
NULL expression is on one side of ||, that || evals to NULL. (Aside:
the || operator in Impala appears to be an alias for OR, though that
seems to be undocumented. IMPALA-452 suggests this is a Hive mimic.)

I also checked Oracle (XE 11.2.0.2.0). Oracle also has a CONCAT()
function and a string concatenation operator ||. They are equivalent.
There is a difference between Oracle behavior and other databases: a
CONCAT() or || with only nulls or empty strings evaluates to NULL.
That's somewhat different edge behavior, but the CONCAT('something',
NULL) case does mimic PostgreSQL CONCAT(), not Impala CONCAT().

If Impala CONCAT() is meant to behave like PostgreSQL ||, then I can
change the random query generator to write the correct SQL dialect
from the same logical query to reduce false positive discrepancies. If
Impala CONCAT() is meant to behave like PostgreSQL CONCAT(), then
there's a defect. Please let me know the intent so I can act
accordingly.

Thanks!

Re: Unable to start Hive with testdata.bin/run-all.sh

[Michael Brown]

That email (and change) has been a big help.

'./buildall.sh -noclean -notests -start_minicluster
-start_impala_cluster' has worked very well for me.

On Wed, Oct 26, 2016 at 10:43 AM, Tim Armstrong  wrote:
> Sorry about this - I sent out an email earlier titled "Please read if you
> use ./testdata/bin/run-all.sh as part of your workflow" with some
> explanation/steps.
>
> Either you can run ./bin/create-test-configuration.sh then run-all.sh or
> just ./buildall.sh -start_minicluster
>
> On Wed, Oct 26, 2016 at 10:36 AM, Jim Apple  wrote:
>
>> I am seeing the error below when trying to get my development
>> environment up and running. Has anyone else seen and worked around
>> this before?
>>
>> 2016-10-26 10:27:16,719 ERROR Datastore.Schema
>> (Log4JLogger.java:error(125)) - Failed initialising database.
>> Unable to open a test connection to the given database. JDBC url =
>> jdbc:derby:;databaseName=metastore_db;create=true, username = APP.
>> Terminating connection pool (set lazyInit to true if you expect to
>> start your database after your app). Original Exception: --
>> java.sql.SQLException: Failed to start database 'metastore_db' with
>> class loader sun.misc.Launcher$AppClassLoader@3ad6a0e0, see the next
>> exception for details.
>> at org.apache.derby.impl.jdbc.SQLExceptionFactory.getSQLExcepti
>> on(Unknown
>> Source)
>> at org.apache.derby.impl.jdbc.SQLExceptionFactory.getSQLExcepti
>> on(Unknown
>> Source)
>> at org.apache.derby.impl.jdbc.Util.seeNextException(Unknown
>> Source)
>> at org.apache.derby.impl.jdbc.EmbedConnection.bootDatabase(Unknown
>> Source)
>> at org.apache.derby.impl.jdbc.EmbedConnection.(Unknown
>> Source)
>> [MANY MORE LINES]
>> at org.apache.hadoop.util.RunJar.main(RunJar.java:136)
>> Caused by: ERROR XJ040: Failed to start database 'metastore_db' with
>> class loader sun.misc.Launcher$AppClassLoader@3ad6a0e0, see the next
>> exception for details.
>> at org.apache.derby.iapi.error.StandardException.newException(
>> Unknown
>> Source)
>> at org.apache.derby.impl.jdbc.SQLExceptionFactory.wrapArgsForTr
>> ansportAcrossDRDA(Unknown
>> Source)
>> ... 84 more
>> Caused by: ERROR XSDB6: Another instance of Derby may have already
>> booted the database /home/jbapple/Impala/metastore_db.
>> at org.apache.derby.iapi.error.StandardException.newException(
>> Unknown
>> Source)
>> at org.apache.derby.iapi.error.StandardException.newException(
>> Unknown
>> Source)
>> [MANY MORE LINES]
>>

[Toolchain-CR] Versioning of build artifacts

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: Versioning of build artifacts
..


Patch Set 2: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb8774a7bd4bcae0c135684078f5ce89a28f6bc2
Gerrit-PatchSet: 2
Gerrit-Project: Toolchain
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Toolchain-CR] Versioning of build artifacts

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: Versioning of build artifacts
..


Patch Set 2:

(2 comments)

Nice idea.

http://gerrit.cloudera.org:8080/#/c/4742/2/buildall.sh
File buildall.sh:

PS2, Line 69: (
Out of curiosity, any reason you put LLVM building in a subshell, but nothing 
else?


http://gerrit.cloudera.org:8080/#/c/4742/2/functions.sh
File functions.sh:

PS2, Line 428:   echo "${UNIQUE_ID}-${GIT_HASH}"
Nit or maybe my preference only: put git hash first?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb8774a7bd4bcae0c135684078f5ce89a28f6bc2
Gerrit-PatchSet: 2
Gerrit-Project: Toolchain
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-2013: Reintroduce steps for checking HBase health in run-hbase.sh

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-2013: Reintroduce steps for checking HBase health in 
run-hbase.sh
..


Patch Set 5:

(9 comments)

Under what circumstances is more than one zookeeper needed?

http://gerrit.cloudera.org:8080/#/c/4348/5//COMMIT_MSG
Commit Message:

PS5, Line 11: The original script was problematic for a couple of reasons (most
: notably because it queried the wrong Zookeeper node) so we stopped
: using it during our mini-cluster HBase start up procedure.
Note quite right. I didn't use it because it wasn't clear it was needed 
anymore, the so-called "HBase race" had been fixed. I didn't notice the master 
vs. rs discrepancy at that time.


http://gerrit.cloudera.org:8080/#/c/4348/5/testdata/bin/check-hbase-nodes.py
File testdata/bin/check-hbase-nodes.py:

PS5, Line 50: parser.add_argument('--timeout', '-t', action='store'
I find action='store' to be redundant since that's the default.


PS5, Line 52:   'Default is 30 seconds.'))
Don't hardcode 30. Use a format string and read from TIMEOUT_SECONDS.


PS5, Line 56:   'e.g, 0.0.0.0:5070. Default is 
localhost:5070.'))
Use a format string and read from HDFS_HOST.


PS5, Line 60:   'e.g, 0.0.0.0:2181. Default is 
localhost:2181.'))
Use a format string and read from ZK_HOSTS.


PS5, Line 66:   '/hbase/master and /hbase/rs.')
Use a format string. Maybe use '-n '.join(HBASE_NODES)  as part of it?


PS5, Line 127: hdfs_client.list('/')
What does this do?


PS5, Line 125: try:
 : hdfs_client = InsecureClient('http://' + args.hdfs_host)
 : hdfs_client.list('/')
 : except requests.exceptions.ConnectionError as e:
 : msg = 'Could not connect to HDFS web host http://{0} - 
{1}'.format(args.hdfs_host, e)
 : LOGGER.error(msg)
 : sys.exit(1)
 : 
 : zk_client = connect_to_zookeeper(args.zookeeper_hosts, 
args.timeout)
 : errors = sum([check_hbase_node(zk_client, node, 
args.timeout) for node in args.nodes])
I suggest you make this a method. The __main__ can get the arguments, the 
errors, and print the error and exit accordingly. It would be better to have a 
separate method to make a connection and do the query work.


PS5, Line 135: if errors:
It would be better to have the explicit numerical > 0 comparison.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9b81f3cfb6ea0ba7b18ce5fcd5d268f515c8b0c3
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Ishaan Joshi 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4100,4112: Qgen: Replace EXTRACT UDF + IS [NOT] DISTINCT FROM in HiveSqlWriter

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-4100,4112: Qgen: Replace EXTRACT UDF + IS [NOT] DISTINCT 
FROM in HiveSqlWriter
..


Patch Set 2:

Note to committer: there's no GVO path for this, so I suggest you submit along 
with +2.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3922ca61af59ecd2899c911b1a03e11ab5c26e11
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: stak...@cloudera.com
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: stak...@cloudera.com
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4100,4112: Qgen: Replace EXTRACT UDF + IS [NOT] DISTINCT FROM in HiveSqlWriter

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-4100,4112: Qgen: Replace EXTRACT UDF + IS [NOT] DISTINCT 
FROM in HiveSqlWriter
..


Patch Set 2: Code-Review+1

This needs a committer's +2.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3922ca61af59ecd2899c911b1a03e11ab5c26e11
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: stak...@cloudera.com
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: stak...@cloudera.com
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4100, IMPALA-4112: RQG-on-Hive: Replace EXTRACT UDF and IS [NOT] DISTINCT FROM in HiveSqlWriter

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-4100, IMPALA-4112: RQG-on-Hive: Replace EXTRACT UDF and 
IS [NOT] DISTINCT FROM in HiveSqlWriter
..


Patch Set 1:

(2 comments)

Thanks for the patch! This is easy to review since the changes are small and 
isolated to HiveSqlWriter.

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

Line 7: IMPALA-4100, IMPALA-4112: RQG-on-Hive: Replace EXTRACT UDF and IS [NOT] 
DISTINCT FROM in HiveSqlWriter
Please keep the commit message to < 90 characters at most.

For this line, you can convey the same info with:

  IMPALA-4100,4112: qgen: replace EXTRACT UDF and IS [NOT] DISTINCT FROM in 
HiveSqlWriter


http://gerrit.cloudera.org:8080/#/c/4357/1/tests/comparison/model_translator.py
File tests/comparison/model_translator.py:

PS1, Line 462: self.operator_funcs['IsNotDistinctFrom'] = '({0}) <=> ({1})'
 : self.operator_funcs['IsNotDistinctFromOp'] = '({0}) <=> 
({1})'
 : self.operator_funcs['IsDistinctFrom'] = 'NOT(({0}) <=> 
({1}))'
What do you think about:

  self.operator_funcs.update({
  'IsNotDistinctFrom': '({0}) <=> ({1})',
  # etc.
  })


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3922ca61af59ecd2899c911b1a03e11ab5c26e11
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: stak...@cloudera.com
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3718: Support subset of functional-query for Kudu

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-3718: Support subset of functional-query for Kudu
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4175/1/tests/common/test_result_verifier.py
File tests/common/test_result_verifier.py:

PS1, Line 333: # TINYINT). Bypass the type # checking by ignoring the 
actual types of the Avro
Nit: You left the # when joining the line.


PS1, Line 336:   if 'TIMESTAMP' in expected_types:
 : LOG.info("TIMESTAMP columns unsupported in %s, skipping 
verification." %\
 : file_format)
 : return
It's weird to see this logic again.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iada88e078352e4462745d9a9a1b5111260d21acc
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3718: Support subset of functional-query for Kudu

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-3718: Support subset of functional-query for Kudu
..


Patch Set 1:

(6 comments)

In addition to the inline comments, I generally disagree on the surface with 
the use of xfail. It seems like many or all of them should be skips. Any reason 
you chose xfail?

Also, have you tested this end to end? Some ideas:

1. ./buildall.sh -format -testdata [etc]
2. Private data load job
3. ./buildall.sh -format -snapshot_file snapshot_from_private_job 
-metastore_snapshot_file metastore_snapshot_from_private_job [etc]

http://gerrit.cloudera.org:8080/#/c/4175/1/testdata/bin/generate-schema-statements.py
File testdata/bin/generate-schema-statements.py:

Line 594: print "Ignore partitions on Kudu"
Include the db and table name here so we know what we're ignoring.


http://gerrit.cloudera.org:8080/#/c/4175/1/testdata/datasets/functional/functional_schema_template.sql
File testdata/datasets/functional/functional_schema_template.sql:

PS1, Line 598: SELECT row_number() over (order by year, month, id, day),
For my education, was the ordering of "id, day" in the ORDER BY intentional? 
Why that instead of "day, id"?


PS1, Line 1063: text_comma_backslash_newline
This table is defined as a kudu table in schema_constraints.csv L181, but it 
doesn't have a CREATE_KUDU section. Does it need one?


http://gerrit.cloudera.org:8080/#/c/4175/1/testdata/datasets/functional/schema_constraints.csv
File testdata/datasets/functional/schema_constraints.csv:

PS1, Line 177: table_name:testtbl, constraint:only, table_format:kudu/none/none
How is it that this constraint was already defined but only just now had a 
CREATE_KUDU added for it (functional_schema_template.sql, L789 )?


http://gerrit.cloudera.org:8080/#/c/4175/1/testdata/workloads/functional-query/queries/QueryTest/aggregation.test
File testdata/workloads/functional-query/queries/QueryTest/aggregation.test:

Line 836: select min(distinct NULL), max(distinct NULL) from alltypes
Why this change? Thanks.


http://gerrit.cloudera.org:8080/#/c/4175/1/testdata/workloads/functional-query/queries/QueryTest/create_kudu.test
File testdata/workloads/functional-query/queries/QueryTest/create_kudu.test:

PS1, Line 4: drop table if exists managed_kudu;
Fwiw, this won't be needed once the commit lies atop 
https://gerrit.cloudera.org/#/c/4155/


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iada88e078352e4462745d9a9a1b5111260d21acc
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4096: Allow clean.sh to work from snapshots

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-4096: Allow clean.sh to work from snapshots
..


Patch Set 3: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I12dd9035298151557491009680d66d25c8f58c1d
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4096: Allow clean.sh to work from snapshots

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-4096: Allow clean.sh to work from snapshots
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4336/2/bin/clean.sh
File bin/clean.sh:

PS2, Line 55: git rev-parse &>dev/null
Here and elsewhere, you're missing the leading / for /dev/null unless this is 
some fancy bashism I don't know about.

Can you also make sure that the &> operator is supported on CentOS 5? &>> is 
definitely not (see https://gerrit.cloudera.org/#/c/3531/ , for example)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I12dd9035298151557491009680d66d25c8f58c1d
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3491: Use unique database fixture in test partitioning.py

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-3491: Use unique database fixture in test_partitioning.py
..


Patch Set 1: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib1b33d9977a98894288662a711805e9a54329ec8
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4047: DO NOT SUBMIT Highlight all occurrences of CDH/cdh

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-4047: DO NOT SUBMIT Highlight all occurrences of CDH/cdh
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4187/1/infra/deploy/deploy.py
File infra/deploy/deploy.py:

> Opened https://jira.cloudera.com/browse/CDH-44005 to track this. mikeb, do 
Taken.


http://gerrit.cloudera.org:8080/#/c/4187/1/tests/comparison/leopard/impala_docker_env.py
File tests/comparison/leopard/impala_docker_env.py:

PS1, Line 34: 
: 
> Done. I also changed DEFAULT_BRANCH_NAME to 'asf-gerrit/master'. Is origin 
1. If there's nothing wrong with Cloudera producing an Impala Docker image, 
there should be nothing wrong with the fact that its default branch is still 
cdh5-trunk and based off Cloudera Impala. I say leave the default branch alone.

2. origin is the correct remote name for now. See above.

Now, if these are wrong politically, a separate task needs to be made to change 
out the Docker image is generated and published.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icb37e2ef0cd9fa0e581d359c5dd3db7812b7b2c8
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3491: Use unique database fixture in test insert parquet.py

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-3491: Use unique database fixture in 
test_insert_parquet.py
..


Patch Set 1:

David Knupp, want to take this?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I790b2ed5236640c7263826d1d2a74b64d43ac6f7
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4075: Fix import kudu module exception on conftest.py

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-4075: Fix import kudu module exception on conftest.py
..


Patch Set 1:

Thanks for the patch. My personal preference would be to have a more 
comprehensive patch that does one of these, in order of preference:

1. Fix underlying cause of IMPALA-4075, in which Impala and Kudu aren't working 
well together in your dev environment. This should be working, and it isn't. 
Why not?

2. A more comprehensive strategy to support "KUDU_IS_SUPPORTED=false" 
scenarios. It's not tenable to inspect the environment variable all over the 
place. A solution here would be to augment tests/common/environ.py to be a 
method or variable that reports whether kudu is supported, and fix up the 
handful of places that inspect the KUDU_IS_SUPPORTED environment variable 
directly.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idd22f1ca10651c86841bc1ed7d108b26e3d16ee9
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: hewenting 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4047: DO NOT SUBMIT Highlight all occurrences of CDH/cdh

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-4047: DO NOT SUBMIT Highlight all occurrences of CDH/cdh
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4187/1/tests/comparison/leopard/impala_docker_env.py
File tests/comparison/leopard/impala_docker_env.py:

PS1, Line 34: 
: 
> I do not believe there is anything in the apache license that prevents anyo
OK then. I suggest DEFAULT_DOCKER_IMAGE_NAME be changed to 
'cloudera/impala-dev'.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icb37e2ef0cd9fa0e581d359c5dd3db7812b7b2c8
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3491: Use unique database fixture in test nested types.py

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-3491: Use unique database fixture in test_nested_types.py
..


Patch Set 1: Code-Review+1

(2 comments)

Seems straightforward. I found two lines that aren't yours but could use some 
attention.

http://gerrit.cloudera.org:8080/#/c/4302/1/tests/query_test/test_nested_types.py
File tests/query_test/test_nested_types.py:

PS1, Line 22: import random
Double check me, but I don't believe this import is needed.


PS1, Line 82:   TESTFILE_DIR = os.environ['IMPALA_HOME'] + 
"/testdata/parquet_nested_types_encodings"
Use os.path.join to form this directory path.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0c56df0c6a5f771222dedb69353f8bebe01d5a90
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3491: Use unique database fixture in test ddl.py.

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-3491: Use unique database fixture in test_ddl.py.
..


Patch Set 5: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idf667dd5e960768879c019e2037cf48ad4e4241b
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: No

Re: Sorry for the spam

[Michael Brown]

My hunch is you pushed to the Cloudera Impala Gerrit. There's an old
master branch lying there.

Maybe use "git remote rename" or "git remote remove" ?

(I muscle-memory-typo'd this same thing the other day but got lucky
and the push was rejected.)

On Thu, Sep 1, 2016 at 12:49 PM, Thomas Tauber-Marshall
 wrote:
> For anyone wondering, I just pushed the wrong branch to gerrit, resulting
> in a few dozen reviews being created. I'm currently working on abandoning
> them all.
>
> Sorry for the spam.
>
> Thanks,
> Thomas

[Impala-ASF-CR] IMPALA-3491: Use unique database fixture in test ddl.py.

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-3491: Use unique database fixture in test_ddl.py.
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4155/4/tests/metadata/test_ddl.py
File tests/metadata/test_ddl.py:

PS4, Line 339:  Note that we cannot run this test in parallel
 : # with multiple test vectors because the test modifies and 
checks a single
 : # impalad metric.
> Was debating with myself about this. I still prefer to run in parallel if w
Let's talk in person about this test tomorrow.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idf667dd5e960768879c019e2037cf48ad4e4241b
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3491: Use unique database fixture in test ddl.py.

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-3491: Use unique database fixture in test_ddl.py.
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4155/4/tests/metadata/test_ddl.py
File tests/metadata/test_ddl.py:

PS4, Line 339:  Note that we cannot run this test in parallel
 : # with multiple test vectors because the test modifies and 
checks a single
 : # impalad metric.
Given this, should the test be marked to run serially?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idf667dd5e960768879c019e2037cf48ad4e4241b
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4047: DO NOT SUBMIT Highlight all occurrences of CDH/cdh

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-4047: DO NOT SUBMIT Highlight all occurrences of CDH/cdh
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4187/1/tests/comparison/cluster.py
File tests/comparison/cluster.py:

PS1, Line 30: from cm_api.api_client import ApiResource as CmApiResource
> Why is the CM API a problem? It's publicly available and we are certainly a
Maybe it's not a problem at all! It's an ASF / political question.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icb37e2ef0cd9fa0e581d359c5dd3db7812b7b2c8
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4047: DO NOT SUBMIT Highlight all occurrences of CDH/cdh

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-4047: DO NOT SUBMIT Highlight all occurrences of CDH/cdh
..


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/4187/1/infra/deploy/deploy.py
File infra/deploy/deploy.py:

> This whole file is CM specific. I don't know what to do with it.
My vote is to move it to a Cloudera-internal repo (Impala-aux) after ensuring 
there are no automated users/callers.


http://gerrit.cloudera.org:8080/#/c/4187/1/tests/comparison/cluster.py
File tests/comparison/cluster.py:

PS1, Line 30: from cm_api.api_client import ApiResource as CmApiResource
What do we do about cm_api? It's imported in a few places and part of 
infra/python/deps/requirements.txt

https://pypi.python.org/pypi/cm-api


http://gerrit.cloudera.org:8080/#/c/4187/1/tests/comparison/leopard/impala_docker_env.py
File tests/comparison/leopard/impala_docker_env.py:

PS1, Line 34: 
: 
> None of this would work outside of cloudera networks, what to do with it?
I remembered there is a public Docker image here 
https://hub.docker.com/r/cloudera/impala-dev/ .

It's not clear whether we could update the image to use the public one and have 
that in ASF, and it's not clear whether Cloudera is permitted to publish a 
Docker image of Impala anymore.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icb37e2ef0cd9fa0e581d359c5dd3db7812b7b2c8
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3491: Use unique database fixture in test ddl.py.

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-3491: Use unique database fixture in test_ddl.py.
..


Patch Set 3: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4155/3/tests/conftest.py
File tests/conftest.py:

PS3, Line 284:   return first_db_name
I don't really like that we return a db name but more than one DB is created if 
num_dbs > 1, and that tests using the fixture then have to do some string 
concatenation to derive those DBs' names, and that .test files only know 1 
database, and that they have to do the same string concatenation.

However, that's a lot of other mess to fix for this patch, so I'm willing to 
live with that. The .test problem is particularly gross and doesn't have an 
immediately obvious solution.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idf667dd5e960768879c019e2037cf48ad4e4241b
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3491: Use unique database fixture in test join queries.py.

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-3491: Use unique database fixture in 
test_join_queries.py.
..


Patch Set 1: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib639ff8a37dbf64840606f88badff8f2590587b6
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3491: Use unique database fixture in test ddl.py.

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-3491: Use unique database fixture in test_ddl.py.
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4155/2/tests/metadata/test_hms_integration.py
File tests/metadata/test_hms_integration.py:

PS2, Line 59: self.run_stmt_in_hive("create database hms_sanity_db")
In testing this test, I identified a test flake: this test can't be 
successfully run twice in a row in a dev environment, because hms_sanity_db 
will exist after the test completes, and then fail here after that.

You can reproduce the problem via the following, twice, from the tests/ 
directory:

   impala-py.test -k test_sanity metadata/test_hms_integration.py

One suggestion is to "DROP DATABASE IF EXISTS ... CASCADE" before L59.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idf667dd5e960768879c019e2037cf48ad4e4241b
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3491: Use unique database fixture in test ddl.py.

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-3491: Use unique database fixture in test_ddl.py.
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4155/1/testdata/workloads/functional-query/queries/QueryTest/alter-table.test
File testdata/workloads/functional-query/queries/QueryTest/alter-table.test:

PS1, Line 263: create external table t_part (i int) partitioned by (j int, s 
string)
 : location 
'$FILESYSTEM_PREFIX/test-warehouse/$DATABASE.db/t_part_tmp';
 : alter table t_part add partition (j=cast(2-1 as int), s='2012');
 : alter table t_part add if not exists partition (j=1, s='2012');
 : alter table t_part add if not exists partition (j=1, 
s='2012/withslash');
 : alter table t_part add partition (j=1, s=substring('foo2013bar', 
4, 8));
> If any of thee queries fail, the client throws a Beeswax exception which is
Done


http://gerrit.cloudera.org:8080/#/c/4155/1/tests/metadata/test_hms_integration.py
File tests/metadata/test_hms_integration.py:

Line 55:   def test_sanity(self, vector):
> I'm not sure our execute_query_expect_success even makes sense, at least fo
Done. Tested by hand and I'm convinced.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idf667dd5e960768879c019e2037cf48ad4e4241b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3491: Use unique database fixture in test ddl.py.

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-3491: Use unique database fixture in test_ddl.py.
..


Patch Set 1:

(5 comments)

Eyes began to glaze over at last 10% of test_ddl.py. Here's some other comments 
for now.

http://gerrit.cloudera.org:8080/#/c/4155/1/testdata/workloads/functional-query/queries/QueryTest/alter-table.test
File testdata/workloads/functional-query/queries/QueryTest/alter-table.test:

PS1, Line 263: create external table t_part (i int) partitioned by (j int, s 
string)
 : location 
'$FILESYSTEM_PREFIX/test-warehouse/$DATABASE.db/t_part_tmp';
 : alter table t_part add partition (j=cast(2-1 as int), s='2012');
 : alter table t_part add if not exists partition (j=1, s='2012');
 : alter table t_part add if not exists partition (j=1, 
s='2012/withslash');
 : alter table t_part add partition (j=1, s=substring('foo2013bar', 
4, 8));
Will the test framework adequately fail the test if one of these reports an 
error? It matters here, because unlike in some cases below, there's no SELECT 
at the end of this to implicitly verify they all worked.


http://gerrit.cloudera.org:8080/#/c/4155/1/tests/metadata/test_ddl.py
File tests/metadata/test_ddl.py:

PS1, Line 234: assert "ddl_test_db" not in self.all_db_names()
(I found this by searching for the removed members of TEST_DBS.)

  assert unique_database not in self.all_db_names()


PS1, Line 255: unique_dabatase2 = unique_database + '2'
 : self._create_db(unique_dabatase2, sync=True)
Clever, but it's unfortunate you have to do this. I'm not aware of pytest being 
able to use the same fixture twice (or more) in a test.

Should we parametrize unique_database optionally to return a set of databases 
instead of just one? It seems like it could use your incrementing numerical 
suffix scheme and be fine.


PS1, Line 328: create_fn_stmt = "create function {0}.f() returns int "\
 :  "location '{1}/libTestUdfs.so' 
symbol='NoArgs'"\
 :  .format(unique_database, WAREHOUSE)
Tip for here and elsewhere: You can join strings across lines without \ if they 
are within parens.

create_fn_stmt = ("create function {0}.f() returns int "
  "location '{1}/libTestUdfs.so' symbol='NoArgs'"
  .format(unique_database, WAREHOUSE))


http://gerrit.cloudera.org:8080/#/c/4155/1/tests/metadata/test_hms_integration.py
File tests/metadata/test_hms_integration.py:

Line 55:   def test_sanity(self, vector):
Not your changes, but in case you want to improve: This test doesn't have many 
asserts. I think it could be improved by replacing the self.client.execute() 
statements with self.execute_query_expect_success() statement. L70 for sure is 
never verified.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idf667dd5e960768879c019e2037cf48ad4e4241b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4001: qgen: add proof of concept tests for Query() objects

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-4001: qgen: add proof of concept tests for Query() 
objects
..


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4081/4/tests/comparison/tests/fake_query.py
File tests/comparison/tests/fake_query.py:

PS4, Line 83:   Return a Query object constructed by the keyword args above. 
select_clause and
:   from_clause are required.
:   """
> Why are there different restrictions? I guess the query generator could gen
Leaving as is.

The reason is that when the query generator runs, a sparse Query() object gets 
instantiated, built up over time, inspected, and added to over the course of 
building the query. What I need here are already fully formed. So the 
restriction is put in place for unit tests, not the flow of generating a random 
query.

This might be a good way to identify places in code that demonstrate what I'm 
talking about:

  $ git grep -En "( query\.[a-z_]+ = |query = Query|\.current_query)" -- 
tests/comparison/query_generator.py

If we enforced the restriction you speak of on Query() objects, we have to 
really refactor the query generator. Without automated tests, I'd rather not do 
that. Otherwise we risk some regression that's difficult to detect without 
collecting stats to validate our query profiles (planned in IMPALA-3993).


http://gerrit.cloudera.org:8080/#/c/4081/4/tests/comparison/tests/query_object_testdata.py
File tests/comparison/tests/query_object_testdata.py:

PS4, Line 106: FakeSelectClause(
 : FakeFirstValue(
> Ok, well I'm not trying to slow things down but something to consider for t
Thanks. I consider the wording of the TODO in fake_query.py a good note for 
this.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ed1960430ae0af469986e33f88aecb6fa74e999
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4001: qgen: add proof of concept tests for Query() objects

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-4001: qgen: add proof of concept tests for Query() 
objects
..


Patch Set 4: Code-Review+1

carry +1 again, I guess. Gerrit is annoying.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ed1960430ae0af469986e33f88aecb6fa74e999
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4001: qgen: add proof of concept tests for Query() objects

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-4001: qgen: add proof of concept tests for Query() 
objects
..


Patch Set 4: -Code-Review

(3 comments)

Matthew, thanks for the review. Please see comments. I'd like responses before 
I work on the next patch set.

http://gerrit.cloudera.org:8080/#/c/4081/4/tests/comparison/tests/fake_query.py
File tests/comparison/tests/fake_query.py:

PS4, Line 83:   Return a Query object constructed by the keyword args above. 
select_clause and
:   from_clause are required.
:   """
> Why not change Query() to support keyword args to initialize?
I could, but I still need something here that enforces the SELECT and FROM 
clause attributes being set when building full Queries for unit tests. I don't 
want that enforcement for building Queries with the query generator. It didn't 
seem worth changing the actual Query() object for that purpose.

L18-33 tries generally to explain this, but maybe specific aspects of Query 
aren't explained as well as I could.

Would you like a better explanation in a comment?


http://gerrit.cloudera.org:8080/#/c/4081/4/tests/comparison/tests/query_object_testdata.py
File tests/comparison/tests/query_object_testdata.py:

PS4, Line 26: namedtuple(
> This might be obvious for other folks, but I had to google to determine tha
It's a better way to specify a data container with meaningful member names. 
It's closer to a struct, where you can't add fields.

1. Raw dictionaries and tuples aren't particularly helpful. You have to read 
code to understand what they're doing.

2. The attribute set is immutable, unlike in a class. This prevents the bad 
habit allowed by classes whereby attributes may be added or removed by a 
caller, changing the class's interface on the fly. In fact the query generator 
is full of these. It's because you have to read the code that sets the 
attributes to know what they do, as opposed to reading the class definition. So 
we get to use the features of the standard library to enforce rules.

3. But you get the syntactic sugar of attribute names to set and get data, 
instead of just positional orientation as in regular tuples.

The middle bits of the top answer here are good:

http://stackoverflow.com/questions/2970608/what-are-named-tuples-in-python#2970722

If this answer satisfies you, can you click Done? If not, let me know and I'll 
change to a class implementation if you wish, or add a comment. However, should 
all future namedtuples have a comment?


PS4, Line 106: FakeSelectClause(
 : FakeFirstValue(
> I wonder if we could make the query objects easier to construct, e.g. makin
So far the Fake* aren't actually classes. They are functions that abstract away 
the way objects get created and allow for a nested way to build full Query() 
objects. Note that for some, like FromClause we don't need them. However, the 
builder pattern is an intriguing idea.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ed1960430ae0af469986e33f88aecb6fa74e999
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3491: Use unique database fixture in test ddl.py.

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-3491: Use unique database fixture in test_ddl.py.
..


Patch Set 1:

Alex, I'm ACK'ing that I have seen this review request.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idf667dd5e960768879c019e2037cf48ad4e4241b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4001: qgen: add proof of concept tests for Query() objects

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-4001: qgen: add proof of concept tests for Query() 
objects
..


Patch Set 4: Code-Review+1

rebase, carry +1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ed1960430ae0af469986e33f88aecb6fa74e999
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4009: qgen: add docs, utility for installing Oracle as ref database

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-4009: qgen: add docs, utility for installing Oracle as 
ref database
..


Patch Set 3:

Thanks Matthew. Could you commit this? I don't think it makes sense to waste 
GVO cycles on this patch.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib3bf84b7afa8851c49a8d0f0a1ceded94f4de158
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4001: qgen: add proof of concept tests for Query() objects

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-4001: qgen: add proof of concept tests for Query() 
objects
..


Patch Set 3: Code-Review+1

carry +1 since there are no functional changes between patch sets 2 and 3.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ed1960430ae0af469986e33f88aecb6fa74e999
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4001: qgen: add proof of concept tests for Query() objects

[Michael Brown (Code Review)]

Hello David Knupp,

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

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

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

Change subject: IMPALA-4001: qgen: add proof of concept tests for Query() 
objects
..

IMPALA-4001: qgen: add proof of concept tests for Query() objects

This patch adds a simple proof-of-concept test framework, and a few
tests, for the random query generator, specifically the portion of the
random query generator that is responsible for taking a Query object and
doing something with it. The two pieces of functionality I chose for
exhibition are

1. Writing the query into Impala SQL
2. Reporting characteristics in the SELECT clause (used internally)

In the interest of keeping the patch small, I have not added many tests,
nor have I chose to focus on more areas for test. On its own this is
fairly simple. As I add features to this portion of the query generator,
though, it will be more useful to test new functionality and also
regression test the framework.

Change-Id: I2ed1960430ae0af469986e33f88aecb6fa74e999
---
A tests/comparison/tests/README
A tests/comparison/tests/fake_query.py
A tests/comparison/tests/query_object_testdata.py
A tests/comparison/tests/test_query_objects.py
4 files changed, 335 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/4081/3
-- 
To view, visit http://gerrit.cloudera.org:8080/4081
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2ed1960430ae0af469986e33f88aecb6fa74e999
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 

[Impala-ASF-CR] IMPALA-4001: qgen: add proof of concept tests for Query() objects

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-4001: qgen: add proof of concept tests for Query() 
objects
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4081/2/tests/comparison/tests/query_object_testdata.py
File tests/comparison/tests/query_object_testdata.py:

Line 70: testid='select col from table',
> A minor followup thought -- it might be nice to have testid as the first el
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ed1960430ae0af469986e33f88aecb6fa74e999
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4009: qgen: add docs, utility for installing Oracle as ref database

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-4009: qgen: add docs, utility for installing Oracle as 
ref database
..


Patch Set 3:

Thomas, see what you think. I simply removed the redundant sub-steps in the 
section you highlighted, since the instant-client's directions have those 
already.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib3bf84b7afa8851c49a8d0f0a1ceded94f4de158
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4009: qgen: add docs, utility for installing Oracle as ref database

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-4009: qgen: add docs, utility for installing Oracle as 
ref database
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4095/1/tests/comparison/ORACLE.txt
File tests/comparison/ORACLE.txt:

Line 68: 1. Refer to the instant-client installation instructions. As of this
> I'm confused by the structure here, since these instructions are referring 
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib3bf84b7afa8851c49a8d0f0a1ceded94f4de158
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4009: qgen: add docs, utility for installing Oracle as ref database

[Michael Brown (Code Review)]

Hello Matthew Jacobs,

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

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

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

Change subject: IMPALA-4009: qgen: add docs, utility for installing Oracle as 
ref database
..

IMPALA-4009: qgen: add docs, utility for installing Oracle as ref database

This patch adds documentation and a small utility for users wishing to
run the random query generator against Oracle as a reference database.
In particular, the patch

* points users toward installing Oracle Database.
* provides directions on incorporating a Python Oracle client
  (cx_Oracle) into our impala-python virtual environment.
* provides a small utility that will verify installation and
  connectivity between client and server

This documentation isn't meant to be a complete guide. Other patches
will add documentation for actually loading random query generator test
data into Oracle, for example.

Change-Id: Ib3bf84b7afa8851c49a8d0f0a1ceded94f4de158
---
A tests/comparison/ORACLE.txt
M tests/comparison/README
A tests/comparison/util/verify-oracle-connection.py
3 files changed, 166 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/4095/3
-- 
To view, visit http://gerrit.cloudera.org:8080/4095
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib3bf84b7afa8851c49a8d0f0a1ceded94f4de158
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Thomas Tauber-Marshall 

[Impala-ASF-CR] IMPALA-4001: qgen: add proof of concept tests for Query() objects

[Michael Brown (Code Review)]

Michael Brown has uploaded a new patch set (#2).

Change subject: IMPALA-4001: qgen: add proof of concept tests for Query() 
objects
..

IMPALA-4001: qgen: add proof of concept tests for Query() objects

This patch adds a simple proof-of-concept test framework, and a few
tests, for the random query generator, specifically the portion of the
random query generator that is responsible for taking a Query object and
doing something with it. The two pieces of functionality I chose for
exhibition are

1. Writing the query into Impala SQL
2. Reporting characteristics in the SELECT clause (used internally)

In the interest of keeping the patch small, I have not added many tests,
nor have I chose to focus on more areas for test. On its own this is
fairly simple. As I add features to this portion of the query generator,
though, it will be more useful to test new functionality and also
regression test the framework.

Change-Id: I2ed1960430ae0af469986e33f88aecb6fa74e999
---
A tests/comparison/tests/README
A tests/comparison/tests/fake_query.py
A tests/comparison/tests/query_object_testdata.py
A tests/comparison/tests/test_query_objects.py
4 files changed, 335 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/4081/2
-- 
To view, visit http://gerrit.cloudera.org:8080/4081
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2ed1960430ae0af469986e33f88aecb6fa74e999
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Taras Bobrovytsky 

[Impala-ASF-CR] IMPALA-4001: qgen: add proof of concept tests for Query() objects

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-4001: qgen: add proof of concept tests for Query() 
objects
..


Patch Set 1:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/4081/1/tests/comparison/tests/fake_query.py
File tests/comparison/tests/fake_query.py:

PS1, Line 97: must at least a
> "must at least *contain* a" ...?
Done


PS1, Line 97: fram
> s/fram/from/
Done


http://gerrit.cloudera.org:8080/#/c/4081/1/tests/comparison/tests/query_object_testdata.py
File tests/comparison/tests/query_object_testdata.py:

PS1, Line 64: DATASET
> So -- these are really test cases, yes? Could this just be called QUERY_TES
Done


http://gerrit.cloudera.org:8080/#/c/4081/1/tests/comparison/tests/test_query_objects.py
File tests/comparison/tests/test_query_objects.py:

PS1, Line 25: query_something
> I'm a little confused by this. query_something is a ... QueryTest object (n
Done


Line 53: @pytest.mark.parametrize('query_test', DATASET, ids=_idfn)
> How do you feel about moving all of the tests to the end of the file, rathe
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ed1960430ae0af469986e33f88aecb6fa74e999
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4009: qgen: add docs, utility for installing Oracle as ref database

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-4009: qgen: add docs, utility for installing Oracle as 
ref database
..


Patch Set 2:

Thanks for the reviews. In patch set 2, I added a small utility to verify 
installation and connectivity. Thomas, let me address your comments in the next 
patch set.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib3bf84b7afa8851c49a8d0f0a1ceded94f4de158
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4009: qgen: add docs, utility for installing Oracle as ref database

[Michael Brown (Code Review)]

Hello Matthew Jacobs,

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

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

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

Change subject: IMPALA-4009: qgen: add docs, utility for installing Oracle as 
ref database
..

IMPALA-4009: qgen: add docs, utility for installing Oracle as ref database

This patch adds documentation and a small utility for users wishing to
run the random query generator against Oracle as a reference database.
In particular, the patch

* points users toward installing Oracle Database.
* provides directions on incorporating a Python Oracle client
  (cx_Oracle) into our impala-python virtual environment.
* provides a small utility that will verify installation and
  connectivity between client and server

This documentation isn't meant to be a complete guide. Other patches
will add documentation for actually loading random query generator test
data into Oracle, for example.

Change-Id: Ib3bf84b7afa8851c49a8d0f0a1ceded94f4de158
---
A tests/comparison/ORACLE.txt
M tests/comparison/README
A tests/comparison/util/verify-oracle-connection.py
3 files changed, 176 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/4095/2
-- 
To view, visit http://gerrit.cloudera.org:8080/4095
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib3bf84b7afa8851c49a8d0f0a1ceded94f4de158
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Thomas Tauber-Marshall 

[Impala-ASF-CR] IMPALA-4009: qgen: add documentation for installing Oracle as ref database

[Michael Brown (Code Review)]

Michael Brown has uploaded a new change for review.

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

Change subject: IMPALA-4009: qgen: add documentation for installing Oracle as 
ref database
..

IMPALA-4009: qgen: add documentation for installing Oracle as ref database

This patch adds documentation for users wishing to run the random query
generator against Oracle as a reference database. In particular, the
patch points users toward installing Oracle Database, and it provides
directions on incorporating a Python Oracle client into our
impala-python virtual environment.

This documentation isn't meant to be a complete guide. Other patches
will add documentation for actually loading random query generator test
data into Oracle, for example.

Change-Id: Ib3bf84b7afa8851c49a8d0f0a1ceded94f4de158
---
A tests/comparison/ORACLE.txt
M tests/comparison/README
2 files changed, 110 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/4095/1
-- 
To view, visit http://gerrit.cloudera.org:8080/4095
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib3bf84b7afa8851c49a8d0f0a1ceded94f4de158
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown 

[Impala-ASF-CR] IMPALA-4001: qgen: add proof of concept tests for Query() objects

[Michael Brown (Code Review)]

Michael Brown has uploaded a new change for review.

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

Change subject: IMPALA-4001: qgen: add proof of concept tests for Query() 
objects
..

IMPALA-4001: qgen: add proof of concept tests for Query() objects

This patch adds a simple proof-of-concept test framework, and a few
tests, for the random query generator, specifically the portion of the
random query generator that is responsible for taking a Query object and
doing something with it. The two pieces of functionality I chose for
exhibition are

1. Writing the query into Impala SQL
2. Reporting characteristics in the SELECT clause (used internally)

In the interest of keeping the patch small, I have not added many tests,
nor have I chose to focus on more areas for test. On its own this is
fairly simple. As I add features to this portion of the query generator,
though, it will be more useful to test new functionality and also
regression test the framework.

Change-Id: I2ed1960430ae0af469986e33f88aecb6fa74e999
---
A tests/comparison/tests/README
A tests/comparison/tests/fake_query.py
A tests/comparison/tests/query_object_testdata.py
A tests/comparison/tests/test_query_objects.py
4 files changed, 335 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/4081/1
-- 
To view, visit http://gerrit.cloudera.org:8080/4081
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I2ed1960430ae0af469986e33f88aecb6fa74e999
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown 

[Impala-ASF-CR] IMPALA-3954: Add unique database to scanner test

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-3954: Add unique_database to scanner test
..


Patch Set 1: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic48a4bac3df6a40cb5cb10c6f1c42583952c6c86
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: No

[Impala-ASF-CR] Fix stress test runner bug introduced by IMPALA-3969

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: Fix stress test runner bug introduced by IMPALA-3969
..


Patch Set 2: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I86484bb7c92ae1069f6a07cf3ea5027740364150
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] Fix stress test runner bug introduced by IMPALA-3969

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: Fix stress test runner bug introduced by IMPALA-3969
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4019/1/tests/stress/concurrent_select.py
File tests/stress/concurrent_select.py:

PS1, Line 705: self.common_query_options = {}
Thanks. In haste, I think I mistook L680 for what should have gone here, 
instead, and it's on me for not having tested the path. I think we could delete 
L680. Do you agree?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I86484bb7c92ae1069f6a07cf3ea5027740364150
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: Yes