[Impala-ASF-CR] IMPALA-9384: Improve Impala shell usability by enabling live progress in interactive mode

2020-03-05 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15219 )

Change subject: IMPALA-9384: Improve Impala shell usability by enabling 
live_progress in interactive mode
..


Patch Set 8:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/5438/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3765b775f663fa227e59728acffe4d5ea9a5e2d3
Gerrit-Change-Number: 15219
Gerrit-PatchSet: 8
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 06 Mar 2020 07:57:57 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9466: impala-shell client retry for hs2-http protocol

2020-03-05 Thread Abhishek Rawat (Code Review)
Abhishek Rawat has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/15378


Change subject: IMPALA-9466: impala-shell client retry for hs2-http protocol
..

IMPALA-9466: impala-shell client retry for hs2-http protocol

Added retries for idempotent rpcs:
OpenSession, PingImpalaHS2Service, GetResultSetMetadata,
CloseImpalaOperation (non dmls), CancelOperation, GetOperationStatus,
GetRuntimeProfile, GetExecSummary, GetLog

Retries were also added to the 'set all' query execution and subsequent
result fetch in the ImpalaHS2Client._open_session()

The retries are only supported for hs2-http protocol and enabled by
default. At most there are 3 tries for a failed rpc with atleast 2
second wait duration between tries.

Only failed rpcs due to an error in the http transport are retried and
if an rpc failed because the server returned an error in the rpc
response then such scenarios are not retriable.

Improved error diagnostics by dumping stack trace when ImpalaShell.
_execute_stmt() gets an 'Unknown Exception'.

Testing:
-Added a custom_cluster test which injects fault into the http transport
and checks expected behavior from the various rpcs. Some of these tests
leave the session in an open state and so these tests are not suitable
for the e2e test framework which have metric verifiers expecting related
metrics to be 0 at the end of the test.
- Manually tested real world scenarios with impala-shell client
communicating with an impala coordinator via a fault injecting istio mesh.

Change-Id: I0da9e9e8d34a340eaf763397cc095ff6260d65d5
---
M shell/impala_client.py
M shell/impala_shell.py
A tests/custom_cluster/test_hs2_fault_injection.py
3 files changed, 389 insertions(+), 48 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0da9e9e8d34a340eaf763397cc095ff6260d65d5
Gerrit-Change-Number: 15378
Gerrit-PatchSet: 3
Gerrit-Owner: Abhishek Rawat 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-9466: impala-shell client retry for hs2-http protocol

2020-03-05 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15378 )

Change subject: IMPALA-9466: impala-shell client retry for hs2-http protocol
..


Patch Set 3:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/15378/3/shell/impala_client.py
File shell/impala_client.py:

http://gerrit.cloudera.org:8080/#/c/15378/3/shell/impala_client.py@657
PS3, Line 657: r
flake8: E731 do not assign a lambda expression, use a def


http://gerrit.cloudera.org:8080/#/c/15378/3/shell/impala_client.py@685
PS3, Line 685: ;
flake8: E703 statement ends with a semicolon


http://gerrit.cloudera.org:8080/#/c/15378/3/shell/impala_client.py@715
PS3, Line 715: r
flake8: E731 do not assign a lambda expression, use a def


http://gerrit.cloudera.org:8080/#/c/15378/3/shell/impala_client.py@733
PS3, Line 733: r
flake8: E731 do not assign a lambda expression, use a def


http://gerrit.cloudera.org:8080/#/c/15378/3/shell/impala_client.py@744
PS3, Line 744: r
flake8: E731 do not assign a lambda expression, use a def


http://gerrit.cloudera.org:8080/#/c/15378/3/shell/impala_client.py@812
PS3, Line 812: r
flake8: E731 do not assign a lambda expression, use a def


http://gerrit.cloudera.org:8080/#/c/15378/3/shell/impala_client.py@829
PS3, Line 829: r
flake8: E731 do not assign a lambda expression, use a def


http://gerrit.cloudera.org:8080/#/c/15378/3/shell/impala_client.py@849
PS3, Line 849: r
flake8: E731 do not assign a lambda expression, use a def


http://gerrit.cloudera.org:8080/#/c/15378/3/shell/impala_client.py@858
PS3, Line 858: r
flake8: E731 do not assign a lambda expression, use a def


http://gerrit.cloudera.org:8080/#/c/15378/3/tests/custom_cluster/test_hs2_fault_injection.py
File tests/custom_cluster/test_hs2_fault_injection.py:

http://gerrit.cloudera.org:8080/#/c/15378/3/tests/custom_cluster/test_hs2_fault_injection.py@65
PS3, Line 65: /
flake8: E226 missing whitespace around arithmetic operator


http://gerrit.cloudera.org:8080/#/c/15378/3/tests/custom_cluster/test_hs2_fault_injection.py@223
PS3, Line 223:
flake8: E211 whitespace before '('



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0da9e9e8d34a340eaf763397cc095ff6260d65d5
Gerrit-Change-Number: 15378
Gerrit-PatchSet: 3
Gerrit-Owner: Abhishek Rawat 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Fri, 06 Mar 2020 07:41:18 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-9384: Improve Impala shell usability by enabling live progress in interactive mode

2020-03-05 Thread Alice Fan (Code Review)
Hello Andrew Sherman, David Knupp, Tim Armstrong, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-9384: Improve Impala shell usability by enabling 
live_progress in interactive mode
..

IMPALA-9384: Improve Impala shell usability by enabling live_progress in 
interactive mode

In order to improve usability, this patch makes Impala shell show query
processing status while the query is running. The patch enables shell
option live_progress by default when a user launches impala shell in
the interactive mode. The patch also adds a new command line flag
"--disable_live_progress", which allows a user to disable live_progress
at runtime. In the interactive mode, a user can disable live_progress
by either using the command line flag or setting the option as False in
the config file. As for in the non-interactive mode (when the -q or -f
options are used), live reporting is not supported. Impala-shell will
disable live_progress if the mode is detected.

Testing:
- Added and updated tests in test_shell_interactive.py and 
test_shell_commandline.py
- Successfully ran all shell related tests

Change-Id: I3765b775f663fa227e59728acffe4d5ea9a5e2d3
---
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
5 files changed, 47 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/19/15219/8
--
To view, visit http://gerrit.cloudera.org:8080/15219
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3765b775f663fa227e59728acffe4d5ea9a5e2d3
Gerrit-Change-Number: 15219
Gerrit-PatchSet: 8
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-9357: Fix race condition in alter database event

2020-03-05 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15260 )

Change subject: IMPALA-9357: Fix race condition in alter_database event
..


Patch Set 7:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/5437/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I472fd8a55740769ee5cdb84e48422a4ab39a8d1e
Gerrit-Change-Number: 15260
Gerrit-PatchSet: 7
Gerrit-Owner: Vihang Karajgaonkar 
Gerrit-Reviewer: Anurag Mantripragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Comment-Date: Fri, 06 Mar 2020 02:10:24 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-4080 [part 3]: Moved codegen code for TupleRowComparator into a Config object

2020-03-05 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/15281 )

Change subject: IMPALA-4080 [part 3]: Moved codegen code for TupleRowComparator 
into a Config object
..

IMPALA-4080 [part 3]: Moved codegen code for TupleRowComparator into a
Config object

This introduces a new class called TupleRowComparatorConfig that houses
the fragment level parameters needed to create a TupleRowComparator
object. It also codegens the TupleRowComparator::Compare() method that
is shared by all instances of TupleRowComparator objects created using
this config. Also, moved the related code to initiate codegen on this
class for the exchange, sort, partial sort and top-n nodes from their
exec nodes to their plan nodes.

Lastly, this also moved the code responsible for codegening
TopNNode::InsertBatch() from its exec node to its plan node.

Testing:
TODO: Sucessfully run exhaustive tests. Need to re-run after resolving
  rebase conflicts.
Manually verified that codegen works for all modified exec nodes.

