[Impala-ASF-CR] IMPALA-8175: improve tests minicluster obj

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12412 )

Change subject: IMPALA-8175: improve tests_minicluster_obj
..


Patch Set 2:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/12412/1/tests/comparison/cluster.py@850
PS1, Line 850: [i]mpalad
> I'll fix this. I am not immediately sure how testing succeeded.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I558b3157bb16ef3d169c0d3e795e03700a17ffe4
Gerrit-Change-Number: 12412
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Fri, 08 Feb 2019 17:41:01 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8175: improve tests minicluster obj

[Michael Brown (Code Review)]

Hello Andrew Sherman, Philip Zeyliger, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-8175: improve tests_minicluster_obj
..

IMPALA-8175: improve tests_minicluster_obj

Adjust minicluster impalad pgrep detection usage to be compatible with
CentOS 6 pgrep.  Skip the test if not in a minicluster, because the test
will fail. Don't run the test in exhaustive: it's most important this
test run pre-merge, which is core.

I was able to use sh -c "pgrep ..." and impala-py.test to test this
locally on CentOS 6 and Ubuntu 16.

Change-Id: I558b3157bb16ef3d169c0d3e795e03700a17ffe4
---
M tests/comparison/cluster.py
M tests/infra/test_stress_infra.py
2 files changed, 13 insertions(+), 3 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I558b3157bb16ef3d169c0d3e795e03700a17ffe4
Gerrit-Change-Number: 12412
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 

[Impala-ASF-CR] IMPALA-8171: minicluster: use exec builtin to start Impala daemons

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12391 )

Change subject: IMPALA-8171: minicluster: use exec builtin to start Impala 
daemons
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12391/1/bin/start-daemon.sh
File bin/start-daemon.sh:

http://gerrit.cloudera.org:8080/#/c/12391/1/bin/start-daemon.sh@33
PS1, Line 33: exec "$@"
I chose this because I didn't see any comments in 
https://gerrit.cloudera.org/#/c/12271/ about exec being intentionally omitted. 
If it turns out exec has a negative consequence and you don't want it here, I 
can fix this in a different way.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34ac8ce964f0c56287cee01360a4ba889f9568d7
Gerrit-Change-Number: 12391
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 07 Feb 2019 16:10:10 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8171: minicluster: use exec builtin to start Impala daemons

[Michael Brown (Code Review)]

Michael Brown has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/12391


Change subject: IMPALA-8171: minicluster: use exec builtin to start Impala 
daemons
..

IMPALA-8171: minicluster: use exec builtin to start Impala daemons

In IMPALA-7779 bin/start-daemon.sh was written to unify the place where
Impala daemons are started. It had a side-effect of keeping itself alive
as a parent process of the actual daemon. This broke the
tests.comparison.cluster abstraction used by the stress test and others
in that the process detection logic was finding false positives.

The old bin/start-*.sh scripts used the exec builtin to call their
daemons. Resurrect this as a simple fix; it also makes the ps list
shorter. Add a system test to catch this in the future.

Change-Id: I34ac8ce964f0c56287cee01360a4ba889f9568d7
---
M bin/start-daemon.sh
M tests/infra/test_stress_infra.py
2 files changed, 14 insertions(+), 1 deletion(-)



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

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

[Impala-ASF-CR] IMPALA-8169: small changes to Leopard

[Michael Brown (Code Review)]

Michael Brown has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/12386


Change subject: IMPALA-8169: small changes to Leopard
..

IMPALA-8169: small changes to Leopard

- Fix a bug in which rsync --chown doesn't work on CentOS 7.

- Update HOST_TESTDATA_EXTERNAL_VOLUME_PATH (for the minicluster data):
  most runs now are on EC2 etc., and they already need a large volume
  for docker images, so just keep the cluster data there, too.

- Reduce extremely verbose logging.

- Default to a database that's part of dataload (tpch_kudu).

- Change some of the controller variables to my preferred defaults.

Change-Id: I169f60dad53d2e4980ed6bd1f350fb0dcf274306
Testing: Regular downstream runs for months.
---
M tests/comparison/leopard/controller.py
M tests/comparison/leopard/impala_docker_env.py
2 files changed, 10 insertions(+), 9 deletions(-)



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

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

[Impala-ASF-CR] IMPALA-8091 addendum: use absolute path for ntp-wait

[Michael Brown (Code Review)]

Michael Brown has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/12369


Change subject: IMPALA-8091 addendum: use absolute path for ntp-wait
..

IMPALA-8091 addendum: use absolute path for ntp-wait

While ntp-wait is in the default $PATH on Ubuntu, it's not on CentOS.
Use the absolute path of the binary (same in both flavors).

Testing:
- Ran privately on CentOS both with and without ntp-wait installed; GVO
  will cover Ubuntu.

Change-Id: I5d709d121a71e62b8ee4027db81b53108f389fdd
---
M testdata/cluster/admin
1 file changed, 4 insertions(+), 2 deletions(-)



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

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

[Impala-ASF-CR] IMPALA-8113: skip test aggregation and test avro primitive in list in S3

[Michael Brown (Code Review)]

Michael Brown has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/12272


Change subject: IMPALA-8113: skip test_aggregation and 
test_avro_primitive_in_list in S3
..

IMPALA-8113: skip test_aggregation and test_avro_primitive_in_list in S3

Procedure for flaky S3 tests right now is to skip them due to eventual
consistency problems. This skips 2 more, test_aggregation and
test_avro_primitive_in_list.

Change-Id: Ica001e0237cf1eff300c6b4e8f28f48e728a0649
---
M tests/query_test/test_aggregation.py
M tests/query_test/test_nested_types.py
2 files changed, 3 insertions(+), 0 deletions(-)



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

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

[Impala-ASF-CR] IMPALA-8091: incremental improvements to NTP sync

[Michael Brown (Code Review)]

Michael Brown has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/12253


Change subject: IMPALA-8091: incremental improvements to NTP sync
..

IMPALA-8091: incremental improvements to NTP sync

- Warn when ntp-wait is not available.

- Install ntp-perl for Redhat-based systems, which provides ntp-wait.
  Debian-based systems already have ntp-wait as provided by ntp.

Change-Id: Ifc8cacf81765d132dd574993f8149231bdbb7bc6
---
M bin/bootstrap_system.sh
M testdata/cluster/admin
2 files changed, 4 insertions(+), 2 deletions(-)



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

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

[Impala-ASF-CR] IMPALA-5847: Fix incorrect use of SET in .test files

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12220 )

Change subject: IMPALA-5847: Fix incorrect use of SET in .test files
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4e4c0f31bf4850642b624acdb1f6cb8837957990
Gerrit-Change-Number: 12220
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Marshall 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 17 Jan 2019 21:50:26 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8089: Fixing run-all-tests timeout raciness.

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12230 )

Change subject: IMPALA-8089: Fixing run-all-tests timeout raciness.
..


Patch Set 1: Code-Review+1

Seems a fine place to start. Bikram, thoughts?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieccfda933f526c116e49c46bf34f7245b357cb07
Gerrit-Change-Number: 12230
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Thu, 17 Jan 2019 18:44:14 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5847: Fix incorrect use of SET in .test files

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12220 )

Change subject: IMPALA-5847: Fix incorrect use of SET in .test files
..


Patch Set 2:

(6 comments)

No issues with implementation. I looked into whether pytest_generate_tests 
could be made to ensure copies of vectors, but I think where the vectors are 
inserted won't permit that. Anyway...

I left some comments that are suggestions about making things a little clearer 
for someone else developing in these .test files, or if someone encounters a 
failure when they use a query option in a .test file when they shouldn't.

http://gerrit.cloudera.org:8080/#/c/12220/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/12220/2//COMMIT_MSG@20
PS2, Line 20: Testing:
: - Passed a full exhaustive run.
Nice!


http://gerrit.cloudera.org:8080/#/c/12220/2/testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan-single-node.test
File 
testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan-single-node.test:

http://gerrit.cloudera.org:8080/#/c/12220/2/testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan-single-node.test@8
PS2, Line 8: # code path because a single scan node instance must process all 
input files.
Does it make sense to say in this test file that num_nodes=1 is expected to be 
set by the Python test?


http://gerrit.cloudera.org:8080/#/c/12220/2/testdata/workloads/functional-query/queries/QueryTest/nested-types-tpch-mem-limit-single-node.test
File 
testdata/workloads/functional-query/queries/QueryTest/nested-types-tpch-mem-limit-single-node.test:

http://gerrit.cloudera.org:8080/#/c/12220/2/testdata/workloads/functional-query/queries/QueryTest/nested-types-tpch-mem-limit-single-node.test@9
PS2, Line 9: # mem_limit is tuned for a 3-node HDFS minicluster.
Same here: does it make sense to say in this test file that num_nodes=1 is 
expected to be set by the Python test?


http://gerrit.cloudera.org:8080/#/c/12220/2/testdata/workloads/functional-query/queries/QueryTest/subquery-single-node.test
File 
testdata/workloads/functional-query/queries/QueryTest/subquery-single-node.test:

http://gerrit.cloudera.org:8080/#/c/12220/2/testdata/workloads/functional-query/queries/QueryTest/subquery-single-node.test@4
PS2, Line 4: # executed on a single node.
Same here: does it make sense to say in this test file that num_nodes=1 is 
expected to be set by the Python test?


http://gerrit.cloudera.org:8080/#/c/12220/2/testdata/workloads/tpch/queries/sort-reservation-usage.test
File testdata/workloads/tpch/queries/sort-reservation-usage.test:

http://gerrit.cloudera.org:8080/#/c/12220/2/testdata/workloads/tpch/queries/sort-reservation-usage.test@6
PS2, Line 6: # the scan uses less reservation.
Same here: does it make sense to say in this test file that num_nodes=1 is 
expected to be set by the Python test?

Also, should this file be renamed to declare that it's "single-node" like you 
did when you broke out other .test files?


http://gerrit.cloudera.org:8080/#/c/12220/2/tests/common/impala_test_suite.py
File tests/common/impala_test_suite.py:

http://gerrit.cloudera.org:8080/#/c/12220/2/tests/common/impala_test_suite.py@468
PS2, Line 468: assert set_pattern_match.groups()[0] not in 
vector.get_value("exec_option"), \
 : "You cannot set a value in a '.test' file if it 
is in the test vector."
Will this print the errant query option when it fails? We want to make sure 
that happens so that failures can be fixed easily.

You could also extend the message to try to help the user fix the issue. I'm 
not sure how to word it to help and also be short. Maybe you could point to 
examples of other tests that do the vector deepcopy and then remove/add to the 
vector trick? Just throwing something out there.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4e4c0f31bf4850642b624acdb1f6cb8837957990
Gerrit-Change-Number: 12220
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Marshall 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 14 Jan 2019 23:11:23 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] Added timeout to run-all-tests

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12086 )

Change subject: Added timeout to run-all-tests
..


Patch Set 2: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iefd3e16a973709f27aeffc7a15bcab8fcdbb349b
Gerrit-Change-Number: 12086
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Thu, 20 Dec 2018 00:48:00 +
Gerrit-HasComments: No

[Impala-ASF-CR] Added timeout to run-all-tests

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12086 )

Change subject: Added timeout to run-all-tests
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/12086/1/bin/run-all-tests-timeout-check.sh
File bin/run-all-tests-timeout-check.sh:

http://gerrit.cloudera.org:8080/#/c/12086/1/bin/run-all-tests-timeout-check.sh@52
PS1, Line 52:  "Test run timed out"
Explain where to find information? ($IMPALA_TIMEOUT_LOGS_DIR)


http://gerrit.cloudera.org:8080/#/c/12086/1/bin/run-all-tests.sh
File bin/run-all-tests.sh:

http://gerrit.cloudera.org:8080/#/c/12086/1/bin/run-all-tests.sh@34
PS1, Line 34: : ${TIMEOUT_FOR_RUN_ALL_TESTS_MINS:=0}
Any reason not to default this to something high, like 20 hours?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iefd3e16a973709f27aeffc7a15bcab8fcdbb349b
Gerrit-Change-Number: 12086
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Mon, 17 Dec 2018 17:50:21 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7926: Fix flakiness in test reconnect

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12045 )

Change subject: IMPALA-7926: Fix flakiness in test_reconnect
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3017ca3bf7b4e33440cffb80e9a48a63bec14434
Gerrit-Change-Number: 12045
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Marshall 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Wed, 12 Dec 2018 20:59:06 +
Gerrit-HasComments: No

[Impala-ASF-CR](asf-site) [DOCS] The wrong version number was fixed for Impala 3.0 Changelog

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12056 )

Change subject: [DOCS] The wrong version number was fixed for Impala 3.0 
Changelog
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-MessageType: comment
Gerrit-Change-Id: I54e4f2a91e65b9243cf230422e4cd50fd0bd8640
Gerrit-Change-Number: 12056
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Fri, 07 Dec 2018 19:33:05 +
Gerrit-HasComments: No

[Impala-ASF-CR](asf-site) [DOCS] Impala 3.1 Docs to be published

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12046 )

Change subject: [DOCS] Impala 3.1 Docs to be published
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-MessageType: comment
Gerrit-Change-Id: Icdfa4df8f75090b4783912713e2820eacbdfa904
Gerrit-Change-Number: 12046
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 06 Dec 2018 22:49:20 +
Gerrit-HasComments: No

[Impala-ASF-CR](asf-site) [DOCS] Impala 3.1 Docs to be published

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12046 )

Change subject: [DOCS] Impala 3.1 Docs to be published
..


Patch Set 1:

I think as part of this patch you also need to "git rm 
impala_abort_on_default_limit_exceeded.html impala_default_order_by_limit.html 
impala_max_io_buffers.html impala_reservation_request_timeout.html 
impala_scan_node_codegen_threshold.html impala_v_cpu_cores.html", because 
building docs from 5189300b2f9b12f147c8dfd39bd5e30613fcfbfe does not produce 
these files.

Sorry that we don't have tooling that does this or anything to catch it. Maybe 
before 3.2.0 we can whip something up.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-MessageType: comment
Gerrit-Change-Id: Icdfa4df8f75090b4783912713e2820eacbdfa904
Gerrit-Change-Number: 12046
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 06 Dec 2018 22:14:26 +
Gerrit-HasComments: No

[Impala-ASF-CR](asf-site) [DOCS] Impala 3.1 Docs to be published

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12046 )

Change subject: [DOCS] Impala 3.1 Docs to be published
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/12046/1//COMMIT_MSG@9
PS1, Line 9: Add Impala docs from branch master, commit hash
   : 5189300b2f9b12f147c8dfd39bd5e30613fcfbfe
Is this the correct hash to use?

  $ git fetch apache
  $ git diff apache/branch-3.1.0 5189300b2f9b12f147c8dfd39bd5e30613fcfbfe -- 
docs

shows ~600 lines of differences. Did one or more doc commits not make it 
into 3.1.0?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-MessageType: comment
Gerrit-Change-Id: Icdfa4df8f75090b4783912713e2820eacbdfa904
Gerrit-Change-Number: 12046
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 06 Dec 2018 20:41:02 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7759: Add Levenshtein edit distance built-in function

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11793 )

Change subject: IMPALA-7759: Add Levenshtein edit distance built-in function
..


Patch Set 3: Code-Review+1

(1 comment)

Soft +2 for the qgen changes.

http://gerrit.cloudera.org:8080/#/c/11793/3/tests/comparison/compat.py
File tests/comparison/compat.py:

http://gerrit.cloudera.org:8080/#/c/11793/3/tests/comparison/compat.py@21
PS3, Line 21: s
Oops, I had a typo here. Should be "exist".



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I549d33ab7cebfa10db2934461c8ec91e2cc1cdcb
Gerrit-Change-Number: 11793
Gerrit-PatchSet: 3
Gerrit-Owner: Greg Rahn 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 28 Nov 2018 16:26:48 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7761: Add multiple DISTINCT to targeted perf and stress test

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11805 )

Change subject: IMPALA-7761: Add multiple DISTINCT to targeted perf and stress 
test
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I400aaf6b6620b4001895eafff785956bffb312c9
Gerrit-Change-Number: 11805
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Marshall 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 09 Nov 2018 00:33:53 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7761: Add multiple DISTINCT to targeted perf and stress test

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11805 )

Change subject: IMPALA-7761: Add multiple DISTINCT to targeted perf and stress 
test
..


Patch Set 3:

This seems fine now. I'm checking how it behaves in a downstream environment on 
a larger TPCH scale factor. Will update after some runs.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I400aaf6b6620b4001895eafff785956bffb312c9
Gerrit-Change-Number: 11805
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Marshall 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 07 Nov 2018 23:00:42 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7809: support Kudu 1.9 in test concurrent schema change

[Michael Brown (Code Review)]

Michael Brown has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/11882


Change subject: IMPALA-7809: support Kudu 1.9 in test_concurrent_schema_change
..

IMPALA-7809: support Kudu 1.9 in test_concurrent_schema_change

This patch extends the match of allowable errors in
test_concurrent_schema_change to work with Kudu 1.9.

Testing: Local with a Kudu 1.9 minicluster environment.

Change-Id: I7fc24bb6a18aecc0cb726b8d66f0aeccf56bbb9b
---
M tests/query_test/test_kudu.py
1 file changed, 2 insertions(+), 1 deletion(-)



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

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

[Impala-ASF-CR] IMPALA-7761: Add multiple DISTINCT to targeted stress and perf

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11805 )

Change subject: IMPALA-7761: Add multiple DISTINCT to targeted stress and perf
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/11805/1//COMMIT_MSG@16
PS1, Line 16: Testing:
: - Ran the test file locally.
> we currently don't run anything in the stress test other than the regular 
> tpch/ds queries. Is that correct?

Correct.

> One problem with putting these queries in the tpch workload and changing the 
> regex is that there are already a number of query files in the tpch workload 
> other than the regular tpch queries which presumably weren't written with the 
> idea of being part of the stress test and aren't necessarily interesting for 
> it.

How is this a problem? The regex could opt in to the queries you add without 
including the ones we're not interested in. I don't personally see anything 
wrong with the stress- idea.

In general there is a problem with our test data and workload organization, 
because a workload is about query characteristics, but a given workload may 
depend on one or more different data sets, and it's kind of a mess. There's 
nothing to bind that workload with the data it needs. Putting everything into 
tpch at least helps keep that binding, albeit softly: "We load TPCH data, and 
here are the queries that use TPCH data".

> how would you feel if I resurrected the targeted-stress workload for this 
> purpose?

It depends on how you plan to implement it. agg_stress.test and 
stress-with-invalidate-refresh.test won't work for the parser because there are 
multiple statements in the files, so you can't just whitelist this entire 
directory. That will make your patch larger.

Note too --query-file-path exists.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I400aaf6b6620b4001895eafff785956bffb312c9
Gerrit-Change-Number: 11805
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Marshall 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 05 Nov 2018 19:10:49 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7761: Add multiple DISTINCT to targeted stress and perf

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11805 )

Change subject: IMPALA-7761: Add multiple DISTINCT to targeted stress and perf
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/11805/1//COMMIT_MSG@16
PS1, Line 16: Testing:
: - Ran the test file locally.
> By "run locally" I mean that I put together a python test that run the file
You could:

1. Put the queries into the tpch workload directory

2. Get them running regularly in query_test/test_tpch.py so that your queries 
run in regular CI immediately

3. Alter the stress test regex and unit test that makes sure all expected 
queries are there

Commentary:

The easiest way to get them into the stress test is to add queries, 1 query per 
.test file, to the tpch workload, and extend the regex to match the files you 
added. To prevent the regex/matching from regressing, there are CI tests to 
make sure we match an expected number of files (at least two bugs in the past 
have busted the matching when people altered the countents of the tpc workload 
directories). That test is here: 
https://github.com/apache/impala/blob/master/tests/infra/test_stress_infra.py#L53
 .