Change-Id: I265604ecbae2c80ff59e697fddfeea4976b25cd9
Reviewed-on: http://gerrit.cloudera.org:8080/15281
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M be/src/exec/exchange-node.cc
M be/src/exec/exchange-node.h
M be/src/exec/partial-sort-node.cc
M be/src/exec/partial-sort-node.h
M be/src/exec/sort-node.cc
M be/src/exec/sort-node.h
M be/src/exec/topn-node.cc
M be/src/exec/topn-node.h
M be/src/runtime/data-stream-test.cc
M be/src/runtime/sorter.cc
M be/src/runtime/sorter.h
M be/src/util/tuple-row-compare-test.cc
M be/src/util/tuple-row-compare.cc
M be/src/util/tuple-row-compare.h
14 files changed, 232 insertions(+), 203 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I265604ecbae2c80ff59e697fddfeea4976b25cd9
Gerrit-Change-Number: 15281
Gerrit-PatchSet: 8
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Norbert Luksa 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-4080 [part 3]: Moved codegen code for TupleRowComparator into a Config object

2020-03-05 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15281 )

Change subject: IMPALA-4080 [part 3]: Moved codegen code for TupleRowComparator 
into a Config object
..


Patch Set 7: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I265604ecbae2c80ff59e697fddfeea4976b25cd9
Gerrit-Change-Number: 15281
Gerrit-PatchSet: 7
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Norbert Luksa 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Fri, 06 Mar 2020 01:55:33 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9357: Fix race condition in alter database event

2020-03-05 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15260 )

Change subject: IMPALA-9357: Fix race condition in alter_database event
..


Patch Set 6:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/5436/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I472fd8a55740769ee5cdb84e48422a4ab39a8d1e
Gerrit-Change-Number: 15260
Gerrit-PatchSet: 6
Gerrit-Owner: Vihang Karajgaonkar 
Gerrit-Reviewer: Anurag Mantripragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Comment-Date: Fri, 06 Mar 2020 01:51:03 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9384: Improve Impala shell usability by enabling live progress in interactive mode

2020-03-05 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15219 )

Change subject: IMPALA-9384: Improve Impala shell usability by enabling 
live_progress in interactive mode
..


Patch Set 7: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3765b775f663fa227e59728acffe4d5ea9a5e2d3
Gerrit-Change-Number: 15219
Gerrit-PatchSet: 7
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 06 Mar 2020 01:40:47 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9357: Fix race condition in alter database event

2020-03-05 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15260 )

Change subject: IMPALA-9357: Fix race condition in alter_database event
..


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15260/7/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:

http://gerrit.cloudera.org:8080/#/c/15260/7/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@402
PS7, Line 402:* successfully acquires both within LOCK_RETRY_TIMEOUT_MS 
millisecs; both locks are held
line too long (91 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I472fd8a55740769ee5cdb84e48422a4ab39a8d1e
Gerrit-Change-Number: 15260
Gerrit-PatchSet: 7
Gerrit-Owner: Vihang Karajgaonkar 
Gerrit-Reviewer: Anurag Mantripragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Comment-Date: Fri, 06 Mar 2020 01:24:42 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-9357: Fix race condition in alter database event

2020-03-05 Thread Vihang Karajgaonkar (Code Review)
Vihang Karajgaonkar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15260 )

Change subject: IMPALA-9357: Fix race condition in alter_database event
..


Patch Set 7:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/15260/6/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:

http://gerrit.cloudera.org:8080/#/c/15260/6/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@436
PS6, Line 436:* method.
> optional: I think this is is not too complex. Maybe we can do it now. We ca
I think this approach definitely helps with reducing the duplicate code, but 
the interface definition is still not very clean. Ideally, a Lockable interface 
should have methods like lock(), unlock() and tryLock(). This makes sure that 
lock object itself is a private member of Table and Db and I think thats a 
better approach. But inorder to do this all the occurances of 
table.getLock().lock() will need to change and a number of unrelated files will 
be added to this change. I would like to separate this to a separate patch. 
Created IMPALA-9468 for this.


http://gerrit.cloudera.org:8080/#/c/15260/6/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@457
PS6, Line 457: uptedException e)
> nit: It'd be better to rename this to something not related to table, like
renamed to LOCK_RETRY_DELAY_MS


http://gerrit.cloudera.org:8080/#/c/15260/6/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@462
PS6, Line 462:
> nit: same here, maybe RETRY_LOCK_TIMEOUT_MS.
changed to LOCK_RETRY_TIMEOUT_MS



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I472fd8a55740769ee5cdb84e48422a4ab39a8d1e
Gerrit-Change-Number: 15260
Gerrit-PatchSet: 7
Gerrit-Owner: Vihang Karajgaonkar 
Gerrit-Reviewer: Anurag Mantripragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Comment-Date: Fri, 06 Mar 2020 01:24:06 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-9357: Fix race condition in alter database event

2020-03-05 Thread Vihang Karajgaonkar (Code Review)
Vihang Karajgaonkar has uploaded a new patch set (#7). ( 
http://gerrit.cloudera.org:8080/15260 )

Change subject: IMPALA-9357: Fix race condition in alter_database event
..

IMPALA-9357: Fix race condition in alter_database event

The race condition is exposed intermittently, on certain builds which
causes test_event_processing::test_self_events test to fail. This
happens because we are checking for self-event identifiers in the Db
object without taking a lock. When a DDL like 'comment on
database is 'test'' is executed, it is possible that the event
processor thread is triggered as soon as the ALTER_DATABASE event is
generated. This may cause event processor fail the self-event detection
since the self-event identifiers are not yet added to the Db object.

The fix adds a Db lock similar to Table lock. Alter db operations
in CatalogOpExecutor now take db locks instead of metastoreDdlLock_
which makes it consistent with table locking protocol.

Testing:
1. Ran existing tests for events processor.
2. This test was failing on centos6 frequently (failed in 1/3 times).
After the fix I ran the test in a loop for 24 hrs (197 iterations) and
the test didn't fail.
3. Ran core tests with CDP and CDH builds.

Change-Id: I472fd8a55740769ee5cdb84e48422a4ab39a8d1e
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/Db.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M tests/custom_cluster/test_event_processing.py
5 files changed, 175 insertions(+), 112 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/60/15260/7
--
To view, visit http://gerrit.cloudera.org:8080/15260
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I472fd8a55740769ee5cdb84e48422a4ab39a8d1e
Gerrit-Change-Number: 15260
Gerrit-PatchSet: 7
Gerrit-Owner: Vihang Karajgaonkar 
Gerrit-Reviewer: Anurag Mantripragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Vihang Karajgaonkar 


[Impala-ASF-CR] IMPALA-9357: Fix race condition in alter database event

2020-03-05 Thread Vihang Karajgaonkar (Code Review)
Vihang Karajgaonkar has uploaded a new patch set (#6). ( 
http://gerrit.cloudera.org:8080/15260 )

Change subject: IMPALA-9357: Fix race condition in alter_database event
..

IMPALA-9357: Fix race condition in alter_database event

The race condition is exposed intermittently, on certain builds which
causes test_event_processing::test_self_events test to fail. This
happens because we are checking for self-event identifiers in the Db
object without taking a lock. When a DDL like 'comment on
database is 'test'' is executed, it is possible that the event
processor thread is triggered as soon as the ALTER_DATABASE event is
generated. This may cause event processor fail the self-event detection
since the self-event identifiers are not yet added to the Db object.

The fix adds a Db lock similar to Table lock. Alter db operations
in CatalogOpExecutor now take db locks instead of metastoreDdlLock_
which makes it consistent with table locking protocol.

Testing:
1. Ran existing tests for events processor.
2. This test was failing on centos6 frequently (failed in 1/3 times).
After the fix I ran the test in a loop for 24 hrs (197 iterations) and
the test didn't fail.
3. Ran core tests with CDP and CDH builds.

Change-Id: I472fd8a55740769ee5cdb84e48422a4ab39a8d1e
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/Db.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M tests/custom_cluster/test_event_processing.py
5 files changed, 170 insertions(+), 106 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/60/15260/6
--
To view, visit http://gerrit.cloudera.org:8080/15260
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I472fd8a55740769ee5cdb84e48422a4ab39a8d1e
Gerrit-Change-Number: 15260
Gerrit-PatchSet: 6
Gerrit-Owner: Vihang Karajgaonkar 
Gerrit-Reviewer: Anurag Mantripragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Vihang Karajgaonkar 


[Impala-ASF-CR] IMPALA-8674: fix bug where REMOTE runtime filter always marked disabled

2020-03-05 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15308 )

Change subject: IMPALA-8674: fix bug where REMOTE runtime filter always marked 
disabled
..


Patch Set 4:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/5435/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82a5a776103abd0a6d73336bebc65e22b4e13fef
Gerrit-Change-Number: 15308
Gerrit-PatchSet: 4
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 06 Mar 2020 00:57:09 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8674: fix bug where REMOTE runtime filter always marked disabled

2020-03-05 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15308 )