You could also run the queries in query_test/test_tpch.py, at which point 
you'll have regular CI on these queries as opposed to them being put into odd 
workloads.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I400aaf6b6620b4001895eafff785956bffb312c9
Gerrit-Change-Number: 11805
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Marshall 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 31 Oct 2018 17:54:08 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7783: Skip test default timezone when testing a real cluster.

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11820 )

Change subject: IMPALA-7783: Skip test_default_timezone when testing a real 
cluster.
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia4d4c503d2c77136cedd8f3fd830b6ce70d4457f
Gerrit-Change-Number: 11820
Gerrit-PatchSet: 3
Gerrit-Owner: David Knupp 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Tue, 30 Oct 2018 15:54:26 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6374: fix handling of commas in .test files

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11800 )

Change subject: IMPALA-6374: fix handling of commas in .test files
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I18ddcb0440490ddf8184be66d3681038a1615dd9
Gerrit-Change-Number: 11800
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 30 Oct 2018 15:30:55 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6374: fix handling of commas in .test files

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11800 )

Change subject: IMPALA-6374: fix handling of commas in .test files
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11800/3/tests/common/test_result_verifier.py
File tests/common/test_result_verifier.py:

http://gerrit.cloudera.org:8080/#/c/11800/3/tests/common/test_result_verifier.py@117
PS3, Line 117: i
You must mean "i + 1" here, yes?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I18ddcb0440490ddf8184be66d3681038a1615dd9
Gerrit-Change-Number: 11800
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Mon, 29 Oct 2018 22:54:56 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] CDH-73537: Skip test default timezone when testing a real cluster.

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11820 )

Change subject: CDH-73537: Skip test_default_timezone when testing a real 
cluster.
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11820/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11820/2//COMMIT_MSG@7
PS2, Line 7: CDH-73537
We need an upstream Jira here, not one for Cloudera.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia4d4c503d2c77136cedd8f3fd830b6ce70d4457f
Gerrit-Change-Number: 11820
Gerrit-PatchSet: 2
Gerrit-Owner: David Knupp 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Mon, 29 Oct 2018 22:30:35 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7761: Add multiple DISTINCT to targeted stress and perf

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11805 )

Change subject: IMPALA-7761: Add multiple DISTINCT to targeted stress and perf
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/11805/1//COMMIT_MSG@16
PS1, Line 16: Testing:
: - Ran the test file locally.
The queries are fine, but what does it mean to run these locally? I'm not too 
familiar with these workloads (targeted-perf, targeted-stress). Do the queries 
get run in GVO? Is it expected downstream larger-scale testing will run these? 
The stress test (concurrent_select.py) won't find these queries, for example.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I400aaf6b6620b4001895eafff785956bffb312c9
Gerrit-Change-Number: 11805
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Marshall 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 29 Oct 2018 22:29:11 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7673: Support values from other variables in Impala shell --var

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11623 )

Change subject: IMPALA-7673: Support values from other variables in Impala 
shell --var
..


Patch Set 9: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib5b9fda329c45f2e5682f3cbc76d29ceca2e226a
Gerrit-Change-Number: 11623
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Mon, 15 Oct 2018 16:20:11 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7690: Make test pool config change while queued compatible with python 2.6

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11660 )

Change subject: IMPALA-7690: Make test_pool_config_change_while_queued 
compatible with python 2.6
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id2593609e5be288054d1361f0fe57580e17ea042
Gerrit-Change-Number: 11660
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Pooja Nilangekar 
Gerrit-Comment-Date: Thu, 11 Oct 2018 20:52:06 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7693: stress test: fix Query().name

[Michael Brown (Code Review)]

Michael Brown has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/11651 )

Change subject: IMPALA-7693: stress test: fix Query().name
..

IMPALA-7693: stress test: fix Query().name

In the refactor as part of IMPALA-7460, loading of TPC queries no longer
returned query names (i.e., Q37). The name's presence doesn't change the
behavior of the stress test, but it does lead to nicer debuggable and
reable things, like log messages, profiles, result hashes, and runtime
info.

- Change load_tpc_queries() to return a dictionary, not a list.
- Set the .name attribute.
- Enhance the unit test to at least ensure load_tpc_queries() does not
  regress again.

Testing, in addition to passing test above:
- Ran stress test and performed binary search. Made sure query names
  were present in logging, runtime info, result hashes, and profiles.

Change-Id: Ie8c40beababf4c122dc8fed6c0544ee37871b9b2
Reviewed-on: http://gerrit.cloudera.org:8080/11651
Reviewed-by: Impala Public Jenkins 
Tested-by: Michael Brown 
---
M tests/infra/test_stress_infra.py
M tests/stress/concurrent_select.py
M tests/util/test_file_parser.py
3 files changed, 10 insertions(+), 5 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved
  Michael Brown: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie8c40beababf4c122dc8fed6c0544ee37871b9b2
Gerrit-Change-Number: 11651
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 

[Impala-ASF-CR] IMPALA-7693: stress test: fix Query().name

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11651 )

Change subject: IMPALA-7693: stress test: fix Query().name
..


Patch Set 2: Verified+1

> Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3301/

https://issues.apache.org/jira/browse/IMPALA-7696 which is unrelated.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie8c40beababf4c122dc8fed6c0544ee37871b9b2
Gerrit-Change-Number: 11651
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Thu, 11 Oct 2018 16:43:11 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7693: stress test: fix Query().name

[Michael Brown (Code Review)]

Michael Brown has removed a vote on this change.

Change subject: IMPALA-7693: stress test: fix Query().name
..


Removed Verified-1 by Impala Public Jenkins 
--
To view, visit http://gerrit.cloudera.org:8080/11651
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ie8c40beababf4c122dc8fed6c0544ee37871b9b2
Gerrit-Change-Number: 11651
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 

[Impala-ASF-CR] IMPALA-7693: stress test: fix Query().name

[Michael Brown (Code Review)]

Michael Brown has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/11651


Change subject: IMPALA-7693: stress test: fix Query().name
..

IMPALA-7693: stress test: fix Query().name

In the refactor as part of IMPALA-7460, loading of TPC queries no longer
returned query names (i.e., Q37). The name's presence doesn't change the
behavior of the stress test, but it does lead to nicer debuggable and
reable things, like log messages, profiles, result hashes, and runtime
info.

- Change load_tpc_queries() to return a dictionary, not a list.
- Set the .name attribute.
- Enhance the unit test to at least ensure load_tpc_queries() does not
  regress again.

Testing, in addition to passing test above:
- Ran stress test and performed binary search. Made sure query names
  were present in logging, runtime info, result hashes, and profiles.

Change-Id: Ie8c40beababf4c122dc8fed6c0544ee37871b9b2
---
M tests/infra/test_stress_infra.py
M tests/stress/concurrent_select.py
M tests/util/test_file_parser.py
3 files changed, 10 insertions(+), 5 deletions(-)



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

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

[Impala-ASF-CR] [DOCS] Built-in Functions doc format Changes

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11619 )

Change subject: [DOCS] Built-in Functions doc format Changes
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I797532463da8d29fe5bc7543cfdfb5b2b82db197
Gerrit-Change-Number: 11619
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Wed, 10 Oct 2018 18:15:18 +
Gerrit-HasComments: No

[Impala-ASF-CR] [DOCS] Built-in Functions doc format Changes

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11619 )

Change subject: [DOCS] Built-in Functions doc format Changes
..


Patch Set 2:

I don't think you used "git add" or something else went wrong for PS 2. The 
reasons for my suspicions:

1. The upload of PS2 just says "Uploaded patch set 2: Commit message was 
updated." This is all that Gerrit detected changed.

2. The diff between PS1 and PS2 (diff of diffs) doesn't show any meaningful 
changes to file. https://gerrit.cloudera.org/#/c/11619/1..2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I797532463da8d29fe5bc7543cfdfb5b2b82db197
Gerrit-Change-Number: 11619
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Tue, 09 Oct 2018 22:02:27 +
Gerrit-HasComments: No

[Impala-ASF-CR] [DOCS] Built-in Functions doc format Changes

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11619 )

Change subject: [DOCS] Built-in Functions doc format Changes
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11619/2/docs/topics/impala_conversion_functions.xml
File docs/topics/impala_conversion_functions.xml:

http://gerrit.cloudera.org:8080/#/c/11619/2/docs/topics/impala_conversion_functions.xml@221
PS2, Line 221: 
Why the decision to keep them around rather than just remove?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I797532463da8d29fe5bc7543cfdfb5b2b82db197
Gerrit-Change-Number: 11619
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Tue, 09 Oct 2018 20:43:17 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7647: Add HS2/Impyla dimension to TestQueries

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11546 )

Change subject: IMPALA-7647: Add HS2/Impyla dimension to TestQueries
..


Patch Set 7: Code-Review+1

This looks good, and because some downstream tests we run at Cloudera were 
affected, I made sure they pass in that environment too. The only failures are 
outstanding known issues. I'll let David do the final review since he had 
comments before.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9908ccc4d3df50365be8043b883cacafca52661e
Gerrit-Change-Number: 11546
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Nithya Janarthanan 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 08 Oct 2018 15:41:43 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7643: report # queries actually executing in stress test

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11587 )

Change subject: IMPALA-7643: report # queries actually executing in stress test
..


Patch Set 3: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11587/2/tests/stress/concurrent_select.py
File tests/stress/concurrent_select.py:

http://gerrit.cloudera.org:8080/#/c/11587/2/tests/stress/concurrent_select.py@1153
PS2, Line 1153: while True:
  :   query_state = cursor.status()
  :   # Check if the query got past the PENDING/INITIALIZED 
states, either because
  :   # it's executing or hit an error.
  :   if (not started_running_or_cancelled and query_state not 
in ('PENDING_STATE',
  :   
'INITIALIZED_STATE')):
  : started_run
> Good point. I tried to clarify it with some comments, lmk if it could be fu
Perfect, thanks.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5692e8e5ba3224becefc24437197bf5a5b450335
Gerrit-Change-Number: 11587
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 05 Oct 2018 23:13:40 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7643: report # queries actually executing in stress test

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11587 )

Change subject: IMPALA-7643: report # queries actually executing in stress test
..


Patch Set 2:

(4 comments)

Can you also do a quick binary search? It's fine if you do that just with TPCH 
or something. Sometimes that can regress.