Change subject: IMPALA-8674: fix bug where REMOTE runtime filter always marked 
disabled
..


Patch Set 4:

(3 comments)

This is looking good. I have a bit of nit-picky feedback, nothing serious. This 
is mainly about fitting with the codebase's existing style and making it 
slightly easier to maintain in future.

http://gerrit.cloudera.org:8080/#/c/15308/4/be/src/runtime/coordinator-filter-state.h
File be/src/runtime/coordinator-filter-state.h:

http://gerrit.cloudera.org:8080/#/c/15308/4/be/src/runtime/coordinator-filter-state.h@100
PS4, Line 100:   void set_all_updates_received() { all_updates_received_ = 
true; }
I'd consider making this an argument to Disable() and DisableAndRelease(), 
because at each callsite a programmer already needs to think about whether they 
should also be calling set_all_updates_received(). If it's a non-optional 
argument this forces them to think about it.


http://gerrit.cloudera.org:8080/#/c/15308/4/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:

http://gerrit.cloudera.org:8080/#/c/15308/4/be/src/runtime/coordinator.cc@605
PS4, Line 605: if (state.disabled()) {
Maybe simplify to:

  row.push_back(state.received_all_updates() || !state.disabled());

This is optional, I just have a strong bias towards minimising the number of 
control flow blocks, I find it easier to follow the logic of the function.


http://gerrit.cloudera.org:8080/#/c/15308/4/be/src/runtime/coordinator.cc@1366
PS4, Line 1366: if (!disabled()) {
nit: the coding style is usually to put conditions on a single line if they fit

  if (!disabled()) set_all_updates_received();

Also, this is totally optional, but it might be slightly more readable if you 
avoided the double negative in these conditions by defining an enabled() 
accessor method that just returned !disabled().



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82a5a776103abd0a6d73336bebc65e22b4e13fef
Gerrit-Change-Number: 15308
Gerrit-PatchSet: 4
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 06 Mar 2020 00:36:32 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5904: (part 3) Fix more TSAN bugs

2020-03-05 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15363 )

Change subject: IMPALA-5904: (part 3) Fix more TSAN bugs
..


Patch Set 1: Code-Review+2

(4 comments)

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

http://gerrit.cloudera.org:8080/#/c/15363/1//COMMIT_MSG@16
PS1, Line 16: * Data race in be/src/exprs/scalar-expr-evaluator.cc:285:26
I'm confused how this worked in the first place - it seems like it'd break the 
sort order. Good to fix it.


http://gerrit.cloudera.org:8080/#/c/15363/1/be/src/rpc/hs2-http-test.cc
File be/src/rpc/hs2-http-test.cc:

http://gerrit.cloudera.org:8080/#/c/15363/1/be/src/rpc/hs2-http-test.cc@37
PS1, Line 37: using namespace impala;
I think you can remove this now.


http://gerrit.cloudera.org:8080/#/c/15363/1/be/src/runtime/data-stream-test.cc
File be/src/runtime/data-stream-test.cc:

http://gerrit.cloudera.org:8080/#/c/15363/1/be/src/runtime/data-stream-test.cc@349
PS1, Line 349:   void CreateTupleComparator(TupleRowComparator*& 
less_than_comparator) {
Pass by ** to keep consistent with coding conventions.


http://gerrit.cloudera.org:8080/#/c/15363/1/be/src/runtime/descriptors.cc
File be/src/runtime/descriptors.cc:

http://gerrit.cloudera.org:8080/#/c/15363/1/be/src/runtime/descriptors.cc@617
PS1, Line 617: i++
nit: ++i



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b7b119e256085d1ba6977e1161fc658273b242
Gerrit-Change-Number: 15363
Gerrit-PatchSet: 1
Gerrit-Owner: Sahil Takiar 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 06 Mar 2020 00:21:46 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8674: fix bug where REMOTE runtime filter always marked disabled

2020-03-05 Thread Riza Suminto (Code Review)
Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15308 )

Change subject: IMPALA-8674: fix bug where REMOTE runtime filter always marked 
disabled
..


Patch Set 4:

(7 comments)

Thanks Fang-Yu, the following changes address your comments.

http://gerrit.cloudera.org:8080/#/c/15308/3/be/src/runtime/coordinator-filter-state.h
File be/src/runtime/coordinator-filter-state.h:

http://gerrit.cloudera.org:8080/#/c/15308/3/be/src/runtime/coordinator-filter-state.h@99
PS3, Line 99:   bool received_all_updates() const { return 
all_updates_received_; }
:   void set_all_updates_received() { all_updates_received_ = true;
> nit: Is it better to change the names of the functions to received_all_upda
Done


http://gerrit.cloudera.org:8080/#/c/15308/3/be/src/runtime/coordinator-filter-state.h@162
PS3, Line 162: coordi
> nit: Is it better to change it from 'filter' to 'the coordinator'?
Done


http://gerrit.cloudera.org:8080/#/c/15308/3/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:

http://gerrit.cloudera.org:8080/#/c/15308/3/be/src/runtime/coordinator.cc@1266
PS3, Line 1266:
> nit: The filter may have already been disabled.
Removed for shorter comment.


http://gerrit.cloudera.org:8080/#/c/15308/3/be/src/runtime/coordinator.cc@1267
PS3, Line 1267: >Disable()
> nit: the coordinator receives an.
Removed for shorter comment.


http://gerrit.cloudera.org:8080/#/c/15308/3/be/src/runtime/coordinator.cc@1268
PS3, Line 1268:
> nit: updates have.
Done, moved to ApplyUpdate().


http://gerrit.cloudera.org:8080/#/c/15308/3/be/src/runtime/coordinator.cc@1269
PS3, Line 1269: arams.mutable_dst_qu
> nit: Change it to 'all_updates_received_' if you decide to change the name 
Done, also moved to ApplyUpdate().


http://gerrit.cloudera.org:8080/#/c/15308/3/be/src/runtime/coordinator.cc@1270
PS3, Line 1270: rpc_params.set_filter_id(params.filter_id());
> Taking a closer look at Coordinator::FilterState::ApplyUpdate(), I was wond
Make sense. So all flag flipping happen within ApplyUpdate().
Moved this along the comment under ApplyUpdate().



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82a5a776103abd0a6d73336bebc65e22b4e13fef
Gerrit-Change-Number: 15308
Gerrit-PatchSet: 4
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 06 Mar 2020 00:21:24 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-9414 (part 1): Copy THttpClient from Thrift into Impala

2020-03-05 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15283 )

Change subject: IMPALA-9414 (part 1): Copy THttpClient from Thrift into Impala
..


Patch Set 5: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2662f1d4d455120442ef7c0c198685c07207aeed
Gerrit-Change-Number: 15283
Gerrit-PatchSet: 5
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 06 Mar 2020 00:17:30 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8674: fix bug where REMOTE runtime filter always marked disabled

2020-03-05 Thread Riza Suminto (Code Review)
Hello Fang-Yu Rao, Tim Armstrong, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-8674: fix bug where REMOTE runtime filter always marked 
disabled
..

IMPALA-8674: fix bug where REMOTE runtime filter always marked disabled

When a runtime filter has remote target, coordinator will Disable the
FilterState upon arrival of the last filter update to prevent another
update towards that filter. As consequence, such runtime filter will
always be displayed as disabled in runtime profile (Enabled column is
equal to false in Final filter table), when in reality the runtime
filter has heard back from all pending backends and complete. The
Enabled column should correctly distinguish between failed runtime
filter vs complete runtime filter. To do so, we add
all_updates_received_ flag in FilterState class and set it to true
after filter received enough filter update from pending backends to
proceed. If all_updates_received_ is true, then that runtime filter is
considered as enabled.

Testing:
- Add row regex in runtime_filters.test, query 6, to verify REMOTE
  runtime filter is marked as enabled in final filter table
- Run and pass test_runtime_filters.py
- Run and pass core tests

Change-Id: I82a5a776103abd0a6d73336bebc65e22b4e13fef
---
M be/src/runtime/coordinator-filter-state.h
M be/src/runtime/coordinator.cc
M testdata/workloads/functional-query/queries/QueryTest/runtime_filters.test
3 files changed, 25 insertions(+), 1 deletion(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I82a5a776103abd0a6d73336bebc65e22b4e13fef
Gerrit-Change-Number: 15308
Gerrit-PatchSet: 4
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8674: fix bug where REMOTE runtime filter always marked disabled

2020-03-05 Thread Fang-Yu Rao (Code Review)
Fang-Yu Rao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15308 )

Change subject: IMPALA-8674: fix bug where REMOTE runtime filter always marked 
disabled
..


Patch Set 3:

(7 comments)

Hi Riza, thanks for addressing my previous comments! The patch looks good to 
me. I only left some very minor comments.

http://gerrit.cloudera.org:8080/#/c/15308/3/be/src/runtime/coordinator-filter-state.h
File be/src/runtime/coordinator-filter-state.h:

http://gerrit.cloudera.org:8080/#/c/15308/3/be/src/runtime/coordinator-filter-state.h@99
PS3, Line 99:   bool received_all_update() const { return all_update_received_; 
}
:   void set_all_update_received() { all_update_received_ = true; }
nit: Is it better to change the names of the functions to 
received_all_updates() and set_all_updates_received(), respectively?
Also is it better to change 'all_update_received_' to 'all_updates_received_'?


http://gerrit.cloudera.org:8080/#/c/15308/3/be/src/runtime/coordinator-filter-state.h@162
PS3, Line 162: filter
nit: Is it better to change it from 'filter' to 'the coordinator'?


http://gerrit.cloudera.org:8080/#/c/15308/3/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:

http://gerrit.cloudera.org:8080/#/c/15308/3/be/src/runtime/coordinator.cc@1266
PS3, Line 1266: Filter may already disabled
nit: The filter may have already been disabled.


http://gerrit.cloudera.org:8080/#/c/15308/3/be/src/runtime/coordinator.cc@1267
PS3, Line 1267: it receive
nit: the coordinator receives an.


http://gerrit.cloudera.org:8080/#/c/15308/3/be/src/runtime/coordinator.cc@1268
PS3, Line 1268: update has
nit: updates have.


http://gerrit.cloudera.org:8080/#/c/15308/3/be/src/runtime/coordinator.cc@1269
PS3, Line 1269: all_update_received_
nit: Change it to 'all_updates_received_' if you decide to change the name of 
this variable.


http://gerrit.cloudera.org:8080/#/c/15308/3/be/src/runtime/coordinator.cc@1270
PS3, Line 1270: if (!state->disabled()) state->set_all_update_received();
Taking a closer look at Coordinator::FilterState::ApplyUpdate(), I was 
wondering if it would be more intuitive to call set_all_update_received() at 
the end of ApplyUpdate().

Specifically, after 
https://github.com/apache/impala/blob/master/be/src/runtime/coordinator.cc#L1352-L1354,
 we additionally check whether or not it is true that 'pending_count_' is equal 
to 0 and '!disabled()' evaluates to true. If so, we call 
set_all_update_received().

Let me know if I have missed something. I am fine with both approaches.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82a5a776103abd0a6d73336bebc65e22b4e13fef
Gerrit-Change-Number: 15308
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 05 Mar 2020 22:27:25 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-9414 (part 2): Support the 'Expect: 100-continue' http header

2020-03-05 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15284 )

Change subject: IMPALA-9414 (part 2): Support the 'Expect: 100-continue' http 
header
..


Patch Set 6: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4153968551acd58b25c7923c2ebf75ee29a7e76b
Gerrit-Change-Number: 15284
Gerrit-PatchSet: 6
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 05 Mar 2020 21:59:33 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9357: Fix race condition in alter database event

2020-03-05 Thread Vihang Karajgaonkar (Code Review)
Vihang Karajgaonkar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15260 )

Change subject: IMPALA-9357: Fix race condition in alter_database event
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15260/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:

http://gerrit.cloudera.org:8080/#/c/15260/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@449
PS5, Line 449: table
> Did you mean DB?
Thanks for spotting this. Fixed.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I472fd8a55740769ee5cdb84e48422a4ab39a8d1e
Gerrit-Change-Number: 15260
Gerrit-PatchSet: 5
Gerrit-Owner: Vihang Karajgaonkar 
Gerrit-Reviewer: Anurag Mantripragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Comment-Date: Thu, 05 Mar 2020 21:49:48 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-9429: Unioned partition columns break partition pruning

2020-03-05 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15371 )

Change subject: IMPALA-9429: Unioned partition columns break partition pruning
..


Patch Set 1:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/5434/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1c1384c2cd1ad5f7024449196f9a348ecdccb60b
Gerrit-Change-Number: 15371
Gerrit-PatchSet: 1
Gerrit-Owner: Kurt Deschler 
Gerrit-Reviewer: Anurag Mantripragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Kurt Deschler 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Comment-Date: Thu, 05 Mar 2020 21:46:28 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-4080 [part 3]: Moved codegen code for TupleRowComparator into a Config object

2020-03-05 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15281 )

Change subject: IMPALA-4080 [part 3]: Moved codegen code for TupleRowComparator 
into a Config object
..


Patch Set 6:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/5433/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I265604ecbae2c80ff59e697fddfeea4976b25cd9
Gerrit-Change-Number: 15281
Gerrit-PatchSet: 6
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Norbert Luksa 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 05 Mar 2020 21:44:17 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9414 (part 2): Support the 'Expect: 100-continue' http header

2020-03-05 Thread David Knupp (Code Review)
David Knupp has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15284 )

Change subject: IMPALA-9414 (part 2): Support the 'Expect: 100-continue' http 
header
..


Patch Set 6:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/15284/6/shell/THttpClient.py
File shell/THttpClient.py:

http://gerrit.cloudera.org:8080/#/c/15284/6/shell/THttpClient.py@35
PS6, Line 35: class THttpClient(TTransportBase):
I guess I'm not convinced that subclassing is better than straight copying, so 
this is fine.

What do you think about the idea of renaming this to something like 
ImpalaThriftHttpClient, and adding a docstring or comment block at the top 
explaining the derivation and differences of this code. We have various places 
in our code (thirft_sasl.py and prettytable) where we copy a file or module 
from some other source repo, and it takes some digging to find out whether it's 
a straight copy or not, or what version is was copied from, and why. E.g., I 
don't know why we chose to copy thrift_sasl.py. And prettytable that shows up 
in our impala-python env as v0.7.2, even though it's not really the same as the 
upstream v0.7.2.

Feel free to push back.


http://gerrit.cloudera.org:8080/#/c/15284/1/shell/util.py
File shell/util.py:

http://gerrit.cloudera.org:8080/#/c/15284/1/shell/util.py@1
PS1, Line 1:
> Ah okay. Seems like I could get it to work my adding a subdirectory, say sh
I think your solution is fine. The main thing is just to put them all into one 
easy to find location. Having them all in impala_client.py always seemed kinda 
weird.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4153968551acd58b25c7923c2ebf75ee29a7e76b
Gerrit-Change-Number: 15284
Gerrit-PatchSet: 6
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 05 Mar 2020 21:24:03 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-9429: Unioned partition columns break partition pruning