http://gerrit.cloudera.org:8080/#/c/11587/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11587/2//COMMIT_MSG@17
PS2, Line 17: 2
1?


http://gerrit.cloudera.org:8080/#/c/11587/2/tests/stress/concurrent_select.py
File tests/stress/concurrent_select.py:

http://gerrit.cloudera.org:8080/#/c/11587/2/tests/stress/concurrent_select.py@1144
PS2, Line 1144: canel
cancel


http://gerrit.cloudera.org:8080/#/c/11587/2/tests/stress/concurrent_select.py@1153
PS2, Line 1153:   if (not started_running and query_state not in 
('PENDING_STATE',
  :   
'INITIALIZED_STATE')):
  : started_running = True
  : self.parent.increment_num_queries_started_running()
  :
  :   if query_state not in ('PENDING_STATE', 
'INITIALIZED_STATE', 'RUNNING_STATE'):
  : return True
Please leave a comment on the logic here. While it makes sense if you look at 
common/thrift/hive-1-api/TCLIService.thrift, it makes less sense if you're just 
trying to read the code in this file. In particular, the confusing part is that 
something called "increment_num_queries_started_running()" (with the word 
"running" in it) gets called if query_state = 'RUNNING_STATE', but we're still 
not ready to return True L1159 due to L1158. A bit of commenting makes the 
logic more clear for readers.


http://gerrit.cloudera.org:8080/#/c/11587/2/tests/stress/concurrent_select.py@1164
PS2, Line 1164: self._cancel(cursor, report)
  : if not started_running:
  :   self.parent.increment_num_queries_started_running()
I'm trying to understand the reasoning for the increment here. I think that 
means a comment is necessary. :)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5692e8e5ba3224becefc24437197bf5a5b450335
Gerrit-Change-Number: 11587
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 05 Oct 2018 17:57:22 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6249: Expose several build flags via web UI

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11410 )

Change subject: IMPALA-6249: Expose several build flags via web UI
..


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11410/6/tests/common/environ.py
File tests/common/environ.py:

http://gerrit.cloudera.org:8080/#/c/11410/6/tests/common/environ.py@249
PS6, Line 249:   IMPALAD_BUILD = ImpaladLocalBuild(IMPALA_HOME)
> @Michael, David do we support using Enums in our Python code in Impala? Or
I don't think we should introduce an external dependency (i.e., pip install 
something) for this.

Typical usage to get enums in Python is like:

A, B, C = range(3)

Example: 
https://github.com/apache/impala/blob/ea2809f5ddcf48f3f41dcd12743e8a17b4ea8cd7/tests/comparison/query.py#L45

In that example, the items were put in as class attributes in order to provide 
a clear namespace.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I47e3ad4cbf844909bdaf22a6f9d7bd915dce3f19
Gerrit-Change-Number: 11410
Gerrit-PatchSet: 6
Gerrit-Owner: Sahil Takiar 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Tue, 02 Oct 2018 22:08:50 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7589: default query options for custom cluster

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11463 )

Change subject: IMPALA-7589: default query options for custom cluster
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I14f60ea8746657a731e48850b0e48300a2b7c66d
Gerrit-Change-Number: 11463
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Thu, 20 Sep 2018 18:27:52 +
Gerrit-HasComments: No

[Impala-ASF-CR] update Flask to latest (1.0.2)

[Michael Brown (Code Review)]

Michael Brown has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/11335


Change subject: update Flask to latest (1.0.2)
..

update Flask to latest (1.0.2)

Similar to Fabric and Paramiko, make Flask part of extended test
requirements and upgrade it to its latest.

Testing:
- Impala builds, can do a full data load.
- Change works in my local environment.

Change-Id: Ibfc01d562e4d7fe48443d15074bd4c7d0176d2a0
---
M infra/python/deps/extended-test-requirements.txt
M infra/python/deps/requirements.txt
M tests/comparison/leopard/front_end.py
3 files changed, 9 insertions(+), 6 deletions(-)



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

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

[Impala-ASF-CR] IMPALA-7356 (part 2 of ?): restrict number of coordinators

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11316 )

Change subject: IMPALA-7356 (part 2 of ?): restrict number of coordinators
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43a6f095fc5274bf649833e0324815ebd266c0e2
Gerrit-Change-Number: 11316
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Fri, 24 Aug 2018 19:01:12 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7460 part 2: upgrade Paramiko and Fabric in extended test env

[Michael Brown (Code Review)]

Michael Brown has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/11308


Change subject: IMPALA-7460 part 2: upgrade Paramiko and Fabric in extended 
test env
..

IMPALA-7460 part 2: upgrade Paramiko and Fabric in extended test env

Upgrade Paramiko to the latest, 2.4.1. Paramiko drastically changed its
dependencies in Paramiko 2, dropping defunct Pycrypto and using Cryptography
instead.

https://github.com/paramiko/paramiko/issues/637
https://github.com/paramiko/paramiko/pull/394

This change implicitly removes the dependency on Pycrypto.

Also upgrade Fabric to the latest 1.x version, 1.14.0.

Testing:
- This works in my development environment.
- This works in my downstream stress and query gen environments.
- This works when doing a full data load.
- Impala still builds on a variety of OSs.

Change-Id: I0636d8113be449953420e1d5773f63d7c91943e3
---
M infra/python/deps/extended-test-requirements.txt
1 file changed, 7 insertions(+), 6 deletions(-)



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

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

[Impala-ASF-CR] IMPALA-7479: Harmonize parquet versions.

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11299 )

Change subject: IMPALA-7479: Harmonize parquet versions.
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia47b0871f25171510d7cb39593f3e94aadb9adeb
Gerrit-Change-Number: 11299
Gerrit-PatchSet: 2
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Thu, 23 Aug 2018 15:25:00 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7399: Emit a junit xml report when trapping errors

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11257 )

Change subject: IMPALA-7399: Emit a junit xml report when trapping errors
..


Patch Set 9: Code-Review+1

(2 comments)

Phil had good comments. I had looked at an earlier patch set and didn't have 
much more to say. This looks good to me now though.

http://gerrit.cloudera.org:8080/#/c/11257/9/lib/python/impala_py_lib/jenkins/generate_junitxml.py
File lib/python/impala_py_lib/jenkins/generate_junitxml.py:

http://gerrit.cloudera.org:8080/#/c/11257/9/lib/python/impala_py_lib/jenkins/generate_junitxml.py@164
PS9, Line 164: {}
Not python2.6-compatible. Use {0}


http://gerrit.cloudera.org:8080/#/c/11257/9/testdata/bin/check-schema-diff.sh
File testdata/bin/check-schema-diff.sh:

http://gerrit.cloudera.org:8080/#/c/11257/9/testdata/bin/check-schema-diff.sh@30
PS9, Line 30: the same trap function used by other shell scripts
just say "we don't call setup_report_build_error".



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd62045bb43357abc2b89a78afff499149d3c3fc
Gerrit-Change-Number: 11257
Gerrit-PatchSet: 9
Gerrit-Owner: David Knupp 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Nithya Janarthanan 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Wed, 22 Aug 2018 22:00:43 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7460 part 1: require user to install Paramiko and Fabric

[Michael Brown (Code Review)]

Hello Lars Volker, Philip Zeyliger, David Knupp, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-7460 part 1: require user to install Paramiko and Fabric
..

IMPALA-7460 part 1: require user to install Paramiko and Fabric

- Remove Fabric and Paramiko as requirements. They aren't needed by
  anything in buildall.sh.
- Add a means to install into the impala-python virtual environment by hand.
  impala-pip is fine for this.
- Add another requirements file for extended testing. The dependency
  situation is messy and untangling that out of impala-python and into
  lib/python should be out of the scope of IMPALA-7460.
- Update core tests, which cover real regressions that have happened in
  the past, to run against locations that don't require a Paramiko
  import. This moves some logic out of concurrent_select.py into a
  thinner module.
- Insulate ssh_util from globally-scoped import so that it only imports
  when needed.

Testing:
- This works in my development environment.
- This works in my downstream stress and query gen environments.
- This works when doing a full data load.
- Impala still builds on a variety of OSs.

Todo:
- A subsequent review will update the versions.

Change-Id: Ibf9010a0387b52c95b7bda5d1d4606eba1008b65
---
A bin/impala-pip
M infra/python/deps/compiled-requirements.txt
A infra/python/deps/extended-test-requirements.txt
M tests/comparison/cluster.py
M tests/comparison/leopard/impala_docker_env.py
M tests/infra/test_stress_infra.py
M tests/stress/concurrent_select.py
M tests/util/cluster_controller.py
M tests/util/parse_util.py
M tests/util/ssh_util.py
M tests/util/test_file_parser.py
11 files changed, 157 insertions(+), 82 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/11264/5
--
To view, visit http://gerrit.cloudera.org:8080/11264
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibf9010a0387b52c95b7bda5d1d4606eba1008b65
Gerrit-Change-Number: 11264
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 

[Impala-ASF-CR] WIP: IMPALA-7460 part 1: require user to install Paramiko and Fabric

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11264 )

Change subject: WIP: IMPALA-7460 part 1: require user to install Paramiko and 
Fabric
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibf9010a0387b52c95b7bda5d1d4606eba1008b65
Gerrit-Change-Number: 11264
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Wed, 22 Aug 2018 20:12:05 +
Gerrit-HasComments: No

[Impala-ASF-CR] WIP: IMPALA-7460 part 1: require user to install Paramiko and Fabric

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11264 )

Change subject: WIP: IMPALA-7460 part 1: require user to install Paramiko and 
Fabric
..


Patch Set 4:

> Expect another non-trivial update.

It turns out to fix data load I just had to move an import to be more 
localized. I also updated the testing section of the commit message.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibf9010a0387b52c95b7bda5d1d4606eba1008b65
Gerrit-Change-Number: 11264
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Wed, 22 Aug 2018 19:22:27 +
Gerrit-HasComments: No

[Impala-ASF-CR] WIP: IMPALA-7460 part 1: require user to install Paramiko and Fabric

[Michael Brown (Code Review)]

Hello Lars Volker, Philip Zeyliger, David Knupp, Impala Public Jenkins,

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

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

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

Change subject: WIP: IMPALA-7460 part 1: require user to install Paramiko and 
Fabric
..

WIP: IMPALA-7460 part 1: require user to install Paramiko and Fabric

- Remove Fabric and Paramiko as requirements. They aren't needed by
  anything in buildall.sh.
- Add a means to install into the impala-python virtual environment by hand.
  impala-pip is fine for this.
- Add another requirements file for extended testing. The dependency
  situation is messy and untangling that out of impala-python and into
  lib/python should be out of the scope of IMPALA-7460.
- Update core tests, which cover real regressions that have happened in
  the past, to run against locations that don't require a Paramiko
  import. This moves some logic out of concurrent_select.py into a
  thinner module.
- Insulate ssh_util from globally-scoped import so that it only imports
  when needed.

Testing:
- This works in my development environment.
- This works in my downstream stress and query gen environments.
- This works when doing a full data load.
- Impala still builds on a variety of OSs.

Todo:
- A subsequent review will update the versions.

Change-Id: Ibf9010a0387b52c95b7bda5d1d4606eba1008b65
---
A bin/impala-pip
M infra/python/deps/compiled-requirements.txt
A infra/python/deps/extended-test-requirements.txt
M tests/comparison/cluster.py
M tests/comparison/leopard/impala_docker_env.py
M tests/infra/test_stress_infra.py
M tests/stress/concurrent_select.py
M tests/util/cluster_controller.py
M tests/util/parse_util.py
M tests/util/ssh_util.py
M tests/util/test_file_parser.py
11 files changed, 157 insertions(+), 82 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/11264/4
--
To view, visit http://gerrit.cloudera.org:8080/11264
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibf9010a0387b52c95b7bda5d1d4606eba1008b65
Gerrit-Change-Number: 11264
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 

[Impala-ASF-CR] WIP: IMPALA-7460 part 1: require user to install Paramiko and Fabric

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11264 )

Change subject: WIP: IMPALA-7460 part 1: require user to install Paramiko and 
Fabric
..


Patch Set 3:

> Thanks. I'm considering this still a WIP as I do some testing not
 > covered by GVO. I'll carry this if I only make trivial changes.

This breaks data load, so I'm going to have to iron that out. Expect another 
non-trivial update.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibf9010a0387b52c95b7bda5d1d4606eba1008b65
Gerrit-Change-Number: 11264
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Tue, 21 Aug 2018 17:02:16 +
Gerrit-HasComments: No

[Impala-ASF-CR] WIP: IMPALA-7460 part 1: require user to install Paramiko and Fabric

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11264 )

Change subject: WIP: IMPALA-7460 part 1: require user to install Paramiko and 
Fabric
..


Patch Set 3:

> Patch Set 3: Code-Review+2

Thanks. I'm considering this still a WIP as I do some testing not covered by 
GVO. I'll carry this if I only make trivial changes.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibf9010a0387b52c95b7bda5d1d4606eba1008b65
Gerrit-Change-Number: 11264
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Tue, 21 Aug 2018 16:13:29 +
Gerrit-HasComments: No

[Impala-ASF-CR] WIP: IMPALA-7460 part 1: require user to install Paramiko and Fabric

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11264 )

Change subject: WIP: IMPALA-7460 part 1: require user to install Paramiko and 
Fabric
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11264/2/bin/impala-pip
File bin/impala-pip:

http://gerrit.cloudera.org:8080/#/c/11264/2/bin/impala-pip@21
PS2, Line 21: exec "$PY_DIR/env/bin/pip" "$@"
> This is funny. I'm surprised it took this long for someone to create an imp
Thank you for catching this. Done.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibf9010a0387b52c95b7bda5d1d4606eba1008b65
Gerrit-Change-Number: 11264
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Tue, 21 Aug 2018 15:39:11 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] WIP: IMPALA-7460 part 1: require user to install Paramiko and Fabric

[Michael Brown (Code Review)]

Hello Lars Volker, Philip Zeyliger, David Knupp,

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

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

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

Change subject: WIP: IMPALA-7460 part 1: require user to install Paramiko and 
Fabric
..

WIP: IMPALA-7460 part 1: require user to install Paramiko and Fabric

- Remove Fabric and Paramiko as requirements. They aren't needed by
  anything in buildall.sh.
- Add a means to install into the impala-python virtual environment by hand.
  impala-pip is fine for this.
- Add another requirements file for extended testing. The dependency
  situation is messy and untangling that out of impala-python and into
  lib/python should be out of the scope of IMPALA-7460.
- Update core tests, which cover real regressions that have happened in
  the past, to run against locations that don't require a Paramiko
  import. This moves some logic out of concurrent_select.py into a
  thinner module.

Testing:
- This works on my development environment. I still need to test it in a
 few other environments and run exhaustive tests.

Todo:
- This diff keeps the same versions as we had before. Either a
  subsequent patch set or review will update the versions.

Change-Id: Ibf9010a0387b52c95b7bda5d1d4606eba1008b65
---
A bin/impala-pip
M infra/python/deps/compiled-requirements.txt
A infra/python/deps/extended-test-requirements.txt
M tests/comparison/leopard/impala_docker_env.py
M tests/infra/test_stress_infra.py
M tests/stress/concurrent_select.py
M tests/util/cluster_controller.py
M tests/util/parse_util.py
M tests/util/ssh_util.py
M tests/util/test_file_parser.py
10 files changed, 154 insertions(+), 81 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibf9010a0387b52c95b7bda5d1d4606eba1008b65
Gerrit-Change-Number: 11264
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 

[Impala-ASF-CR] WIP: IMPALA-7460 part 1: require user to install Paramiko and Fabric

[Michael Brown (Code Review)]

Michael Brown has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/11264


Change subject: WIP: IMPALA-7460 part 1: require user to install Paramiko and 
Fabric
..

WIP: IMPALA-7460 part 1: require user to install Paramiko and Fabric

- Remove Fabric and Paramiko as requirements. They aren't needed by
  anything in buildall.sh.
- Add a means to install into the impala-python virtual environment by hand.
  impala-pip is fine for this.
- Add another requirements file for extended testing. The dependency
  situation is messy and untangling that out of impala-python and into
  lib/python should be out of the scope of IMPALA-7460.
- Update core tests, which cover real regressions that have happened in
  the past, to run against locations that don't require a Paramiko
  import. This moves some logic out of concurrent_select.py into a
  thinner module.

Testing:
- This works on my development environment. I still need to test it in a
 few other environments and run exhaustive tests.

Todo:
- This diff keeps the same versions as we had before. Either a
  subsequent patch set or review will update the versions.

Change-Id: Ibf9010a0387b52c95b7bda5d1d4606eba1008b65
---
A bin/impala-pip
M infra/python/deps/compiled-requirements.txt
A infra/python/deps/extended-test-requirements.txt
M tests/comparison/leopard/impala_docker_env.py
M tests/infra/test_stress_infra.py
M tests/stress/concurrent_select.py
M tests/util/cluster_controller.py
M tests/util/parse_util.py
M tests/util/ssh_util.py
M tests/util/test_file_parser.py
10 files changed, 154 insertions(+), 81 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibf9010a0387b52c95b7bda5d1d4606eba1008b65
Gerrit-Change-Number: 11264
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 

[Impala-ASF-CR] tests: remove unused failure injector test library

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11226 )

Change subject: tests: remove unused failure_injector test library
..


Patch Set 1: Code-Review+2

We know where we can find it, so sure.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ae8aa13acc85ec2f3a345d460babdb8c55a56cd
Gerrit-Change-Number: 11226
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Fri, 17 Aug 2018 15:54:29 +
Gerrit-HasComments: No

[Impala-ASF-CR] tests: ensure consistent logging format across tests

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11225 )

Change subject: tests: ensure consistent logging format across tests
..


Patch Set 1: Code-Review+2

(1 comment)

Than you for doing this.

http://gerrit.cloudera.org:8080/#/c/11225/1/tests/conftest.py
File tests/conftest.py:

http://gerrit.cloudera.org:8080/#/c/11225/1/tests/conftest.py@56
PS1, Line 56:   logging.basicConfig(level=logging.INFO, format='-- 
%(threadName)s: %(message)s')
> Should we put a timestamp in here? We have a variety of these that do have
+1 for a timestamp



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55ef0214b43f87da2d71804913ba4caa964f789f
Gerrit-Change-Number: 11225
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 17 Aug 2018 15:53:42 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7356 (part 1 of ?): admission control stress

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11205 )

Change subject: IMPALA-7356 (part 1 of ?): admission control stress
..


Patch Set 5: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11205/5/tests/stress/concurrent_select.py
File tests/stress/concurrent_select.py:

http://gerrit.cloudera.org:8080/#/c/11205/5/tests/stress/concurrent_select.py@450
PS5, Line 450: "AC Reject", "AC Timeout",
> I thought about that too. I decided not to handle that case separately, par
Sounds fine. We can think about the scrolling columns issue separately.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id31a77f1fe6854a56ce54d1de333793e18087be4
Gerrit-Change-Number: 11205
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 14 Aug 2018 18:09:45 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7356 (part 1 of ?): admission control stress

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11205 )

Change subject: IMPALA-7356 (part 1 of ?): admission control stress
..


Patch Set 5:

(1 comment)

Seems fine and straightforward, just one usability question below.

http://gerrit.cloudera.org:8080/#/c/11205/5/tests/stress/concurrent_select.py
File tests/stress/concurrent_select.py:

http://gerrit.cloudera.org:8080/#/c/11205/5/tests/stress/concurrent_select.py@450
PS5, Line 450: "AC Reject", "AC Timeout",
Do you have thoughts on always printing these columns vs. only printing these 
columns when --test-admission-control=true ?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id31a77f1fe6854a56ce54d1de333793e18087be4
Gerrit-Change-Number: 11205
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 14 Aug 2018 17:07:11 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7428: Fix flaky test shell command line::test large sql

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11201 )

Change subject: IMPALA-7428: Fix flaky test_shell_command_line::test_large_sql
..