2020-03-05 Thread Kurt Deschler (Code Review)
Kurt Deschler has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/15371


Change subject: IMPALA-9429: Unioned partition columns break partition pruning
..

IMPALA-9429: Unioned partition columns break partition pruning

In the case of a union query where predicates are pushed into the union,
predicate Exprs can contain SlotReft that are transformed into constants
after analysis and eligible for constant folding. During partition
pruning there is a check that eligible constant folding has already
occurred which was failing and reporting IllegalStateException since the
surrounding code only handles specific cases.

This fix adds constant a folding call after union substitutions occur
from SingleNodePlanner.createUnionPlan

Testing:
Testcases added to PlannerTest/union.test based on provided repo using
alltypes tables.

Change-Id: I1c1384c2cd1ad5f7024449196f9a348ecdccb60b
---
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M testdata/workloads/functional-planner/queries/PlannerTest/union.test
2 files changed, 97 insertions(+), 0 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1c1384c2cd1ad5f7024449196f9a348ecdccb60b
Gerrit-Change-Number: 15371
Gerrit-PatchSet: 1
Gerrit-Owner: Kurt Deschler 


[Impala-ASF-CR] IMPALA-4080 [part 3]: Moved codegen code for TupleRowComparator into a Config object

2020-03-05 Thread Bikramjeet Vig (Code Review)
Bikramjeet Vig has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15281 )

Change subject: IMPALA-4080 [part 3]: Moved codegen code for TupleRowComparator 
into a Config object
..


Patch Set 6: Code-Review+2

Carrying forward Zoltan and Tim's +2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I265604ecbae2c80ff59e697fddfeea4976b25cd9
Gerrit-Change-Number: 15281
Gerrit-PatchSet: 6
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Norbert Luksa 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 05 Mar 2020 21:01:17 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-4080 [part 3]: Moved codegen code for TupleRowComparator into a Config object

2020-03-05 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15281 )

Change subject: IMPALA-4080 [part 3]: Moved codegen code for TupleRowComparator 
into a Config object
..


Patch Set 7: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I265604ecbae2c80ff59e697fddfeea4976b25cd9
Gerrit-Change-Number: 15281
Gerrit-PatchSet: 7
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Norbert Luksa 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 05 Mar 2020 21:01:47 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-4080 [part 3]: Moved codegen code for TupleRowComparator into a Config object

2020-03-05 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15281 )

Change subject: IMPALA-4080 [part 3]: Moved codegen code for TupleRowComparator 
into a Config object
..


Patch Set 7:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5456/ 
DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I265604ecbae2c80ff59e697fddfeea4976b25cd9
Gerrit-Change-Number: 15281
Gerrit-PatchSet: 7
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Norbert Luksa 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 05 Mar 2020 21:01:48 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-4080 [part 3]: Moved codegen code for TupleRowComparator into a Config object

2020-03-05 Thread Bikramjeet Vig (Code Review)
Hello Norbert Luksa, Daniel Becker, Zoltan Borok-Nagy, Tim Armstrong, Csaba 
Ringhofer, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-4080 [part 3]: Moved codegen code for TupleRowComparator 
into a Config object
..

IMPALA-4080 [part 3]: Moved codegen code for TupleRowComparator into a
Config object

This introduces a new class called TupleRowComparatorConfig that houses
the fragment level parameters needed to create a TupleRowComparator
object. It also codegens the TupleRowComparator::Compare() method that
is shared by all instances of TupleRowComparator objects created using
this config. Also, moved the related code to initiate codegen on this
class for the exchange, sort, partial sort and top-n nodes from their
exec nodes to their plan nodes.

Lastly, this also moved the code responsible for codegening
TopNNode::InsertBatch() from its exec node to its plan node.

Testing:
TODO: Sucessfully run exhaustive tests. Need to re-run after resolving
  rebase conflicts.
Manually verified that codegen works for all modified exec nodes.

Change-Id: I265604ecbae2c80ff59e697fddfeea4976b25cd9
---
M be/src/exec/exchange-node.cc
M be/src/exec/exchange-node.h
M be/src/exec/partial-sort-node.cc
M be/src/exec/partial-sort-node.h
M be/src/exec/sort-node.cc
M be/src/exec/sort-node.h
M be/src/exec/topn-node.cc
M be/src/exec/topn-node.h
M be/src/runtime/data-stream-test.cc
M be/src/runtime/sorter.cc
M be/src/runtime/sorter.h
M be/src/util/tuple-row-compare-test.cc
M be/src/util/tuple-row-compare.cc
M be/src/util/tuple-row-compare.h
14 files changed, 232 insertions(+), 203 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I265604ecbae2c80ff59e697fddfeea4976b25cd9
Gerrit-Change-Number: 15281
Gerrit-PatchSet: 6
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Norbert Luksa 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-4080 [part 3]: Moved codegen code for TupleRowComparator into a Config object

2020-03-05 Thread Bikramjeet Vig (Code Review)
Bikramjeet Vig has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15281 )

Change subject: IMPALA-4080 [part 3]: Moved codegen code for TupleRowComparator 
into a Config object
..


Patch Set 6:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/15281/5/be/src/exec/topn-node.h
File be/src/exec/topn-node.h:

http://gerrit.cloudera.org:8080/#/c/15281/5/be/src/exec/topn-node.h@123
PS5, Line 123:   TupleDescriptor* const output_tuple_desc_;
> Could be TupleDescriptor* const to document that it's set in constructor.
Done


http://gerrit.cloudera.org:8080/#/c/15281/5/be/src/util/tuple-row-compare.h
File be/src/util/tuple-row-compare.h:

http://gerrit.cloudera.org:8080/#/c/15281/5/be/src/util/tuple-row-compare.h@74
PS5, Line 74:   /// Indicates the sorting ordering used. Specified using the 
SORT BY clause.
> nit: you could add a comment here, something like "Indicates the ordering u
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I265604ecbae2c80ff59e697fddfeea4976b25cd9
Gerrit-Change-Number: 15281
Gerrit-PatchSet: 6
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Norbert Luksa 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 05 Mar 2020 21:00:44 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-9384: Improve Impala shell usability by enabling live progress in interactive mode

2020-03-05 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15219 )

Change subject: IMPALA-9384: Improve Impala shell usability by enabling 
live_progress in interactive mode
..


Patch Set 7: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3765b775f663fa227e59728acffe4d5ea9a5e2d3
Gerrit-Change-Number: 15219
Gerrit-PatchSet: 7
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 05 Mar 2020 20:51:55 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9384: Improve Impala shell usability by enabling live progress in interactive mode

2020-03-05 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15219 )

Change subject: IMPALA-9384: Improve Impala shell usability by enabling 
live_progress in interactive mode
..


Patch Set 7:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5455/ 
DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3765b775f663fa227e59728acffe4d5ea9a5e2d3
Gerrit-Change-Number: 15219
Gerrit-PatchSet: 7
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 05 Mar 2020 20:51:57 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9414 (part 2): Support the 'Expect: 100-continue' http header

2020-03-05 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15284 )

Change subject: IMPALA-9414 (part 2): Support the 'Expect: 100-continue' http 
header
..


Patch Set 6:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/5432/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4153968551acd58b25c7923c2ebf75ee29a7e76b
Gerrit-Change-Number: 15284
Gerrit-PatchSet: 6
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 05 Mar 2020 20:40:33 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9414 (part 2): Support the 'Expect: 100-continue' http header

2020-03-05 Thread Thomas Tauber-Marshall (Code Review)
Thomas Tauber-Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15284 )

Change subject: IMPALA-9414 (part 2): Support the 'Expect: 100-continue' http 
header
..


Patch Set 6:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/15284/5/be/src/transport/THttpTransport.cpp
File be/src/transport/THttpTransport.cpp:

http://gerrit.cloudera.org:8080/#/c/15284/5/be/src/transport/THttpTransport.cpp@255
PS5, Line 255:
> Are there any cases in which we'd wish to reject the request (based on the
That's a good question. There's definitely a lot of room for improvements 
around the hs2 http interface for preventing problems from poorly behaving or 
malicious clients.