Patch Set 3: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11201/2/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:

http://gerrit.cloudera.org:8080/#/c/11201/2/tests/shell/test_shell_commandline.py@688
PS2, Line 688: os.write(sql_file, "col_{0} as b{1},\n".format(i, i))
 : os.write(sql_file, "col_{0} as c{1}{2}\n".format(
 : i, i, "," if i < num_cols - 1 else ""))
 : os.write(sql_file, "from non_existence_large_ta
> With the rewrite, the test now finishes in 1 second or less :) I don't thin
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic87891f34872da65aac5ce02caf01da1c050efa5
Gerrit-Change-Number: 11201
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Tue, 14 Aug 2018 16:51:47 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7428: Fix flaky test shell command line::test large sql

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11201 )

Change subject: IMPALA-7428: Fix flaky test_shell_command_line::test_large_sql
..


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/11201/2/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:

http://gerrit.cloudera.org:8080/#/c/11201/2/tests/shell/test_shell_commandline.py@681
PS2, Line 681: non-existence
non-existent


http://gerrit.cloudera.org:8080/#/c/11201/2/tests/shell/test_shell_commandline.py@681
PS2, Line 681: to be
remove "to be " ?


http://gerrit.cloudera.org:8080/#/c/11201/2/tests/shell/test_shell_commandline.py@688
PS2, Line 688: os.write(sql_file, 'col_{0} as a{1},\n'.format(i, i))
 : os.write(sql_file, 'col_{0} as b{1},\n'.format(i, i))
 : os.write(sql_file, 'col_{0} as c{1}{2}\n'.format(
 : i, i, ',' if i < num_cols - 1 else ''))
If you're concerned about speed, do you want to consolidate these writes() into 
one?

You can use multiline python strings with triple quotes and textwrap.dedent() 
to generate the string without leading spaces, if you wish. You can also use 
keyword-based formatting for readability, like "{i}".format(i=i).



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic87891f34872da65aac5ce02caf01da1c050efa5
Gerrit-Change-Number: 11201
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Tue, 14 Aug 2018 16:40:25 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] Ignore flake8 W503 about breaking before operators

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11196 )

Change subject: Ignore flake8 W503 about breaking before operators
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieedde356f613ce57caf3fe190fb44e0404337f97
Gerrit-Change-Number: 11196
Gerrit-PatchSet: 1
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 13 Aug 2018 16:19:30 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7317: loosen flake8 rules

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11102 )

Change subject: IMPALA-7317: loosen flake8 rules
..


Patch Set 2: Code-Review+2

(1 comment)

Feel free to carry +2 after addressing the comment.

http://gerrit.cloudera.org:8080/#/c/11102/2/setup.cfg
File setup.cfg:

http://gerrit.cloudera.org:8080/#/c/11102/2/setup.cfg@23
PS2, Line 23: ignore = E111,E114,E121,E125,E128,E701
E127 is mentioned L19 but absent L23.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaaa8377025e732231ac0f1f1f2db67298a171997
Gerrit-Change-Number: 11102
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 01 Aug 2018 23:29:17 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7317: loosen flake8 rules

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11102 )

Change subject: IMPALA-7317: loosen flake8 rules
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11102/1/setup.cfg
File setup.cfg:

http://gerrit.cloudera.org:8080/#/c/11102/1/setup.cfg@23
PS1, Line 23: ignore = E111,E114,E121,E128,E701
> I'm not sure the "wrong" example is really wrong, at least according to fla
It sounds as if we agree that by choosing 2-space indents, we've confused the 
tool. However, it is possible to make the tool happy without exclusions. That 
is what I was trying to point out.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaaa8377025e732231ac0f1f1f2db67298a171997
Gerrit-Change-Number: 11102
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 01 Aug 2018 22:20:37 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7317: loosen flake8 rules

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11102 )

Change subject: IMPALA-7317: loosen flake8 rules
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/11102/1/setup.cfg
File setup.cfg:

http://gerrit.cloudera.org:8080/#/c/11102/1/setup.cfg@19
PS1, Line 19: # E121,E127,E128: ignored to allow some of the more common idioms 
for multi-line statement
: # indentation used in the Impala code.
Excluding these leads to a broader interpretation of how Python whitespace 
should be applied and thus inconsistencies in style. I don't think keeping them 
enforced would produce an undue burden on submitters, because we are using 
--diff and not "punishing" a submitter for existing code that violates these.

However, I won't block this if the community prefers.


http://gerrit.cloudera.org:8080/#/c/11102/1/setup.cfg@21
PS1, Line 21: # E701: ignored to allow if statement body on same line as 
conditional
I can live with this, as PEP-008 doesn't forbid it outright.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaaa8377025e732231ac0f1f1f2db67298a171997
Gerrit-Change-Number: 11102
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 01 Aug 2018 21:58:32 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7317: loosen flake8 rules

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11102 )

Change subject: IMPALA-7317: loosen flake8 rules
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11102/1/setup.cfg
File setup.cfg:

http://gerrit.cloudera.org:8080/#/c/11102/1/setup.cfg@23
PS1, Line 23: ignore = E111,E114,E121,E128,E701
> I'm getting issues with E125 also with code like this:
The correct formatting for the above is this:

  def long_function_name_so_i_need_to_wrap_params(
  long, parameter, list
  ):
pass

Also acceptable:

  def long_function_name_so_i_need_to_wrap_params2(long, parameter, list):
pass


  def long_function_name_so_i_need_to_wrap_params3(long, parameter,
   list):
pass

Wrong, and I see this a lot:

  def long_function_name_so_i_need_to_wrap_params4(long, parameter,
  list):
pass

Note that with the very first example, a call is treated the way you'd prefer. 
This passes:

  long_function_name_so_i_need_to_wrap_params(
  1, 1, 1)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaaa8377025e732231ac0f1f1f2db67298a171997
Gerrit-Change-Number: 11102
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 01 Aug 2018 21:55:26 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11054 )

Change subject: IMPALA-7317: add scripts to post flake8 comments
..


Patch Set 17: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 17
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 30 Jul 2018 22:56:32 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11054 )

Change subject: IMPALA-7317: add scripts to post flake8 comments
..


Patch Set 14:

Seems fine. Have you yet tested reviews with non-Python files that flake8 won't 
understand?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 14
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 30 Jul 2018 21:54:30 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-110 (part 3): Add multiple DISTINCT support to query generator

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11073 )

Change subject: IMPALA-110 (part 3): Add multiple DISTINCT support to query 
generator
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4a3f14655719ade7b2f6471c561dba4007fd46fa
Gerrit-Change-Number: 11073
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Marshall 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Sun, 29 Jul 2018 20:48:15 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7173: [DOCS] Added check options in the load balancer examples

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11006 )

Change subject: IMPALA-7173: [DOCS] Added check options in the load balancer 
examples
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic5148aecf4605373ea6a28bf032bcfdab4f822fd
Gerrit-Change-Number: 11006
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alan Choi 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Laurel Hale 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Mon, 23 Jul 2018 17:43:46 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6923: Update scripts in benchmark folder to store workload and few minor updates

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10100 )

Change subject: IMPALA-6923: Update scripts in benchmark folder to store 
workload and few minor updates
..


Patch Set 12:

(5 comments)

I think the changes are OK for the most part, but they could stand to use 
better Python conventions. One of my inline comments explains how to do that.

http://gerrit.cloudera.org:8080/#/c/10100/12/tests/benchmark/perf_result_datastore.py
File tests/benchmark/perf_result_datastore.py:

http://gerrit.cloudera.org:8080/#/c/10100/12/tests/benchmark/perf_result_datastore.py@23
PS12, Line 23: from subprocess import *
"*" is an antipattern for imports. It is better to be explicit about imported 
symbols.


http://gerrit.cloudera.org:8080/#/c/10100/12/tests/benchmark/perf_result_datastore.py@63
PS12, Line 63:   def insert_workload_summary(self, run_info_id):
 : self._insert_workload_summary(run_info_id)
This doesn't seem to do anything useful, so just rename 
_insert_workload_summary to insert_workload_summary and remove this.


http://gerrit.cloudera.org:8080/#/c/10100/12/tests/benchmark/perf_result_datastore.py@85
PS12, Line 85:   dont_save_profiles):
Negatives are weird. It would be better to call this save_profiles and flip 
conditions. The reason being, readability for stuff like:

  if not dont_save_profiles

gets confusing.


http://gerrit.cloudera.org:8080/#/c/10100/12/tests/benchmark/perf_result_datastore.py@88
PS12, Line 88: temp_profile_folder = 
os.path.join(home_dir,"workspace","impala-workload-runner","profiles")
This is badly formated. There should be spaces between parameters. 
impala-flake8 perf_result_datastore.py should help find problems like this. 
Please run it against this file.

You don't have to fix all the existing flake8 errors. But for your own 
submissions, you should format them better.

You can do that via "git diff HEAD^ | impala-flake8 --diff" from the root 
Impala directory.


http://gerrit.cloudera.org:8080/#/c/10100/12/tests/benchmark/perf_result_datastore.py@155
PS12, Line 155: self._cache: Dictionary of with run_info/user as 
key/value
This isn't quite right. self._cache appears to be a hash in which a tuple of 
(run_info, user) is the key.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ica7c00ad59963d466bae9e607a4692af0138962c
Gerrit-Change-Number: 10100
Gerrit-PatchSet: 12
Gerrit-Owner: Nithya Janarthanan 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Jinchul Kim 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Nithya Janarthanan 
Gerrit-Comment-Date: Wed, 18 Jul 2018 23:38:37 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7314: Doc generation should fail on error

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10976 )

Change subject: IMPALA-7314: Doc generation should fail on error
..


Patch Set 2: Code-Review+2

I'm happy to promote my +1. For docs, use 
https://jenkins.impala.io/job/gerrit-docs-submit/build


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic452aa282a3f2a761e3b04a7460e0d86bc51d721
Gerrit-Change-Number: 10976
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 18 Jul 2018 23:25:08 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7314: Doc generation should fail on error

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10976 )

Change subject: IMPALA-7314: Doc generation should fail on error
..


Patch Set 2: Code-Review+1

Please make sure Alex understands and agrees with the workflow changes.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic452aa282a3f2a761e3b04a7460e0d86bc51d721
Gerrit-Change-Number: 10976
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 18 Jul 2018 22:16:40 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6810: runtime row filters.test: omit pool name in pattern

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10942 )

Change subject: IMPALA-6810: runtime_row_filters.test: omit pool name in pattern
..


Patch Set 2: Code-Review+2

rebase


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3fe6beb169dc6bfefabde9dc7a4632c1a5e63fa7
Gerrit-Change-Number: 10942
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 13 Jul 2018 17:27:06 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6810: runtime row filters.test: omit pool name in pattern

[Michael Brown (Code Review)]

Michael Brown has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10942


Change subject: IMPALA-6810: runtime_row_filters.test: omit pool name in pattern
..

IMPALA-6810: runtime_row_filters.test: omit pool name in pattern

Some downstream tests run this with a fair-scheduler.xml set that, while
not changing admission control behavior, does change the name of the
pool. Omit the pool name to permit that downstream test to succeed.

Testing:
- local with change in minicluster
- downstream in environment as well

Change-Id: I3fe6beb169dc6bfefabde9dc7a4632c1a5e63fa7
---
M testdata/workloads/functional-query/queries/QueryTest/runtime_row_filters.test
1 file changed, 3 insertions(+), 4 deletions(-)



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

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

[Impala-ASF-CR] IMPALA-7259: Improve Impala shell performance

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10939 )

Change subject: IMPALA-7259: Improve Impala shell performance
..


Patch Set 7: Code-Review+1

(1 comment)

for the test

http://gerrit.cloudera.org:8080/#/c/10939/7/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:

http://gerrit.cloudera.org:8080/#/c/10939/7/tests/shell/test_shell_commandline.py@31
PS7, Line 31: from time import sleep
: from time import time
Sorry, you can do:

  from time import sleep, time



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idac9f3caed7c44846a8c922dbe5ca3bf3b095b81
Gerrit-Change-Number: 10939
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 13 Jul 2018 17:03:08 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7238: Use custom timeout for create unique database

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10862 )

Change subject: IMPALA-7238: Use custom timeout for create unique database
..


Patch Set 1: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10862/1/tests/conftest.py
File tests/conftest.py:

http://gerrit.cloudera.org:8080/#/c/10862/1/tests/conftest.py@359
PS1, Line 359: db_name=db_name
Missing timeout=timeout here as well.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4f2beb5bc027a4bb44e854bf1dd8919807a92ea0
Gerrit-Change-Number: 10862
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 09 Jul 2018 17:04:05 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6119: Fix issue with multiple partitions sharing same location

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10543 )

Change subject: IMPALA-6119: Fix issue with multiple partitions sharing same 
location
..


Patch Set 20:

(1 comment)

+2 for test_partition_metadata.py

http://gerrit.cloudera.org:8080/#/c/10543/18/tests/metadata/test_partition_metadata.py
File tests/metadata/test_partition_metadata.py:

http://gerrit.cloudera.org:8080/#/c/10543/18/tests/metadata/test_partition_metadata.py@127
PS18, Line 127: table %
> Done
Thanks for updating. I hadn't intended for you to update the entire module, 
just the tests you were working on. I'm sorry for not being clearer on that.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2a54bc8224bcefe65b83de2df58bb84629f2aa4a
Gerrit-Change-Number: 10543
Gerrit-PatchSet: 20
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Fri, 29 Jun 2018 17:46:51 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5900: [DOCS] Doc fe service threads startup option

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10795 )

Change subject: IMPALA-5900: [DOCS] Doc fe_service_threads startup  option
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f0a417a4aa07b8082037fc6ff355e62ce1493e5
Gerrit-Change-Number: 10795
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Fri, 22 Jun 2018 17:43:38 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5900: [DOCS] Doc fe service threads startup option

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10795 )

Change subject: IMPALA-5900: [DOCS] Doc fe_service_threads startup  option
..


Patch Set 1:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/10795/1/docs/topics/impala_config_options.xml
File docs/topics/impala_config_options.xml:

http://gerrit.cloudera.org:8080/#/c/10795/1/docs/topics/impala_config_options.xml@a305
PS1, Line 305:
What's the plan for this? Why remove all of it?


http://gerrit.cloudera.org:8080/#/c/10795/1/docs/topics/impala_config_options.xml@320
PS1, Line 320: reads and writes
"reads from and writes to"?


http://gerrit.cloudera.org:8080/#/c/10795/1/docs/topics/impala_config_options.xml@330
PS1, Line 330: clients
 :   has
subject-verb disagreement


http://gerrit.cloudera.org:8080/#/c/10795/1/docs/topics/impala_config_options.xml@332
PS1, Line 332: connection
connections


http://gerrit.cloudera.org:8080/#/c/10795/1/docs/topics/impala_config_options.xml@333
PS1, Line 333: most of connections
either "most of the" or "most", not "most of?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f0a417a4aa07b8082037fc6ff355e62ce1493e5
Gerrit-Change-Number: 10795
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Fri, 22 Jun 2018 17:25:22 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6119: Fix issue with multiple partitions sharing same location

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10543 )

Change subject: IMPALA-6119: Fix issue with multiple partitions sharing same 
location
..


Patch Set 18:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/10543/18/tests/metadata/test_partition_metadata.py
File tests/metadata/test_partition_metadata.py:

http://gerrit.cloudera.org:8080/#/c/10543/18/tests/metadata/test_partition_metadata.py@124
PS18, Line 124: # Create three partitions where two of them point to the 
same location.
  : self.filesystem_client.make_dir(TBL_NAME + '/p1')
  : self.filesystem_client.make_dir(TBL_NAME + '/p2')
This looks wrong, because the directories made here will only be 
"same_loc_test/{p1,p2}" and not rooted at TBL_LOCATION. Are these lines needed?

And if they are needed, then they need to have a uniqueness to them, because as 
it stands, parallel tests will overwrite each other's directories in this spot. 
Include unique_database as part of the directory name to solve this problem.


http://gerrit.cloudera.org:8080/#/c/10543/18/tests/metadata/test_partition_metadata.py@127
PS18, Line 127: execute
Here and elsewhere, there's a execute_query_expect_success() method you should 
consider instead for statements you expect always to succeed.


http://gerrit.cloudera.org:8080/#/c/10543/18/tests/metadata/test_partition_metadata.py@163
PS18, Line 163: it's
Are we (Impala) in control of this message? It should be "its".


http://gerrit.cloudera.org:8080/#/c/10543/18/tests/metadata/test_partition_metadata.py@168
PS18, Line 168: assert false
If we are here, then we never raised an exception. It would be polite to show 
the partition information as part of diagnostics when the test fails.


http://gerrit.cloudera.org:8080/#/c/10543/18/tests/metadata/test_partition_metadata.py@177
PS18, Line 177: pytest.skip()
You can insert a reason here. It's mildly helpful to someone that happens to 
see this go by on a console.


http://gerrit.cloudera.org:8080/#/c/10543/18/tests/metadata/test_partition_metadata.py@184
PS18, Line 184: # Cleanup any existing data in the table directory.
  : self.filesystem_client.delete_file_dir(TBL_NAME, 
recursive=True)
As before, I can't tell that this is needed.

As before, if needed, this can collide with tests that are using a directory of 
TBL_NAME in L125-126, assuming the pwd is the same.


http://gerrit.cloudera.org:8080/#/c/10543/18/tests/metadata/test_partition_metadata.py@187
PS18, Line 187: execute
As before, execute_query_expect_success() in places where you expect success.


http://gerrit.cloudera.org:8080/#/c/10543/18/tests/metadata/test_partition_metadata.py@191
PS18, Line 191: # Create two partitions pointing to the same location.
  : self.filesystem_client.make_dir(TBL_NAME + '/p1')
Same comment for L185 applies here.


http://gerrit.cloudera.org:8080/#/c/10543/18/tests/metadata/test_partition_metadata.py@202
PS18, Line 202: hive_cmd = ("ALTER TABLE %s DROP PARTITION (j=1)" % 
FQ_TBL_NAME)
  : call(["hive", "-e", hive_cmd])
You can use self.hive_client() instead.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2a54bc8224bcefe65b83de2df58bb84629f2aa4a
Gerrit-Change-Number: 10543
Gerrit-PatchSet: 18
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Fri, 22 Jun 2018 16:00:59 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-2195: Improper handling of comments in queries

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9933 )

Change subject: IMPALA-2195: Improper handling of comments in queries
..


Patch Set 14: Code-Review+2

Seems like we're loosening things a bit, and this was also tested in Python 2.6


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7ac7cb5a30e6dda73ebe761d9f0eb9ba038e14a7
Gerrit-Change-Number: 9933
Gerrit-PatchSet: 14
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Comment-Date: Tue, 12 Jun 2018 21:10:16 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2751: Matching quotes are not required in comments

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10541 )

Change subject: IMPALA-2751: Matching quotes are not required in comments
..


Patch Set 6: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2feae34026a7e63f3d31489f757f093a73ca5d2c
Gerrit-Change-Number: 10541
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 12 Jun 2018 21:08:49 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2195: Improper handling of comments in queries

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9933 )

Change subject: IMPALA-2195: Improper handling of comments in queries
..


Patch Set 14:

+1 since we are trying to ease changes in gently still.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7ac7cb5a30e6dda73ebe761d9f0eb9ba038e14a7
Gerrit-Change-Number: 9933
Gerrit-PatchSet: 14
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Comment-Date: Fri, 08 Jun 2018 14:57:04 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2195: Improper handling of comments in queries

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9933 )

Change subject: IMPALA-2195: Improper handling of comments in queries
..


Patch Set 14: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7ac7cb5a30e6dda73ebe761d9f0eb9ba038e14a7
Gerrit-Change-Number: 9933
Gerrit-PatchSet: 14
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Comment-Date: Fri, 08 Jun 2018 14:55:21 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2195: Improper handling of comments in queries

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9933 )

Change subject: IMPALA-2195: Improper handling of comments in queries
..


Patch Set 13:

(8 comments)

I checked out your patch and ran this from the root Impala.git directory:

  git diff HEAD^ | impala-flake8 --diff

I marked flake8's output for you inline. I also have some ideas for test cases 
(see below).

http://gerrit.cloudera.org:8080/#/c/9933/13/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/9933/13/shell/impala_shell.py@325
PS13, Line 325:  command, args),
./shell/impala_shell.py:325:50: E128 continuation line under-indented for 
visual indent


http://gerrit.cloudera.org:8080/#/c/9933/13/shell/impala_shell.py@1344
PS13, Line 1344: if isinstance(token, sqlparse.sql.Comment): return True
./shell/impala_shell.py:1344:51: E701 multiple statements on one line (colon)


http://gerrit.cloudera.org:8080/#/c/9933/13/shell/impala_shell.py@1346
PS13, Line 1346:   if token.ttype == comment: return True
./shell/impala_shell.py:1346:36: E701 multiple statements on one line (colon)


http://gerrit.cloudera.org:8080/#/c/9933/13/tests/shell/test_shell_interactive.py
File tests/shell/test_shell_interactive.py:

http://gerrit.cloudera.org:8080/#/c/9933/13/tests/shell/test_shell_interactive.py@481
PS13, Line 481: 'select * from 
leading_comment;')
What about a test case like:

  /* comment1
 comment2 */ select * from leading_comment;


http://gerrit.cloudera.org:8080/#/c/9933/13/tests/shell/test_shell_interactive.py@482
PS13, Line 482:   assert 'Fetched 1 row(s)' in result.stderr
What about a test case like:

  /* select * from leading_comment */ select * from leading_comment;


http://gerrit.cloudera.org:8080/#/c/9933/13/tests/shell/test_shell_interactive.py@494
PS13, Line 494:   assert 'Fetched 1 row(s)' in result.stderr
Same as above except using help, not select.


http://gerrit.cloudera.org:8080/#/c/9933/13/tests/shell/test_shell_interactive.py@610
PS13, Line 610:  ImpalaShellClass.strip_leading_comment('/*delete*/\n'
./tests/shell/test_shell_interactive.py:610:10: E127 continuation line 
over-indented for visual indent


http://gerrit.cloudera.org:8080/#/c/9933/13/tests/shell/test_shell_interactive.py@619
PS13, Line 619:  ImpalaShellClass.strip_leading_comment('/*delete*/\n'
./tests/shell/test_shell_interactive.py:619:10: E127 continuation line 
over-indented for visual indent



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7ac7cb5a30e6dda73ebe761d9f0eb9ba038e14a7
Gerrit-Change-Number: 9933
Gerrit-PatchSet: 13
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Comment-Date: Thu, 07 Jun 2018 17:22:22 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-2751: Matching quotes are not required in comments

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10541 )

Change subject: IMPALA-2751: Matching quotes are not required in comments
..


Patch Set 5: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10541/4/tests/shell/test_shell_interactive.py
File tests/shell/test_shell_interactive.py:

http://gerrit.cloudera.org:8080/#/c/10541/4/tests/shell/test_shell_interactive.py@531
PS4, Line 531: ]
> It's the Python 2.6 and Python 2.7 different behaviors with regards to unic
Right, I gathered it was a Python 2.6 problem, but beyond that, I didn't know. 
Thanks for clarifying.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2feae34026a7e63f3d31489f757f093a73ca5d2c
Gerrit-Change-Number: 10541
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Thu, 07 Jun 2018 16:27:59 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-2751: Matching quotes are not required in comments

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10541 )

Change subject: IMPALA-2751: Matching quotes are not required in comments
..


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/10541/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10541/4//COMMIT_MSG@12
PS4, Line 12: The fix is to strip any SQL comments before sending to shlex 
since shlex
: does not understand SQL comments and will raise an exception when 
it
: sees
: unmatched quotes regardless whether the quotes are in the 
comments or
: not.
Misaligned text. Can you clean it up?


http://gerrit.cloudera.org:8080/#/c/10541/4/tests/shell/test_shell_interactive.py
File tests/shell/test_shell_interactive.py:

http://gerrit.cloudera.org:8080/#/c/10541/4/tests/shell/test_shell_interactive.py@531
PS4, Line 531: ]
I missed the reason for the revert. Are additional test cases needed?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2feae34026a7e63f3d31489f757f093a73ca5d2c
Gerrit-Change-Number: 10541
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Thu, 07 Jun 2018 16:14:37 +
Gerrit-HasComments: Yes

[Impala-ASF-CR](2.x) IMPALA-6947: Kudu tests flaky due to rpc timeout

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10603 )

Change subject: IMPALA-6947: Kudu tests flaky due to rpc timeout
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c4a2d87934cd4eb98509512f7060a596894ed53
Gerrit-Change-Number: 10603
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Tue, 05 Jun 2018 15:15:08 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6947: Kudu tests flaky due to rpc timeout

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10466 )

Change subject: IMPALA-6947: Kudu tests flaky due to rpc timeout
..


Patch Set 4: Code-Review+1

Looks fine from my end. The python looks a lot better. Thanks!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c4a2d87934cd4eb98509512f7060a596894ed53
Gerrit-Change-Number: 10466
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Marshall 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Mon, 04 Jun 2018 17:51:17 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2195: Improper handling of comments in queries

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9933 )

Change subject: IMPALA-2195: Improper handling of comments in queries
..


Patch Set 12:

> No takers for +2? This has been lingering for a while, good to get this in.

We're trying to focus on broken build fixes at the moment.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7ac7cb5a30e6dda73ebe761d9f0eb9ba038e14a7
Gerrit-Change-Number: 9933
Gerrit-PatchSet: 12
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Comment-Date: Fri, 01 Jun 2018 15:12:12 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7104: test bloom wait time failing with timeout on asan

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10565 )

Change subject: IMPALA-7104: test_bloom_wait_time failing with timeout on asan
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0271eb83e65b3b5f17e32f6501032f0d12dde38e
Gerrit-Change-Number: 10565
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Marshall 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Thu, 31 May 2018 22:30:28 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6990: TestClientSsl.test tls v12 failing due to Python SSL error

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10529 )

Change subject: IMPALA-6990: TestClientSsl.test_tls_v12 failing due to Python 
SSL error
..


Patch Set 3: Code-Review+1

...pending Phil's comments, which are fair. The Python version comparison looks 
like the accepted idiom (Impyla does it, for example)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I92c66ecaeb94b0c83ee6f1396c082709c21b3187
Gerrit-Change-Number: 10529
Gerrit-PatchSet: 3
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Wed, 30 May 2018 23:58:50 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6947: Kudu tests flaky due to rpc timeout

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10466 )

Change subject: IMPALA-6947: Kudu tests flaky due to rpc timeout
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/10466/1//COMMIT_MSG@18
PS1, Line 18: Testing:
: - Passed a full run of core tests.
Also, since this affects ASAN, I suggest trying this against an ASAN build.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c4a2d87934cd4eb98509512f7060a596894ed53
Gerrit-Change-Number: 10466
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Marshall 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Tue, 29 May 2018 20:33:58 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6947: Kudu tests flaky due to rpc timeout

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10466 )

Change subject: IMPALA-6947: Kudu tests flaky due to rpc timeout
..


Patch Set 1:

(2 comments)

Please add another appropriate reviewer for the changes to the .cc files. It 
looks fine to me but someone more familiar should look as well.

http://gerrit.cloudera.org:8080/#/c/10466/1/be/src/common/global-flags.cc
File be/src/common/global-flags.cc:

http://gerrit.cloudera.org:8080/#/c/10466/1/be/src/common/global-flags.cc@162
PS1, Line 162: DEFINE_int32(kudu_rpc_timeout_ms, 0, "Timeout (milliseconds) set 
for Kudu rpcs. This "
 : "must be a positive value or it will be ignored and Kudu's 
default of 10s will be "
 : "used. There is no way to disable timeouts.");
 :
Is this an option that ought to be hidden away from users/customers? If yes, 
please make sure that in defining this it's properly hidden. Also, it's not 
clear to me on how this relates to kudu_operation_timeout_ms. Can you clean up 
the help text?


http://gerrit.cloudera.org:8080/#/c/10466/1/bin/run-all-tests.sh
File bin/run-all-tests.sh:

http://gerrit.cloudera.org:8080/#/c/10466/1/bin/run-all-tests.sh@84
PS1, Line 84: TEST_START_CLUSTER_ARGS="${TEST_START_CLUSTER_ARGS} 
--impalad_args=--kudu_rpc_timeout_ms=6"
Since this only happens with Kudu on ASAN, I think we only want to enable this 
if we are under those conditions.  A better place to do this is in 
start_impalad_instances() in bin/start-impala-cluster.py. You can use 
tests.common.environ specific_build_type_timeout() to tell you whether you're 
in ASAN or not and set a timeout appropriately. Do a git grep for 
specific_build_type_timeout and you'll see plenty of examples.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c4a2d87934cd4eb98509512f7060a596894ed53
Gerrit-Change-Number: 10466
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Marshall 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Tue, 29 May 2018 20:18:03 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6714: [DOCS] ORC file format support

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10525 )

Change subject: IMPALA-6714: [DOCS] ORC file format support
..


Patch Set 1:

Alex Rodoni is currently most familiar with docs so making them a reviewer. 
Thanks.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib1ee23ed844653c274babdce5a332dbe5c79b630
Gerrit-Change-Number: 10525
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Tue, 29 May 2018 17:05:28 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6933: Avoids db name collisions for Kudu tests

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10513 )

Change subject: IMPALA-6933: Avoids db name collisions for Kudu tests
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7c2f8a35fec90ae0dabe80237d83954668b47f6e
Gerrit-Change-Number: 10513
Gerrit-PatchSet: 2
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 29 May 2018 16:30:32 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7071: make get fs path() idempotent

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10517 )

Change subject: IMPALA-7071: make get_fs_path() idempotent
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I10c7cbb36119883718e60660db2ac7313f14d388
Gerrit-Change-Number: 10517
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Fri, 25 May 2018 19:01:38 +
Gerrit-HasComments: No