I don't think that this patch makes that any worse though, so I think its fine 
to leave it for followup work.

I'll add more comments though.


http://gerrit.cloudera.org:8080/#/c/15284/5/be/src/transport/THttpTransport.cpp@257
PS5, Line 257:   // header, we respond that it can continue with sending the 
request. See Section 8.2.3
> Maybe reference Section 8.2.3 of RFC2616. I found it useful when understand
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4153968551acd58b25c7923c2ebf75ee29a7e76b
Gerrit-Change-Number: 15284
Gerrit-PatchSet: 6
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 05 Mar 2020 19:55:53 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-9414 (part 2): Support the 'Expect: 100-continue' http header

2020-03-05 Thread Thomas Tauber-Marshall (Code Review)
Hello David Knupp, Tim Armstrong, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-9414 (part 2): Support the 'Expect: 100-continue' http 
header
..

IMPALA-9414 (part 2): Support the 'Expect: 100-continue' http header

The 'Expect: 100-continue' http header allows http clients to send
only the headers for their request, get a confirmation back from the
server that the headers are valid, and only then send the body of the
request, avoiding the overhead of sending large requests that will
ultimately fail.

This patch adds support for this in the HS2 HTTP server by having
THttpServer look for the header, and if it's present and the request
is validated returning a '100 Continue' response before reading the
body of the request.

It also adds supports for using this header on large requests sent by
impala-shell.

Testing:
- This case is covered by the existing test_large_sql, however that
  test was previously broken and passing spuriously. This patch fixes
  the test.
- Passed all other shell tests.

Change-Id: I4153968551acd58b25c7923c2ebf75ee29a7e76b
---
M be/src/transport/THttpServer.cpp
M be/src/transport/THttpTransport.cpp
M be/src/transport/THttpTransport.h
M shell/THttpClient.py
M shell/impala_client.py
M shell/impala_shell.py
M shell/make_shell_tarball.sh
M shell/packaging/make_python_package.sh
A shell/shell_exceptions.py
M tests/shell/test_shell_commandline.py
10 files changed, 108 insertions(+), 52 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/84/15284/6
--
To view, visit http://gerrit.cloudera.org:8080/15284
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4153968551acd58b25c7923c2ebf75ee29a7e76b
Gerrit-Change-Number: 15284
Gerrit-PatchSet: 6
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-9384: Improve Impala shell usability by enabling live progress in interactive mode

2020-03-05 Thread David Knupp (Code Review)
David Knupp has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15219 )

Change subject: IMPALA-9384: Improve Impala shell usability by enabling 
live_progress in interactive mode
..


Patch Set 6: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3765b775f663fa227e59728acffe4d5ea9a5e2d3
Gerrit-Change-Number: 15219
Gerrit-PatchSet: 6
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 05 Mar 2020 18:53:26 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8674: fix bug where REMOTE runtime filter always marked disabled

2020-03-05 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15308 )

Change subject: IMPALA-8674: fix bug where REMOTE runtime filter always marked 
disabled
..


Patch Set 3:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/5431/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82a5a776103abd0a6d73336bebc65e22b4e13fef
Gerrit-Change-Number: 15308
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 05 Mar 2020 18:32:16 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9357: Fix race condition in alter database event

2020-03-05 Thread Anurag Mantripragada (Code Review)
Anurag Mantripragada has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15260 )

Change subject: IMPALA-9357: Fix race condition in alter_database event
..


Patch Set 5: Code-Review+1

(1 comment)

Thanks!

http://gerrit.cloudera.org:8080/#/c/15260/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:

http://gerrit.cloudera.org:8080/#/c/15260/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@449
PS5, Line 449: table
Did you mean DB?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I472fd8a55740769ee5cdb84e48422a4ab39a8d1e
Gerrit-Change-Number: 15260
Gerrit-PatchSet: 5
Gerrit-Owner: Vihang Karajgaonkar 
Gerrit-Reviewer: Anurag Mantripragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Comment-Date: Thu, 05 Mar 2020 18:19:17 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8674: fix bug where REMOTE runtime filter always marked disabled

2020-03-05 Thread Riza Suminto (Code Review)
Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15308 )

Change subject: IMPALA-8674: fix bug where REMOTE runtime filter always marked 
disabled
..


Patch Set 3:

(1 comment)

Patch set 3 submitted.

This change will consider a valid always_true filter as enabled. On the 
contrary, filter that becomes always_true due to RPC failure upon reading 
update from backend is considered as disable.

http://gerrit.cloudera.org:8080/#/c/15308/2/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:

http://gerrit.cloudera.org:8080/#/c/15308/2/be/src/runtime/coordinator.cc@1268
PS2, Line 1268: // still enabled at this point, it means all filter update 
has been successfully
> I think I agree with your assessments for point 1 to 5.
Done

I go with this proposed change and flip the flag for case 1 and 4.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82a5a776103abd0a6d73336bebc65e22b4e13fef
Gerrit-Change-Number: 15308
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 05 Mar 2020 17:56:35 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8674: fix bug where REMOTE runtime filter always marked disabled

2020-03-05 Thread Riza Suminto (Code Review)
Hello Fang-Yu Rao, Tim Armstrong, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-8674: fix bug where REMOTE runtime filter always marked 
disabled
..

IMPALA-8674: fix bug where REMOTE runtime filter always marked disabled

When a runtime filter has remote target, coordinator will Disable the
FilterState upon arrival of the last filter update to prevent another
update towards that filter. As consequence, such runtime filter will
always be displayed as disabled in runtime profile (Enabled column is
equal to false in Final filter table), when in reality the runtime
filter has heard back from all pending backends and complete. The
Enabled column should correctly distinguish between failed runtime
filter vs complete runtime filter. To do so, we add
all_update_received_ flag in FilterState class and set it to true
after filter received enough filter update from pending backends to
proceed. If all_update_received_ is true, then that runtime filter is
considered as enabled.

Testing:
- Add row regex in runtime_filters.test, query 6, to verify REMOTE
  runtime filter is marked as enabled in final filter table
- Run and pass test_runtime_filters.py
- Run and pass core tests

Change-Id: I82a5a776103abd0a6d73336bebc65e22b4e13fef
---
M be/src/runtime/coordinator-filter-state.h
M be/src/runtime/coordinator.cc
M testdata/workloads/functional-query/queries/QueryTest/runtime_filters.test
3 files changed, 25 insertions(+), 2 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I82a5a776103abd0a6d73336bebc65e22b4e13fef
Gerrit-Change-Number: 15308
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] WIP: Asynchronous code generation

2020-03-05 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15105 )

Change subject: WIP: Asynchronous code generation
..


Patch Set 18:

I would suggest just unnesting the typedef and putting it at the top of the 
file. Nested definitions cause a lot of problems like this, I think they're 
mostly not a good idea *unless* they're only used internally within the class 
that they're defined in.

I originally used nested definitions fairly liberally myself but I changed my 
mind on this after trying to reorganise DiskIoMgr headers to reduce the number 
of header dependencies - it was impossible without unnesting all of the classes.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7cbfa7c6734dcf03641629429057d6a4194aa6b
Gerrit-Change-Number: 15105
Gerrit-PatchSet: 18
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 05 Mar 2020 15:58:55 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6506: Codegen in ORC scanner for primitives and struct

2020-03-05 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15350 )

Change subject: IMPALA-6506: Codegen in ORC scanner for primitives and struct
..


Patch Set 3: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2352d0c8fc75ff722e931bc8c866b3e43d3636f4
Gerrit-Change-Number: 15350
Gerrit-PatchSet: 3
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 05 Mar 2020 13:21:33 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6506: Codegen in ORC scanner for primitives and struct

2020-03-05 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/15350 )

Change subject: IMPALA-6506: Codegen in ORC scanner for primitives and struct
..

IMPALA-6506: Codegen in ORC scanner for primitives and struct

IMPALA-9228 introduced scratch batch handling for struct and
primitive types in the ORC scanner and the existing scratch batch
logic already supports Codegen for ProcessScratchBatch() function.
This change turns on this Codegen logic for primitives types and
structs in the ORC scanner.

Note, if the query involves collection types then
ProcessScratchBatch() is still codegend but the codegend function
isn't used as the regular row-by-row approach is followed in this
case without using a scratch batch.

Testing:
  - Re-run the whole test suite to check for regressions.
  - Checked the performance on a scale 25 TPCH workload in ORC format
using single_node_perf_run.py. Comparing the query runtimes it
seems that codegen brings a 1-21% improvement for most of the
queries. There is a slight decrease in 3 queries that are not
scan-heavy where codegen doesn't provide any help for scanning.
However, these are short queries where the size of the
degradation is in subseconds so I'd say the decrease is
negligible.
  - Did a manual check for a table that contains both Parquet and ORC
partitions. Verified that in this case ProcessScratchBatch() is
codegend for both formats and the query results are as expected.

Change-Id: I2352d0c8fc75ff722e931bc8c866b3e43d3636f4
Reviewed-on: http://gerrit.cloudera.org:8080/15350
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M be/src/exec/hdfs-columnar-scanner.cc
M be/src/exec/hdfs-columnar-scanner.h
M be/src/exec/hdfs-orc-scanner.cc
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/exec/parquet/hdfs-parquet-scanner.h
6 files changed, 58 insertions(+), 47 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I2352d0c8fc75ff722e931bc8c866b3e43d3636f4
Gerrit-Change-Number: 15350
Gerrit-PatchSet: 4
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-6360: Don't show full query statement on Impala WebUI by default

2020-03-05 Thread Norbert Luksa (Code Review)
Norbert Luksa has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15288 )

Change subject: IMPALA-6360: Don't show full query statement on Impala WebUI by 
default
..


Patch Set 12: Code-Review+1

(1 comment)

Hi, left a comment, otherwise LGTM.

http://gerrit.cloudera.org:8080/#/c/15288/12/tests/webserver/test_web_pages.py
File tests/webserver/test_web_pages.py:

http://gerrit.cloudera.org:8080/#/c/15288/12/tests/webserver/test_web_pages.py@423
PS12, Line 423: query
I think it would be easier to check this test if you would create it with an 
expression like:
"select \"{0}\"".format("x" * 439)
That way you still get a 448 long string, but it's easier to count.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib7109a0be5d1022b4f8d6e72441cf5dc1dc42605
Gerrit-Change-Number: 15288
Gerrit-PatchSet: 12
Gerrit-Owner: Adam Tamas 
Gerrit-Reviewer: Adam Tamas 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Norbert Luksa 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Comment-Date: Thu, 05 Mar 2020 13:12:38 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-4080 [part 3]: Moved codegen code for TupleRowComparator into a Config object

2020-03-05 Thread Zoltan Borok-Nagy (Code Review)
Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15281 )

Change subject: IMPALA-4080 [part 3]: Moved codegen code for TupleRowComparator 
into a Config object
..


Patch Set 5: Code-Review+2

LGTM once Norbert's comment is resolved


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I265604ecbae2c80ff59e697fddfeea4976b25cd9
Gerrit-Change-Number: 15281
Gerrit-PatchSet: 5
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Norbert Luksa 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 05 Mar 2020 12:51:19 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-4080 [part 3]: Moved codegen code for TupleRowComparator into a Config object

2020-03-05 Thread Norbert Luksa (Code Review)
Norbert Luksa has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15281 )

Change subject: IMPALA-4080 [part 3]: Moved codegen code for TupleRowComparator 
into a Config object
..


Patch Set 5: Code-Review+1

(1 comment)

Left a comment, but LGTM.

http://gerrit.cloudera.org:8080/#/c/15281/5/be/src/util/tuple-row-compare.h
File be/src/util/tuple-row-compare.h:

http://gerrit.cloudera.org:8080/#/c/15281/5/be/src/util/tuple-row-compare.h@74
PS5, Line 74:   TSortingOrder::type sorting_order_;
nit: you could add a comment here, something like "Indicates the ordering used 
to instantiate the comparator." And maybe: "Only used with SORT BY queries".



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I265604ecbae2c80ff59e697fddfeea4976b25cd9
Gerrit-Change-Number: 15281
Gerrit-PatchSet: 5
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Norbert Luksa 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 05 Mar 2020 12:28:55 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] WIP: Asynchronous code generation

2020-03-05 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15105 )

Change subject: WIP: Asynchronous code generation
..


Patch Set 18:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/5430/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7cbfa7c6734dcf03641629429057d6a4194aa6b
Gerrit-Change-Number: 15105
Gerrit-PatchSet: 18
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 05 Mar 2020 11:54:43 +
Gerrit-HasComments: No


[Impala-ASF-CR] WIP: Asynchronous code generation

2020-03-05 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15105 )

Change subject: WIP: Asynchronous code generation
..


Patch Set 17:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/5429/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7cbfa7c6734dcf03641629429057d6a4194aa6b
Gerrit-Change-Number: 15105
Gerrit-PatchSet: 17
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 05 Mar 2020 11:43:35 +
Gerrit-HasComments: No


[Impala-ASF-CR] WIP: Asynchronous code generation

2020-03-05 Thread Daniel Becker (Code Review)
Daniel Becker has uploaded a new patch set (#18). ( 
http://gerrit.cloudera.org:8080/15105 )

Change subject: WIP: Asynchronous code generation
..

WIP: Asynchronous code generation

This commit introduces optional asynchronous code generation.

Asynchronous code generation means that instead of waiting for codegen
to finish, the query starts in interpreted mode while codegen is done on
another thread.

All the function pointers that point to codegen'd functions are changed
to be atomic, wrapped in a CodegenFnPtr. These are initialised to
nullptr and as long as they are nullptr, the corresponding interpreted
functions are used (as before). When code generation is ready, the
funtion pointers are set by the codegen thread. No synchronisation is
needed as the function pointers are atomic and it is not a problem if,
at a given moment, only a subset of the codegen'd function pointers are
set and the rest are interpreted.

Asynchronous code generation can be turned on using the ASYNC_CODEGEN
boolean query option.

TODO: The default should be synchronous codegen for now.
TODO: Testing.
TODO: Benchmarks.

Change-Id: Ia7cbfa7c6734dcf03641629429057d6a4194aa6b
---
M be/src/benchmarks/hash-benchmark.cc
A be/src/codegen/codegen-fn-ptr.h
M be/src/codegen/llvm-codegen-test.cc
M be/src/codegen/llvm-codegen.cc
M be/src/codegen/llvm-codegen.h
M be/src/exec/grouping-aggregator.cc
M be/src/exec/grouping-aggregator.h
M be/src/exec/hdfs-avro-scanner.cc
M be/src/exec/hdfs-avro-scanner.h
M be/src/exec/hdfs-columnar-scanner.cc
M be/src/exec/hdfs-columnar-scanner.h
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-scan-node-base.h
M be/src/exec/hdfs-scanner.cc
M be/src/exec/hdfs-scanner.h
M be/src/exec/hdfs-sequence-scanner.cc
M be/src/exec/hdfs-text-scanner.cc
M be/src/exec/non-grouping-aggregator.cc
M be/src/exec/non-grouping-aggregator.h
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/exec/partitioned-hash-join-builder.cc
M be/src/exec/partitioned-hash-join-builder.h
M be/src/exec/partitioned-hash-join-node.cc
M be/src/exec/partitioned-hash-join-node.h
M be/src/exec/select-node.cc
M be/src/exec/select-node.h
M be/src/exec/topn-node.cc
M be/src/exec/topn-node.h
M be/src/exec/union-node.cc
M be/src/exec/union-node.h
M be/src/exprs/expr-codegen-test.cc
M be/src/exprs/scalar-expr.cc
M be/src/exprs/scalar-expr.h
M be/src/exprs/scalar-expr.inline.h
M be/src/exprs/scalar-fn-call.cc
M be/src/exprs/scalar-fn-call.h
M be/src/runtime/fragment-instance-state.cc
M be/src/runtime/krpc-data-stream-sender.cc
M be/src/runtime/krpc-data-stream-sender.h
M be/src/runtime/runtime-state.h
M be/src/service/query-options.cc
M be/src/service/query-options.h
M be/src/util/tuple-row-compare.cc
M be/src/util/tuple-row-compare.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M tests/query_test/test_queries.py
M tests/query_test/test_query_mem_limit.py
48 files changed, 653 insertions(+), 349 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/05/15105/18
-- 
To view, visit http://gerrit.cloudera.org:8080/15105
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia7cbfa7c6734dcf03641629429057d6a4194aa6b
Gerrit-Change-Number: 15105
Gerrit-PatchSet: 18
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] WIP: Asynchronous code generation

2020-03-05 Thread Daniel Becker (Code Review)
Daniel Becker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15105 )

Change subject: WIP: Asynchronous code generation
..


Patch Set 18:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/15105/16/be/src/codegen/codegen-fn-ptr.h
File be/src/codegen/codegen-fn-ptr.h:

http://gerrit.cloudera.org:8080/#/c/15105/16/be/src/codegen/codegen-fn-ptr.h@38
PS16, Line 38: stexpr std::memory_order mem_order_load_ = 
std::memory_order_acquire;
 :   static constexpr std::memory_order mem_order_store_ = 
std::memory_order_release;
> nit: I think you can delete this part
Done


http://gerrit.cloudera.org:8080/#/c/15105/16/be/src/exec/hdfs-scanner.h
File be/src/exec/hdfs-scanner.h:

http://gerrit.cloudera.org:8080/#/c/15105/16/be/src/exec/hdfs-scanner.h@465
PS16, Line 465: i
> nit: we usually use /// for doc comments.
Done


http://gerrit.cloudera.org:8080/#/c/15105/16/be/src/runtime/fragment-instance-state.cc
File be/src/runtime/fragment-instance-state.cc:

http://gerrit.cloudera.org:8080/#/c/15105/16/be/src/runtime/fragment-instance-state.cc@358
PS16, Line 358:
> nit: too many /
Done


http://gerrit.cloudera.org:8080/#/c/15105/17/tests/query_test/test_query_mem_limit.py
File tests/query_test/test_query_mem_limit.py:

http://gerrit.cloudera.org:8080/#/c/15105/17/tests/query_test/test_query_mem_limit.py@130
PS17, Line 130:
> flake8: E501 line too long (92 > 90 characters)
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7cbfa7c6734dcf03641629429057d6a4194aa6b
Gerrit-Change-Number: 15105
Gerrit-PatchSet: 18
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 05 Mar 2020 11:08:27 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] WIP: Asynchronous code generation

2020-03-05 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15105 )

Change subject: WIP: Asynchronous code generation
..


Patch Set 17:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15105/17/tests/query_test/test_query_mem_limit.py
File tests/query_test/test_query_mem_limit.py:

http://gerrit.cloudera.org:8080/#/c/15105/17/tests/query_test/test_query_mem_limit.py@130
PS17, Line 130: h
flake8: E501 line too long (92 > 90 characters)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7cbfa7c6734dcf03641629429057d6a4194aa6b
Gerrit-Change-Number: 15105
Gerrit-PatchSet: 17
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 05 Mar 2020 10:59:01 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] WIP: Asynchronous code generation

2020-03-05 Thread Daniel Becker (Code Review)
Daniel Becker has uploaded a new patch set (#17). ( 
http://gerrit.cloudera.org:8080/15105 )

Change subject: WIP: Asynchronous code generation
..

WIP: Asynchronous code generation

This commit introduces optional asynchronous code generation.

Asynchronous code generation means that instead of waiting for codegen
to finish, the query starts in interpreted mode while codegen is done on
another thread.

All the function pointers that point to codegen'd functions are changed
to be atomic, wrapped in a CodegenFnPtr. These are initialised to
nullptr and as long as they are nullptr, the corresponding interpreted
functions are used (as before). When code generation is ready, the
funtion pointers are set by the codegen thread. No synchronisation is
needed as the function pointers are atomic and it is not a problem if,
at a given moment, only a subset of the codegen'd function pointers are
set and the rest are interpreted.

Asynchronous code generation can be turned on using the ASYNC_CODEGEN
boolean query option.

TODO: The default should be synchronous codegen for now.
TODO: Testing.
TODO: Benchmarks.

Change-Id: Ia7cbfa7c6734dcf03641629429057d6a4194aa6b
---
M be/src/benchmarks/hash-benchmark.cc
A be/src/codegen/codegen-fn-ptr.h
M be/src/codegen/llvm-codegen-test.cc
M be/src/codegen/llvm-codegen.cc
M be/src/codegen/llvm-codegen.h
M be/src/exec/grouping-aggregator.cc
M be/src/exec/grouping-aggregator.h
M be/src/exec/hdfs-avro-scanner.cc
M be/src/exec/hdfs-avro-scanner.h
M be/src/exec/hdfs-columnar-scanner.cc
M be/src/exec/hdfs-columnar-scanner.h
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-scan-node-base.h
M be/src/exec/hdfs-scanner.cc
M be/src/exec/hdfs-scanner.h
M be/src/exec/hdfs-sequence-scanner.cc
M be/src/exec/hdfs-text-scanner.cc
M be/src/exec/non-grouping-aggregator.cc
M be/src/exec/non-grouping-aggregator.h
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/exec/partitioned-hash-join-builder.cc
M be/src/exec/partitioned-hash-join-builder.h
M be/src/exec/partitioned-hash-join-node.cc
M be/src/exec/partitioned-hash-join-node.h
M be/src/exec/select-node.cc
M be/src/exec/select-node.h
M be/src/exec/topn-node.cc
M be/src/exec/topn-node.h
M be/src/exec/union-node.cc
M be/src/exec/union-node.h
M be/src/exprs/expr-codegen-test.cc
M be/src/exprs/scalar-expr.cc
M be/src/exprs/scalar-expr.h
M be/src/exprs/scalar-expr.inline.h
M be/src/exprs/scalar-fn-call.cc
M be/src/exprs/scalar-fn-call.h
M be/src/runtime/fragment-instance-state.cc
M be/src/runtime/krpc-data-stream-sender.cc
M be/src/runtime/krpc-data-stream-sender.h
M be/src/runtime/runtime-state.h
M be/src/service/query-options.cc
M be/src/service/query-options.h
M be/src/util/tuple-row-compare.cc
M be/src/util/tuple-row-compare.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M tests/query_test/test_queries.py
M tests/query_test/test_query_mem_limit.py
48 files changed, 655 insertions(+), 349 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/05/15105/17
-- 
To view, visit http://gerrit.cloudera.org:8080/15105
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia7cbfa7c6734dcf03641629429057d6a4194aa6b
Gerrit-Change-Number: 15105
Gerrit-PatchSet: 17
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-7784: Use unescaped string in partition pruning + fix duplicatedly unescaping strings

2020-03-05 Thread Csaba Ringhofer (Code Review)
Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15278 )

Change subject: IMPALA-7784: Use unescaped string in partition pruning + fix 
duplicatedly unescaping strings
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iea8070f16a74f9aeade294504f2834abb8b3b38f
Gerrit-Change-Number: 15278
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Comment-Date: Thu, 05 Mar 2020 09:38:37 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6506: Codegen in ORC scanner for primitives and struct

2020-03-05 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15350 )

Change subject: IMPALA-6506: Codegen in ORC scanner for primitives and struct
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2352d0c8fc75ff722e931bc8c866b3e43d3636f4
Gerrit-Change-Number: 15350
Gerrit-PatchSet: 3
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 05 Mar 2020 08:24:55 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6506: Codegen in ORC scanner for primitives and struct

2020-03-05 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15350 )

Change subject: IMPALA-6506: Codegen in ORC scanner for primitives and struct
..


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5454/ 
DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2352d0c8fc75ff722e931bc8c866b3e43d3636f4
Gerrit-Change-Number: 15350
Gerrit-PatchSet: 3
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 05 Mar 2020 08:24:56 +
Gerrit-HasComments: No