[Impala-ASF-CR] IMPALA-5969: [DOCS] Adds --auth creds ok in clear to shell options

2018-05-07 Thread Anonymous Coward (Code Review)
Hello Alex Rodoni, Jim Apple,

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

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

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

Change subject: IMPALA-5969: [DOCS] Adds --auth_creds_ok_in_clear to shell 
options
..

IMPALA-5969: [DOCS] Adds --auth_creds_ok_in_clear to shell options

This patch adds --auth_creds_ok_in_clear to the impala_shell_options
documentation xml

Change-Id: I19450ebd839b84a85598d283c04a77662fa5e44e
---
M docs/topics/impala_shell_options.xml
1 file changed, 9 insertions(+), 0 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I19450ebd839b84a85598d283c04a77662fa5e44e
Gerrit-Change-Number: 10236
Gerrit-PatchSet: 2
Gerrit-Owner: shashanknaik...@gmail.com
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: shashanknaik...@gmail.com


[Impala-ASF-CR] IMPALA-6948: Delete catalog update topic entries upon catalog restart

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

Change subject: IMPALA-6948: Delete catalog update topic entries upon catalog 
restart
..

IMPALA-6948: Delete catalog update topic entries upon catalog restart

This commit fixes an issue where the statestore may end up with stale
entries in the catalog update topic that do not correspond to the
catalog objects stored in the catalog. This may occur if the catalog
server restarts and some catalog object (e.g. table) that was known to
the catalog before the restart no longer exists in the Hive Metastore
after the restart.

Fix:
The first update for the catalog update topic that is sent by the catalog
instructs the statestore to clear any entries it may have on this topic
before applying the first update. In that way, we guarantee that the
statestore entries are consistent with the catalog objects stored in the
catalog server. Any coordinator that detects the catalog restart will
receive from the statestore a full topic update that reflects the state
of the catalog server.

Testing:
Added statestore test.

Change-Id: I907509bf92da631ece5efd23c275a613ead00e91

Tmp

Change-Id: I74a8ade8e498ac35cb56d3775d2c67a86988d9b6
Reviewed-on: http://gerrit.cloudera.org:8080/10289
Reviewed-by: Vuk Ercegovac 
Tested-by: Impala Public Jenkins 
---
M be/src/catalog/catalog-server.cc
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
M common/thrift/StatestoreService.thrift
M fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
M tests/statestore/test_statestore.py
6 files changed, 98 insertions(+), 7 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I74a8ade8e498ac35cb56d3775d2c67a86988d9b6
Gerrit-Change-Number: 10289
Gerrit-PatchSet: 5
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 


[Impala-ASF-CR] IMPALA-6948: Delete catalog update topic entries upon catalog restart

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

Change subject: IMPALA-6948: Delete catalog update topic entries upon catalog 
restart
..


Patch Set 4: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I74a8ade8e498ac35cb56d3775d2c67a86988d9b6
Gerrit-Change-Number: 10289
Gerrit-PatchSet: 4
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 08 May 2018 03:31:22 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6970: race with decreasing scanner reservation

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

Change subject: IMPALA-6970: race with decreasing scanner reservation
..

IMPALA-6970: race with decreasing scanner reservation

The usage pattern of DecreaseReservationTo() in
ReturnReservationFromScannerThread() can decrease the reservation by
more than 'bytes' if there is a concurrent IncreaseReservation() call.

The API is changed to fit the requirements of the scanner, which
does not want to release more than the memory reserved for that
scanner thread.

Testing:
Added some backend tests (including concurrency) for the new API.

Change-Id: I653225c981bf674d2b2b947f3a3cb4d8f382d36b
Reviewed-on: http://gerrit.cloudera.org:8080/10314
Reviewed-by: Tim Armstrong 
Tested-by: Impala Public Jenkins 
---
M be/src/benchmarks/bloom-filter-benchmark.cc
M be/src/exec/exec-node.cc
M be/src/exec/exec-node.h
M be/src/exec/hdfs-scan-node.cc
M be/src/runtime/bufferpool/buffer-pool-internal.h
M be/src/runtime/bufferpool/buffer-pool-test.cc
M be/src/runtime/bufferpool/buffer-pool.cc
M be/src/runtime/bufferpool/buffer-pool.h
8 files changed, 74 insertions(+), 35 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I653225c981bf674d2b2b947f3a3cb4d8f382d36b
Gerrit-Change-Number: 10314
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-6970: race with decreasing scanner reservation

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

Change subject: IMPALA-6970: race with decreasing scanner reservation
..


Patch Set 4: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I653225c981bf674d2b2b947f3a3cb4d8f382d36b
Gerrit-Change-Number: 10314
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 08 May 2018 03:04:40 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6987: [DOCS] Update when INVALIDATE METADATA is required

2018-05-07 Thread Alex Rodoni (Code Review)
Alex Rodoni has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10339


Change subject: IMPALA-6987: [DOCS] Update when INVALIDATE METADATA is required
..

IMPALA-6987: [DOCS] Update when INVALIDATE METADATA is required

Change-Id: I2124e14900d0f82569c061cc46006447bb054b36
---
M docs/topics/impala_invalidate_metadata.xml
1 file changed, 4 insertions(+), 17 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2124e14900d0f82569c061cc46006447bb054b36
Gerrit-Change-Number: 10339
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 


[Impala-ASF-CR] IMPALA-6948: Delete catalog update topic entries upon catalog restart

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

Change subject: IMPALA-6948: Delete catalog update topic entries upon catalog 
restart
..


Patch Set 4:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2421/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I74a8ade8e498ac35cb56d3775d2c67a86988d9b6
Gerrit-Change-Number: 10289
Gerrit-PatchSet: 4
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 07 May 2018 23:50:14 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6227: reduce window of metric inconsistency

2018-05-07 Thread Bikramjeet Vig (Code Review)
Bikramjeet Vig has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10330 )

Change subject: IMPALA-6227: reduce window of metric inconsistency
..


Patch Set 2: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10330/2/tests/custom_cluster/test_admission_controller.py
File tests/custom_cluster/test_admission_controller.py:

http://gerrit.cloudera.org:8080/#/c/10330/2/tests/custom_cluster/test_admission_controller.py@545
PS2, Line 545: got
nit: get



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2f16edbec53e49446c4c37ef5f926eedb5604319
Gerrit-Change-Number: 10330
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Comment-Date: Mon, 07 May 2018 23:50:11 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6948: Delete catalog update topic entries upon catalog restart

2018-05-07 Thread Vuk Ercegovac (Code Review)
Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10289 )

Change subject: IMPALA-6948: Delete catalog update topic entries upon catalog 
restart
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I74a8ade8e498ac35cb56d3775d2c67a86988d9b6
Gerrit-Change-Number: 10289
Gerrit-PatchSet: 4
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 07 May 2018 23:44:55 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6948: Delete catalog update topic entries upon catalog restart

2018-05-07 Thread Dimitris Tsirogiannis (Code Review)
Hello Alex Behm, Vuk Ercegovac,

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

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

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

Change subject: IMPALA-6948: Delete catalog update topic entries upon catalog 
restart
..

IMPALA-6948: Delete catalog update topic entries upon catalog restart

This commit fixes an issue where the statestore may end up with stale
entries in the catalog update topic that do not correspond to the
catalog objects stored in the catalog. This may occur if the catalog
server restarts and some catalog object (e.g. table) that was known to
the catalog before the restart no longer exists in the Hive Metastore
after the restart.

Fix:
The first update for the catalog update topic that is sent by the catalog
instructs the statestore to clear any entries it may have on this topic
before applying the first update. In that way, we guarantee that the
statestore entries are consistent with the catalog objects stored in the
catalog server. Any coordinator that detects the catalog restart will
receive from the statestore a full topic update that reflects the state
of the catalog server.

Testing:
Added statestore test.

Change-Id: I907509bf92da631ece5efd23c275a613ead00e91

Tmp

Change-Id: I74a8ade8e498ac35cb56d3775d2c67a86988d9b6
---
M be/src/catalog/catalog-server.cc
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
M common/thrift/StatestoreService.thrift
M fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
M tests/statestore/test_statestore.py
6 files changed, 98 insertions(+), 7 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I74a8ade8e498ac35cb56d3775d2c67a86988d9b6
Gerrit-Change-Number: 10289
Gerrit-PatchSet: 4
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Vuk Ercegovac 


[Impala-ASF-CR] IMPALA-6948: Delete catalog update topic entries upon catalog restart

2018-05-07 Thread Dimitris Tsirogiannis (Code Review)
Dimitris Tsirogiannis has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10289 )

Change subject: IMPALA-6948: Delete catalog update topic entries upon catalog 
restart
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/10289/1/be/src/statestore/statestore.cc
File be/src/statestore/statestore.cc:

http://gerrit.cloudera.org:8080/#/c/10289/1/be/src/statestore/statestore.cc@665
PS1, Line 665:   // cleared.
> makes sense. however, if I were to test these code paths, I would add a cas
Fair enough, added a dcheck to ensure that both from_version and 
clear_topic_entries are not set at the same time.


http://gerrit.cloudera.org:8080/#/c/10289/3/be/src/statestore/statestore.cc
File be/src/statestore/statestore.cc:

http://gerrit.cloudera.org:8080/#/c/10289/3/be/src/statestore/statestore.cc@215
PS3, Line 215:
> if all is correct, the metric won't be negative. however, what happens if i
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I74a8ade8e498ac35cb56d3775d2c67a86988d9b6
Gerrit-Change-Number: 10289
Gerrit-PatchSet: 1
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 07 May 2018 23:33:33 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6970: race with decreasing scanner reservation

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

Change subject: IMPALA-6970: race with decreasing scanner reservation
..


Patch Set 4:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2420/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I653225c981bf674d2b2b947f3a3cb4d8f382d36b
Gerrit-Change-Number: 10314
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 07 May 2018 23:24:24 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6970: race with decreasing scanner reservation

2018-05-07 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10314 )

Change subject: IMPALA-6970: race with decreasing scanner reservation
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I653225c981bf674d2b2b947f3a3cb4d8f382d36b
Gerrit-Change-Number: 10314
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 07 May 2018 23:24:04 +
Gerrit-HasComments: No


[Impala-ASF-CR] test-with-docker: work with git worktree

2018-05-07 Thread Philip Zeyliger (Code Review)
Philip Zeyliger has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10335


Change subject: test-with-docker: work with git worktree
..

test-with-docker: work with git worktree

This commit adds a little of git-wrangling to allow test-with-docker to
work when invoked from git directories managed by "git worktree". These
are different in that they reference another git directory elsewhere on
the file system, which also needs to be mounted into the container.

Change-Id: I9186e0b6f068aacc25f8d691508165c04329fa8b
---
M docker/entrypoint.sh
M docker/test-with-docker.py
2 files changed, 13 insertions(+), 2 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9186e0b6f068aacc25f8d691508165c04329fa8b
Gerrit-Change-Number: 10335
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger 


[Impala-ASF-CR] IMPALA-6970: race with decreasing scanner reservation

2018-05-07 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10314 )

Change subject: IMPALA-6970: race with decreasing scanner reservation
..


Patch Set 3: Code-Review+1

carry +1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I653225c981bf674d2b2b947f3a3cb4d8f382d36b
Gerrit-Change-Number: 10314
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 07 May 2018 23:09:08 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6970: race with decreasing scanner reservation

2018-05-07 Thread Tim Armstrong (Code Review)
Hello Bikramjeet Vig, Dan Hecht,

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

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

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

Change subject: IMPALA-6970: race with decreasing scanner reservation
..

IMPALA-6970: race with decreasing scanner reservation

The usage pattern of DecreaseReservationTo() in
ReturnReservationFromScannerThread() can decrease the reservation by
more than 'bytes' if there is a concurrent IncreaseReservation() call.

The API is changed to fit the requirements of the scanner, which
does not want to release more than the memory reserved for that
scanner thread.

Testing:
Added some backend tests (including concurrency) for the new API.

Change-Id: I653225c981bf674d2b2b947f3a3cb4d8f382d36b
---
M be/src/benchmarks/bloom-filter-benchmark.cc
M be/src/exec/exec-node.cc
M be/src/exec/exec-node.h
M be/src/exec/hdfs-scan-node.cc
M be/src/runtime/bufferpool/buffer-pool-internal.h
M be/src/runtime/bufferpool/buffer-pool-test.cc
M be/src/runtime/bufferpool/buffer-pool.cc
M be/src/runtime/bufferpool/buffer-pool.h
8 files changed, 74 insertions(+), 35 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I653225c981bf674d2b2b947f3a3cb4d8f382d36b
Gerrit-Change-Number: 10314
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-6970: race with decreasing scanner reservation

2018-05-07 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10314 )

Change subject: IMPALA-6970: race with decreasing scanner reservation
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/10314/2/be/src/runtime/bufferpool/buffer-pool-test.cc
File be/src/runtime/bufferpool/buffer-pool-test.cc:

http://gerrit.cloudera.org:8080/#/c/10314/2/be/src/runtime/bufferpool/buffer-pool-test.cc@2158
PS2, Line 2158: thread incre
> nit: maybe just use 'thread' if we need only one
Done


http://gerrit.cloudera.org:8080/#/c/10314/2/be/src/runtime/bufferpool/buffer-pool.h
File be/src/runtime/bufferpool/buffer-pool.h:

http://gerrit.cloudera.org:8080/#/c/10314/2/be/src/runtime/bufferpool/buffer-pool.h@349
PS2, Line 349:
 :   bool IncreaseReservationToFit(int64_t bytes) 
WARN_UNUSED_RESULT;
> Maybe just delete that sentence? It seems obvious that if you do Increase a
Removed. Yeah it really is obvious :).



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I653225c981bf674d2b2b947f3a3cb4d8f382d36b
Gerrit-Change-Number: 10314
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 07 May 2018 23:08:53 +
Gerrit-HasComments: Yes


[Impala-ASF-CR](asf-site) Update download and signature links for 3.0.0 release.

2018-05-07 Thread Michael Brown (Code Review)
Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10333 )

Change subject: Update download and signature links for 3.0.0 release.
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10333/1/downloads.html
File downloads.html:

http://gerrit.cloudera.org:8080/#/c/10333/1/downloads.html@135
PS1, Line 135: 
http://www.apache.org/dyn/closer.cgi?action=downloadfilename=impala/3.0.0/apache-impala-3.0.0.tar.gz
Redirects to 
http://download.nextag.com/apache/impala/3.0.0/apache-impala-3.0.0.tar.gz which 
is 404 for me. Is that expected at this point?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-MessageType: comment
Gerrit-Change-Id: I307d786bf4709d24f1a9511dcc0e6759183e8eae
Gerrit-Change-Number: 10333
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Mon, 07 May 2018 22:45:27 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-4850: [DOCS] COMMENT should come after PARTITIONED BY

2018-05-07 Thread Alex Rodoni (Code Review)
Alex Rodoni has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10316 )

Change subject: IMPALA-4850: [DOCS] COMMENT should come after PARTITIONED BY
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10316/1/docs/topics/impala_create_table.xml
File docs/topics/impala_create_table.xml:

http://gerrit.cloudera.org:8080/#/c/10316/1/docs/topics/impala_create_table.xml@172
PS1, Line 172:   [COMMENT 'table_comment']
> This bug is from a long time ago. In reviewing this, I noticed that there a
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I03fd4a308981955bb52ca79772fe2f7c01b5894f
Gerrit-Change-Number: 10316
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Comment-Date: Mon, 07 May 2018 22:40:57 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-4850: [DOCS] COMMENT should come after PARTITIONED BY

2018-05-07 Thread Alex Rodoni (Code Review)
Hello Joe McDonnell, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-4850: [DOCS] COMMENT should come after PARTITIONED BY
..

IMPALA-4850: [DOCS] COMMENT should come after PARTITIONED BY

Change-Id: I03fd4a308981955bb52ca79772fe2f7c01b5894f
---
M docs/topics/impala_create_table.xml
1 file changed, 10 insertions(+), 13 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I03fd4a308981955bb52ca79772fe2f7c01b5894f
Gerrit-Change-Number: 10316
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 


[Impala-ASF-CR](asf-site) [DOCS] Impala doc site update for 3.0

2018-05-07 Thread Michael Brown (Code Review)
Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10322 )

Change subject: [DOCS] Impala doc site update for 3.0
..


Patch Set 4:

(2 comments)

soft +2. I think this can go in as soon as the dependent review referenced 
below is in

http://gerrit.cloudera.org:8080/#/c/10322/1/docs/build/html/topics/impala_known_issues.html
File docs/build/html/topics/impala_known_issues.html:

http://gerrit.cloudera.org:8080/#/c/10322/1/docs/build/html/topics/impala_known_issues.html@36
PS1, Line 36: div>
:
:Yes, and I committed the fix.
Great, I found it too.


http://gerrit.cloudera.org:8080/#/c/10322/4/impala-docs.html
File impala-docs.html:

http://gerrit.cloudera.org:8080/#/c/10322/4/impala-docs.html@130
PS4, Line 130: docs/changelog-3.0.html
Let's wait for https://gerrit.cloudera.org/#/c/10334/



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf5927efa7baa965095a3ff2fd4ec7411313342d
Gerrit-Change-Number: 10322
Gerrit-PatchSet: 4
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Mon, 07 May 2018 22:35:08 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6970: race with decreasing scanner reservation

2018-05-07 Thread Dan Hecht (Code Review)
Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10314 )

Change subject: IMPALA-6970: race with decreasing scanner reservation
..


Patch Set 2: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10314/2/be/src/runtime/bufferpool/buffer-pool.h
File be/src/runtime/bufferpool/buffer-pool.h:

http://gerrit.cloudera.org:8080/#/c/10314/2/be/src/runtime/bufferpool/buffer-pool.h@349
PS2, Line 349: the postcondition
 :   /// does not hold if called concurrently with a 
DecreaseReservation*() call.
> do you mean that the post-condition of "after which 'bytes' can be used" do
Maybe just delete that sentence? It seems obvious that if you do Increase and 
Decrease without some external synchronization then you may not have 'bytes' at 
the end. Or is it trying to say else?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I653225c981bf674d2b2b947f3a3cb4d8f382d36b
Gerrit-Change-Number: 10314
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 07 May 2018 22:31:49 +
Gerrit-HasComments: Yes


[Impala-ASF-CR](asf-site) Added changelog for 3.0.0

2018-05-07 Thread Michael Brown (Code Review)
Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10334 )

Change subject: Added changelog for 3.0.0
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-MessageType: comment
Gerrit-Change-Id: I895f54fc47bada6b5131f20167405781c7d284b5
Gerrit-Change-Number: 10334
Gerrit-PatchSet: 2
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Mon, 07 May 2018 22:26:45 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6970: race with decreasing scanner reservation

2018-05-07 Thread Bikramjeet Vig (Code Review)
Bikramjeet Vig has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10314 )

Change subject: IMPALA-6970: race with decreasing scanner reservation
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/10314/2/be/src/runtime/bufferpool/buffer-pool-test.cc
File be/src/runtime/bufferpool/buffer-pool-test.cc:

http://gerrit.cloudera.org:8080/#/c/10314/2/be/src/runtime/bufferpool/buffer-pool-test.cc@2158
PS2, Line 2158: thread_group
nit: maybe just use 'thread' if we need only one


http://gerrit.cloudera.org:8080/#/c/10314/2/be/src/runtime/bufferpool/buffer-pool.h
File be/src/runtime/bufferpool/buffer-pool.h:

http://gerrit.cloudera.org:8080/#/c/10314/2/be/src/runtime/bufferpool/buffer-pool.h@349
PS2, Line 349: the postcondition
 :   /// does not hold if called concurrently with a 
DecreaseReservation*() call.
do you mean that the post-condition of "after which 'bytes' can be used" does 
not hold in this case?

if yes, wont this method be confusing to use if its not serving its purpose 
after returning an OK status.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I653225c981bf674d2b2b947f3a3cb4d8f382d36b
Gerrit-Change-Number: 10314
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 07 May 2018 22:19:29 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6974: Use CMAKE POSITION INDEPENDENT CODE in backend

2018-05-07 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10267 )

Change subject: IMPALA-6974: Use CMAKE_POSITION_INDEPENDENT_CODE in backend
..


Patch Set 5: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id37bb5afa6a9b7909bb4efe1390a67f7d1469544
Gerrit-Change-Number: 10267
Gerrit-PatchSet: 5
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 07 May 2018 22:17:21 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-783: [DOCS] Document SHOW CREATE VIEW

2018-05-07 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10313 )

Change subject: IMPALA-783: [DOCS] Document SHOW CREATE VIEW
..


Patch Set 1:

Fredy, any chance you could double-check the content about "SHOW CREATE VIEW" 
permissions. The text LGTM but I don't know the rules in and
 out.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia656c51d41fe7dbf08803313effc0baae680fce0
Gerrit-Change-Number: 10313
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 07 May 2018 22:16:39 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-783: [DOCS] Document SHOW CREATE VIEW

2018-05-07 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10313 )

Change subject: IMPALA-783: [DOCS] Document SHOW CREATE VIEW
..


Patch Set 1: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia656c51d41fe7dbf08803313effc0baae680fce0
Gerrit-Change-Number: 10313
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 07 May 2018 22:16:02 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-4850: [DOCS] COMMENT should come after PARTITIONED BY

2018-05-07 Thread Joe McDonnell (Code Review)
Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10316 )

Change subject: IMPALA-4850: [DOCS] COMMENT should come after PARTITIONED BY
..


Patch Set 1:

(1 comment)

Thanks for working on this. I noticed some other problems with the ordering, so 
I think it would be good to fix these at the same time.

http://gerrit.cloudera.org:8080/#/c/10316/1/docs/topics/impala_create_table.xml
File docs/topics/impala_create_table.xml:

http://gerrit.cloudera.org:8080/#/c/10316/1/docs/topics/impala_create_table.xml@172
PS1, Line 172:   [COMMENT 'table_comment']
This bug is from a long time ago. In reviewing this, I noticed that there are 
several other preexisting problems with our ordering here. Here is the order 
that it should be:
PARTITIONED BY
SORT BY
COMMENT
ROW FORMAT
WITH SERDEPROPERTIES
STORED AS
LOCATION
CACHED IN, etc
TBLPROPERTIES

This ordering is the same across CREATE TABLE, CREATE TABLE AS SELECT, CREATE 
TABLE LIKE {file format}. So, there are multiple locations on this page that 
are incorrect.

CREATE TABLE LIKE {table} is different, so this doesn't apply to that.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I03fd4a308981955bb52ca79772fe2f7c01b5894f
Gerrit-Change-Number: 10316
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Comment-Date: Mon, 07 May 2018 22:08:18 +
Gerrit-HasComments: Yes


[Impala-ASF-CR](asf-site) [DOCS] Impala doc site update for 3.0

2018-05-07 Thread Alex Rodoni (Code Review)
Alex Rodoni has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10322 )

Change subject: [DOCS] Impala doc site update for 3.0
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10322/1/docs/build/html/topics/impala_known_issues.html
File docs/build/html/topics/impala_known_issues.html:

http://gerrit.cloudera.org:8080/#/c/10322/1/docs/build/html/topics/impala_known_issues.html@36
PS1, Line 36: div>
:
:Is this a defect that exists in master?
Yes, and I committed the fix.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf5927efa7baa965095a3ff2fd4ec7411313342d
Gerrit-Change-Number: 10322
Gerrit-PatchSet: 4
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Mon, 07 May 2018 22:07:45 +
Gerrit-HasComments: Yes


[Impala-ASF-CR](asf-site) [DOCS] Impala doc site update for 3.0

2018-05-07 Thread Michael Brown (Code Review)
Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10322 )

Change subject: [DOCS] Impala doc site update for 3.0
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10322/1/docs/build/html/topics/impala_known_issues.html
File docs/build/html/topics/impala_known_issues.html:

http://gerrit.cloudera.org:8080/#/c/10322/1/docs/build/html/topics/impala_known_issues.html@36
PS1, Line 36: 
:   c
: 
> Done
Is this a defect that exists in master?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf5927efa7baa965095a3ff2fd4ec7411313342d
Gerrit-Change-Number: 10322
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Mon, 07 May 2018 22:02:04 +
Gerrit-HasComments: Yes


[Impala-ASF-CR](asf-site) [DOCS] Impala doc site update for 3.0

2018-05-07 Thread Alex Rodoni (Code Review)
Alex Rodoni has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10322 )

Change subject: [DOCS] Impala doc site update for 3.0
..


Patch Set 3:

> (2 comments)

Removed the unused files.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf5927efa7baa965095a3ff2fd4ec7411313342d
Gerrit-Change-Number: 10322
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Mon, 07 May 2018 21:55:23 +
Gerrit-HasComments: No


[Impala-ASF-CR](asf-site) [DOCS] Impala doc site update for 3.0

2018-05-07 Thread Alex Rodoni (Code Review)
Hello Michael Brown, Ambreen Kazi, Sailesh Mukil,

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

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

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

Change subject: [DOCS] Impala doc site update for 3.0
..

[DOCS] Impala doc site update for 3.0

Add Impala docs from branch master.
Commit hash: f20415755401d56103df7f16348cea8ed12fb3d8

Change-Id: Icf5927efa7baa965095a3ff2fd4ec7411313342d
---
M docs/build/html/index.html
D docs/build/html/topics/impala_abort_on_default_limit_exceeded.html
M docs/build/html/topics/impala_abort_on_error.html
M docs/build/html/topics/impala_adls.html
M docs/build/html/topics/impala_admin.html
M docs/build/html/topics/impala_admission.html
M docs/build/html/topics/impala_aggregate_functions.html
M docs/build/html/topics/impala_aliases.html
M docs/build/html/topics/impala_allow_unsupported_formats.html
M docs/build/html/topics/impala_alter_table.html
M docs/build/html/topics/impala_alter_view.html
M docs/build/html/topics/impala_analytic_functions.html
M docs/build/html/topics/impala_appx_count_distinct.html
M docs/build/html/topics/impala_appx_median.html
M docs/build/html/topics/impala_array.html
M docs/build/html/topics/impala_auditing.html
M docs/build/html/topics/impala_authentication.html
M docs/build/html/topics/impala_authorization.html
M docs/build/html/topics/impala_avg.html
M docs/build/html/topics/impala_avro.html
M docs/build/html/topics/impala_batch_size.html
M docs/build/html/topics/impala_bigint.html
M docs/build/html/topics/impala_bit_functions.html
M docs/build/html/topics/impala_boolean.html
M docs/build/html/topics/impala_breakpad.html
M docs/build/html/topics/impala_buffer_pool_limit.html
M docs/build/html/topics/impala_char.html
M docs/build/html/topics/impala_comments.html
M docs/build/html/topics/impala_complex_types.html
M docs/build/html/topics/impala_components.html
M docs/build/html/topics/impala_compression_codec.html
M docs/build/html/topics/impala_compute_stats.html
M docs/build/html/topics/impala_compute_stats_min_sample_size.html
M docs/build/html/topics/impala_concepts.html
M docs/build/html/topics/impala_conditional_functions.html
M docs/build/html/topics/impala_config.html
M docs/build/html/topics/impala_config_options.html
M docs/build/html/topics/impala_config_performance.html
M docs/build/html/topics/impala_connecting.html
M docs/build/html/topics/impala_conversion_functions.html
M docs/build/html/topics/impala_count.html
M docs/build/html/topics/impala_create_database.html
M docs/build/html/topics/impala_create_function.html
M docs/build/html/topics/impala_create_role.html
M docs/build/html/topics/impala_create_table.html
M docs/build/html/topics/impala_create_view.html
M docs/build/html/topics/impala_databases.html
M docs/build/html/topics/impala_datatypes.html
M docs/build/html/topics/impala_datetime_functions.html
M docs/build/html/topics/impala_ddl.html
M docs/build/html/topics/impala_debug_action.html
M docs/build/html/topics/impala_decimal.html
M docs/build/html/topics/impala_decimal_v2.html
M docs/build/html/topics/impala_default_join_distribution_mode.html
D docs/build/html/topics/impala_default_order_by_limit.html
M docs/build/html/topics/impala_default_spillable_buffer_size.html
M docs/build/html/topics/impala_delegation.html
M docs/build/html/topics/impala_delete.html
M docs/build/html/topics/impala_describe.html
M docs/build/html/topics/impala_development.html
M docs/build/html/topics/impala_disable_codegen.html
M docs/build/html/topics/impala_disable_row_runtime_filtering.html
M docs/build/html/topics/impala_disable_streaming_preaggregations.html
M docs/build/html/topics/impala_disable_unsafe_spills.html
M docs/build/html/topics/impala_disk_space.html
M docs/build/html/topics/impala_distinct.html
M docs/build/html/topics/impala_dml.html
M docs/build/html/topics/impala_double.html
M docs/build/html/topics/impala_drop_database.html
M docs/build/html/topics/impala_drop_function.html
M docs/build/html/topics/impala_drop_role.html
M docs/build/html/topics/impala_drop_stats.html
M docs/build/html/topics/impala_drop_table.html
M docs/build/html/topics/impala_drop_view.html
M docs/build/html/topics/impala_exec_single_node_rows_threshold.html
M docs/build/html/topics/impala_exec_time_limit_s.html
M docs/build/html/topics/impala_explain.html
M docs/build/html/topics/impala_explain_level.html
M docs/build/html/topics/impala_explain_plan.html
M docs/build/html/topics/impala_faq.html
M docs/build/html/topics/impala_file_formats.html
M docs/build/html/topics/impala_fixed_issues.html
M docs/build/html/topics/impala_float.html
M docs/build/html/topics/impala_functions.html
M docs/build/html/topics/impala_functions_overview.html
M docs/build/html/topics/impala_grant.html
M docs/build/html/topics/impala_group_by.html
M docs/build/html/topics/impala_group_concat.html
M docs/build/html/topics/impala_hadoop.html
M 

[Impala-ASF-CR] IMPALA-6946: handle negative counts in RLE decoder

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

Change subject: IMPALA-6946: handle negative counts in RLE decoder
..


Patch Set 7: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If75ef3fb12494209918c100f26407cd93b17addb
Gerrit-Change-Number: 10233
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 07 May 2018 21:50:55 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6946: handle negative counts in RLE decoder

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

Change subject: IMPALA-6946: handle negative counts in RLE decoder
..

IMPALA-6946: handle negative counts in RLE decoder

This improves the handling of out-of-range values to avoid hitting various
DCHECKs, including the one in the JIRA. repeat_count_ and literal_count_
are int32_ts. Avoid setting them to a negative value directly or by
integer overflow.

Switch to using uint32_t for "VLQ" encoding, which should be ULEB-128
encoding according to the Parquet standard. This fixes an infinite loop
in PutVlqInt() for negative values - the bug was that shifting right
sign-extended the signed value.

Testing:
Added backend test to exercise handling of large literal and repeat
counts that don't fit in an int32_t. Before these fixes it could trigger
several DCHECKs.

Change-Id: If75ef3fb12494209918c100f26407cd93b17addb
Reviewed-on: http://gerrit.cloudera.org:8080/10233
Reviewed-by: Lars Volker 
Reviewed-by: Dan Hecht 
Tested-by: Impala Public Jenkins 
---
M be/src/util/bit-stream-utils.h
M be/src/util/bit-stream-utils.inline.h
M be/src/util/dict-encoding.h
M be/src/util/rle-encoding.h
M be/src/util/rle-test.cc
5 files changed, 90 insertions(+), 39 deletions(-)

Approvals:
  Lars Volker: Looks good to me, but someone else must approve
  Dan Hecht: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: If75ef3fb12494209918c100f26407cd93b17addb
Gerrit-Change-Number: 10233
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] PREVIEW - IMPALA-6957: calc thread resource requirement in planner

2018-05-07 Thread Dan Hecht (Code Review)
Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10256 )

Change subject: PREVIEW - IMPALA-6957: calc thread resource requirement in 
planner
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10256/4/testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test
File 
testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test:

http://gerrit.cloudera.org:8080/#/c/10256/4/testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test@10
PS4, Line 10: threads
maybe at this level (and below) it should be thread-reservation, just like for 
memory? Since this isn't necessarily the number of threads we'll use, but the 
number we'll additionally require (i.e. reserve).



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I982837ef883457fa4d2adc3bdbdc727353469140
Gerrit-Change-Number: 10256
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 07 May 2018 21:36:37 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] PREVIEW - IMPALA-6957: calc thread resource requirement in planner

2018-05-07 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10256 )

Change subject: PREVIEW - IMPALA-6957: calc thread resource requirement in 
planner
..


Patch Set 4:

(1 comment)

I think we would want to allow rejection based on the per-host value (I'm not 
sure if that would use an upper bound or the actual max based on the schedule - 
maybe the first would be more robust), and also based on the global aggregate 
across fragments. One would protect any single daemon from resource exhaustive, 
the other would be more useful to protect a cluster from being overwhelmed.

http://gerrit.cloudera.org:8080/#/c/10256/4/testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test
File 
testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test:

http://gerrit.cloudera.org:8080/#/c/10256/4/testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test@6
PS4, Line 6: Max Per-Host Resource Reservation: Memory=18.97MB Threads=5
> what does the "Max" here refer to?
This is the maximum possible if all fragment instances were scheduled on a 
single backend.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I982837ef883457fa4d2adc3bdbdc727353469140
Gerrit-Change-Number: 10256
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 07 May 2018 21:17:45 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5216: Make admission control queuing async

2018-05-07 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10060 )

Change subject: IMPALA-5216: Make admission control queuing async
..


Patch Set 7:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/10060/7/be/src/service/client-request-state.cc
File be/src/service/client-request-state.cc:

http://gerrit.cloudera.org:8080/#/c/10060/7/be/src/service/client-request-state.cc@734
PS7, Line 734:
 :   && TOperationState::INITIALIZED_STATE < 
operation_state)
Ahh I missed that this was implied by the DCHECK on my first pass.

I think this code would be easier to understand if it didn't try to be cute 
with the inequality of enums. I couldn't understand this code without pulling 
up TCLIService.thrift and looking at them side-by-side, which is a bad sign.

Can we just make the valid state transitions explicit? I don't think there are 
actually that many distinct transitions to consider. Maybe something like:

  switch (operation_state_) {
case INITIALIZED_STATE:
case PENDING_STATE:
  UpdateOperationState();
  break;
case RUNNING_STATE:
  if (operation_state == FINISHED_STATE) UpdateOperationState();
  break;
default:
  // ..
  break;
  }


http://gerrit.cloudera.org:8080/#/c/10060/7/tests/hs2/test_hs2.py
File tests/hs2/test_hs2.py:

http://gerrit.cloudera.org:8080/#/c/10060/7/tests/hs2/test_hs2.py@468
PS7, Line 468: # ExecSummary.
We should also test getting the exec summary after the query was admitted but 
before it was closed.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I989cf5b259afb8f5bc5c35590c94961c81ce88bf
Gerrit-Change-Number: 10060
Gerrit-PatchSet: 7
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 07 May 2018 21:13:53 +
Gerrit-HasComments: Yes


[Impala-ASF-CR](asf-site) Added changelog for 3.0.0

2018-05-07 Thread Sailesh Mukil (Code Review)
Sailesh Mukil has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/10334 )

Change subject: Added changelog for 3.0.0
..

Added changelog for 3.0.0

Change-Id: I895f54fc47bada6b5131f20167405781c7d284b5
---
A docs/changelog-3.0.html
1 file changed, 410 insertions(+), 0 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I895f54fc47bada6b5131f20167405781c7d284b5
Gerrit-Change-Number: 10334
Gerrit-PatchSet: 2
Gerrit-Owner: Sailesh Mukil 


[Impala-ASF-CR](asf-site) Update download and signature links for 3.0.0 release.

2018-05-07 Thread Sailesh Mukil (Code Review)
Sailesh Mukil has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10333


Change subject: Update download and signature links for 3.0.0 release.
..

Update download and signature links for 3.0.0 release.

Change-Id: I307d786bf4709d24f1a9511dcc0e6759183e8eae
---
M downloads.html
1 file changed, 26 insertions(+), 15 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-MessageType: newchange
Gerrit-Change-Id: I307d786bf4709d24f1a9511dcc0e6759183e8eae
Gerrit-Change-Number: 10333
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil 


[Impala-ASF-CR](asf-site) Added changelog for 2.12.0

2018-05-07 Thread Sailesh Mukil (Code Review)
Sailesh Mukil has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10334


Change subject: Added changelog for 2.12.0
..

Added changelog for 2.12.0

Change-Id: I895f54fc47bada6b5131f20167405781c7d284b5
---
A docs/changelog-3.0.html
1 file changed, 410 insertions(+), 0 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-MessageType: newchange
Gerrit-Change-Id: I895f54fc47bada6b5131f20167405781c7d284b5
Gerrit-Change-Number: 10334
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil 


[Impala-ASF-CR] IMPALA-4850: [DOCS] COMMENT should come after PARTITIONED BY

2018-05-07 Thread Alex Rodoni (Code Review)
Alex Rodoni has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10316 )

Change subject: IMPALA-4850: [DOCS] COMMENT should come after PARTITIONED BY
..


Patch Set 1:

Hi Joe,

Have you had a chance to review this short update?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I03fd4a308981955bb52ca79772fe2f7c01b5894f
Gerrit-Change-Number: 10316
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Comment-Date: Mon, 07 May 2018 21:09:09 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-783: [DOCS] Document SHOW CREATE VIEW

2018-05-07 Thread Alex Rodoni (Code Review)
Alex Rodoni has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10313 )

Change subject: IMPALA-783: [DOCS] Document SHOW CREATE VIEW
..


Patch Set 1:

Hi Tim,
Have you had a chance to review this short change?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia656c51d41fe7dbf08803313effc0baae680fce0
Gerrit-Change-Number: 10313
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 07 May 2018 21:08:27 +
Gerrit-HasComments: No


[Impala-ASF-CR] impala-6233: [DOCS] Documented the COMMENT clause for CREATE VIEW

2018-05-07 Thread Alex Rodoni (Code Review)
Alex Rodoni has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10312 )

Change subject: impala-6233: [DOCS] Documented the COMMENT clause for CREATE 
VIEW
..


Patch Set 2:

Have you had a chance to review this short change?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I176d525925c8dc5c5b83612da43b349049764d2b
Gerrit-Change-Number: 10312
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 07 May 2018 21:07:46 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6227: reduce window of metric inconsistency

2018-05-07 Thread Tim Armstrong (Code Review)
Tim Armstrong has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/10330 )

Change subject: IMPALA-6227: reduce window of metric inconsistency
..

IMPALA-6227: reduce window of metric inconsistency

The admission controller test fetches multiple metrics relating to the
admission controller. Before this patch it fetched the whole metrics
list for each metric, meaning there was a substantial window for
the metrics to be inconsistent for a single backend. Now the metrics are
only fetched once. Metric updates are not transactional so there is
still a small window for raciness if an admission decision is made
exactly when the metrics are fetched.

Also try to detect the specific race between updating "dequeued"
and "admitted" that we saw in practice, since the race is still
possible with a smaller window. In that case we retry getting
the metrics.

Change-Id: I2f16edbec53e49446c4c37ef5f926eedb5604319
---
M bin/start-impala-cluster.py
M tests/common/impala_service.py
M tests/custom_cluster/test_admission_controller.py
3 files changed, 34 insertions(+), 8 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2f16edbec53e49446c4c37ef5f926eedb5604319
Gerrit-Change-Number: 10330
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 


[Impala-ASF-CR] IMPALA-6941: load more text scanner compression plugins

2018-05-07 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10165 )

Change subject: IMPALA-6941: load more text scanner compression plugins
..


Patch Set 10: Code-Review+2

carry +2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If2a9c4a4a11bed81df706e9e834400bfedfe48e6
Gerrit-Change-Number: 10165
Gerrit-PatchSet: 10
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 07 May 2018 20:56:00 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6941: load more text scanner compression plugins

2018-05-07 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10165 )

Change subject: IMPALA-6941: load more text scanner compression plugins
..


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10165/8/be/src/exec/hdfs-text-scanner.h
File be/src/exec/hdfs-text-scanner.h:

http://gerrit.cloudera.org:8080/#/c/10165/8/be/src/exec/hdfs-text-scanner.h@70
PS8, Line 70: has_builtin_support
> should probably be HasBuiltinSupport()
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If2a9c4a4a11bed81df706e9e834400bfedfe48e6
Gerrit-Change-Number: 10165
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 07 May 2018 20:55:54 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] PREVIEW - IMPALA-6957: calc thread resource requirement in planner

2018-05-07 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10256 )

Change subject: PREVIEW - IMPALA-6957: calc thread resource requirement in 
planner
..


Patch Set 4:

I'm interested in your thoughts about the threads metric and how we're showing 
it in the explain plans, etc.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I982837ef883457fa4d2adc3bdbdc727353469140
Gerrit-Change-Number: 10256
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 07 May 2018 20:20:01 +
Gerrit-HasComments: No


[Impala-ASF-CR] PREVIEW - IMPALA-6957: calc thread resource requirement in planner

2018-05-07 Thread Tim Armstrong (Code Review)
Tim Armstrong has uploaded a new patch set (#4). ( 
http://gerrit.cloudera.org:8080/10256 )

Change subject: PREVIEW - IMPALA-6957: calc thread resource requirement in 
planner
..

PREVIEW - IMPALA-6957: calc thread resource requirement in planner

This only factors in fragment execution threads. E.g. this does *not*
try to account for the number of threads on the old Thrift RPC
code path if that is enabled.

This is loosely related to the old VCores estimate, but is different in
that it:
* Directly ties into the notion of required threads in
  ThreadResourceMgr.
* Is a strict upper bound on the number of such threads, rather than
  an estimate.

Does not include "optional" threads. ThreadResourceMgr in the backend
bounds the number of "optional" threads per impalad, so the number of
execution threads on a backend is limited by

  sum(required threads per query) +
  CpuInfo::num_cores() * FLAGS_num_threads_per_core

DCHECKS in the backend enforce that the calculation is correct. They
were actually hit in KuduScanNode because of some races in thread
management leading to multiple "required" threads running. Now the
first thread in the multithreaded scans never exits, which means
that it's always safe for any of the other threads to exit early,
which simplifies the logic a lot.

Testing:
Updated planner tests.

Ran core tests.

Change-Id: I982837ef883457fa4d2adc3bdbdc727353469140
---
M be/src/exec/hdfs-scan-node.cc
M be/src/exec/hdfs-scan-node.h
M be/src/exec/kudu-scan-node.cc
M be/src/exec/kudu-scan-node.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/thread-resource-mgr.cc
M be/src/runtime/thread-resource-mgr.h
M common/thrift/Frontend.thrift
M common/thrift/Planner.thrift
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/PlanFragment.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/ResourceProfile.java
M fe/src/main/java/org/apache/impala/planner/ResourceProfileBuilder.java
M 
testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/disable-codegen.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/fk-pk-join-detection.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test
M testdata/workloads/functional-planner/queries/PlannerTest/max-row-size.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/min-max-runtime-filters.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/partition-pruning.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/sort-expr-materialization.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test
M testdata/workloads/functional-planner/queries/PlannerTest/tablesample.test
M testdata/workloads/functional-planner/queries/PlannerTest/union.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level0.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level1.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level2.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level3.test
33 files changed, 1,837 insertions(+), 1,797 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I982837ef883457fa4d2adc3bdbdc727353469140
Gerrit-Change-Number: 10256
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 


[Impala-ASF-CR] Move admission-controller and catalog metrics into own groups

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

Change subject: Move admission-controller and catalog metrics into own groups
..

Move admission-controller and catalog metrics into own groups

This makes using these metrics a lot easier since they're not mixed
into the big impala-metrics group.

Testing:
Checked /metrics debug page to see that all metrics were now in a
separate section

Change-Id: I17dbbcebc01cc1f5b8e94e593873cdc3dc5e36df
Reviewed-on: http://gerrit.cloudera.org:8080/10302
Reviewed-by: Sailesh Mukil 
Reviewed-by: Philip Zeyliger 
Tested-by: Impala Public Jenkins 
---
M be/src/scheduling/admission-controller.cc
M be/src/util/impalad-metrics.cc
2 files changed, 12 insertions(+), 7 deletions(-)

Approvals:
  Sailesh Mukil: Looks good to me, but someone else must approve
  Philip Zeyliger: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I17dbbcebc01cc1f5b8e94e593873cdc3dc5e36df
Gerrit-Change-Number: 10302
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] Move admission-controller and catalog metrics into own groups

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

Change subject: Move admission-controller and catalog metrics into own groups
..


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I17dbbcebc01cc1f5b8e94e593873cdc3dc5e36df
Gerrit-Change-Number: 10302
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 07 May 2018 20:18:59 +
Gerrit-HasComments: No


[Impala-ASF-CR] test-with-docker: exit properly on failures

2018-05-07 Thread Philip Zeyliger (Code Review)
Philip Zeyliger has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10318 )

Change subject: test-with-docker: exit properly on failures
..

test-with-docker: exit properly on failures

If the build was failing, test-with-docker wouldn't recognize
it and continue with the script; this fixes that.

The bash puzzle I learned here is that

  bash -c "set -e; function f() { false; echo f; }; if f; then echo x; fi"

will print "f" and "x", despite the set -e, even if f is put into
a sub-shell with parentheses.

Change-Id: I285e2f4d07e34898d73beba857e9ac325ed4e6db
Reviewed-on: http://gerrit.cloudera.org:8080/10318
Tested-by: Philip Zeyliger 
Reviewed-by: Joe McDonnell 
---
M docker/entrypoint.sh
1 file changed, 4 insertions(+), 5 deletions(-)

Approvals:
  Philip Zeyliger: Verified
  Joe McDonnell: Looks good to me, approved

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I285e2f4d07e34898d73beba857e9ac325ed4e6db
Gerrit-Change-Number: 10318
Gerrit-PatchSet: 2
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 


[Impala-ASF-CR] test-with-docker: exit properly on failures

2018-05-07 Thread Joe McDonnell (Code Review)
Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10318 )

Change subject: test-with-docker: exit properly on failures
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I285e2f4d07e34898d73beba857e9ac325ed4e6db
Gerrit-Change-Number: 10318
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Mon, 07 May 2018 20:09:43 +
Gerrit-HasComments: No


[Impala-ASF-CR](asf-site) [DOCS] Impala doc site update for 3.0

2018-05-07 Thread Alex Rodoni (Code Review)
Alex Rodoni has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10322 )

Change subject: [DOCS] Impala doc site update for 3.0
..


Patch Set 3:

(2 comments)

> (1 comment)

http://gerrit.cloudera.org:8080/#/c/10322/1/docs/build/html/topics/impala_known_issues.html
File docs/build/html/topics/impala_known_issues.html:

http://gerrit.cloudera.org:8080/#/c/10322/1/docs/build/html/topics/impala_known_issues.html@36
PS1, Line 36: div>
:
:Is this incomplete?
Done


http://gerrit.cloudera.org:8080/#/c/10322/2/impala-docs.html
File impala-docs.html:

http://gerrit.cloudera.org:8080/#/c/10322/2/impala-docs.html@143
PS2, Line 143: PDF 
Documentation for Impala 2.12
> This doesn't point to 2.12 docs. It points to the same path as the 3.0 docs
I need to remove that line. For older releases, we only have PDF available.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf5927efa7baa965095a3ff2fd4ec7411313342d
Gerrit-Change-Number: 10322
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Mon, 07 May 2018 19:44:31 +
Gerrit-HasComments: Yes


[Impala-ASF-CR](asf-site) [DOCS] Impala doc site update for 3.0

2018-05-07 Thread Alex Rodoni (Code Review)
Hello Michael Brown, Ambreen Kazi, Sailesh Mukil,

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

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

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

Change subject: [DOCS] Impala doc site update for 3.0
..

[DOCS] Impala doc site update for 3.0

Add Impala docs from branch master.
Commit hash: f20415755401d56103df7f16348cea8ed12fb3d8

Change-Id: Icf5927efa7baa965095a3ff2fd4ec7411313342d
---
M docs/build/html/index.html
M docs/build/html/topics/impala_abort_on_error.html
M docs/build/html/topics/impala_adls.html
M docs/build/html/topics/impala_admin.html
M docs/build/html/topics/impala_admission.html
M docs/build/html/topics/impala_aggregate_functions.html
M docs/build/html/topics/impala_aliases.html
M docs/build/html/topics/impala_allow_unsupported_formats.html
M docs/build/html/topics/impala_alter_table.html
M docs/build/html/topics/impala_alter_view.html
M docs/build/html/topics/impala_analytic_functions.html
M docs/build/html/topics/impala_appx_count_distinct.html
M docs/build/html/topics/impala_appx_median.html
M docs/build/html/topics/impala_array.html
M docs/build/html/topics/impala_auditing.html
M docs/build/html/topics/impala_authentication.html
M docs/build/html/topics/impala_authorization.html
M docs/build/html/topics/impala_avg.html
M docs/build/html/topics/impala_avro.html
M docs/build/html/topics/impala_batch_size.html
M docs/build/html/topics/impala_bigint.html
M docs/build/html/topics/impala_bit_functions.html
M docs/build/html/topics/impala_boolean.html
M docs/build/html/topics/impala_breakpad.html
M docs/build/html/topics/impala_buffer_pool_limit.html
M docs/build/html/topics/impala_char.html
M docs/build/html/topics/impala_comments.html
M docs/build/html/topics/impala_complex_types.html
M docs/build/html/topics/impala_components.html
M docs/build/html/topics/impala_compression_codec.html
M docs/build/html/topics/impala_compute_stats.html
M docs/build/html/topics/impala_compute_stats_min_sample_size.html
M docs/build/html/topics/impala_concepts.html
M docs/build/html/topics/impala_conditional_functions.html
M docs/build/html/topics/impala_config.html
M docs/build/html/topics/impala_config_options.html
M docs/build/html/topics/impala_config_performance.html
M docs/build/html/topics/impala_connecting.html
M docs/build/html/topics/impala_conversion_functions.html
M docs/build/html/topics/impala_count.html
M docs/build/html/topics/impala_create_database.html
M docs/build/html/topics/impala_create_function.html
M docs/build/html/topics/impala_create_role.html
M docs/build/html/topics/impala_create_table.html
M docs/build/html/topics/impala_create_view.html
M docs/build/html/topics/impala_databases.html
M docs/build/html/topics/impala_datatypes.html
M docs/build/html/topics/impala_datetime_functions.html
M docs/build/html/topics/impala_ddl.html
M docs/build/html/topics/impala_debug_action.html
M docs/build/html/topics/impala_decimal.html
M docs/build/html/topics/impala_decimal_v2.html
M docs/build/html/topics/impala_default_join_distribution_mode.html
M docs/build/html/topics/impala_default_spillable_buffer_size.html
M docs/build/html/topics/impala_delegation.html
M docs/build/html/topics/impala_delete.html
M docs/build/html/topics/impala_describe.html
M docs/build/html/topics/impala_development.html
M docs/build/html/topics/impala_disable_codegen.html
M docs/build/html/topics/impala_disable_row_runtime_filtering.html
M docs/build/html/topics/impala_disable_streaming_preaggregations.html
M docs/build/html/topics/impala_disable_unsafe_spills.html
M docs/build/html/topics/impala_disk_space.html
M docs/build/html/topics/impala_distinct.html
M docs/build/html/topics/impala_dml.html
M docs/build/html/topics/impala_double.html
M docs/build/html/topics/impala_drop_database.html
M docs/build/html/topics/impala_drop_function.html
M docs/build/html/topics/impala_drop_role.html
M docs/build/html/topics/impala_drop_stats.html
M docs/build/html/topics/impala_drop_table.html
M docs/build/html/topics/impala_drop_view.html
M docs/build/html/topics/impala_exec_single_node_rows_threshold.html
M docs/build/html/topics/impala_exec_time_limit_s.html
M docs/build/html/topics/impala_explain.html
M docs/build/html/topics/impala_explain_level.html
M docs/build/html/topics/impala_explain_plan.html
M docs/build/html/topics/impala_faq.html
M docs/build/html/topics/impala_file_formats.html
M docs/build/html/topics/impala_fixed_issues.html
M docs/build/html/topics/impala_float.html
M docs/build/html/topics/impala_functions.html
M docs/build/html/topics/impala_functions_overview.html
M docs/build/html/topics/impala_grant.html
M docs/build/html/topics/impala_group_by.html
M docs/build/html/topics/impala_group_concat.html
M docs/build/html/topics/impala_hadoop.html
M docs/build/html/topics/impala_having.html
M docs/build/html/topics/impala_hbase.html
M docs/build/html/topics/impala_hbase_cache_blocks.html
M 

[Impala-ASF-CR] IMPALA-6948: Delete catalog update topic entries upon catalog restart

2018-05-07 Thread Vuk Ercegovac (Code Review)
Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10289 )

Change subject: IMPALA-6948: Delete catalog update topic entries upon catalog 
restart
..


Patch Set 3:

(2 comments)

looks good. one remaining question about tightening the protocol.

http://gerrit.cloudera.org:8080/#/c/10289/1/be/src/statestore/statestore.cc
File be/src/statestore/statestore.cc:

http://gerrit.cloudera.org:8080/#/c/10289/1/be/src/statestore/statestore.cc@665
PS1, Line 665: 
subscriber->SetLastTopicVersionProcessed(topic_it->first, update.from_version);
> I see what you mean. Ideally, I want to keep that operation (deleting the t
makes sense. however, if I were to test these code paths, I would add a case 
where from_version is set *and* we clear_topic_entries(). current code in the 
catalog (a subscriber) does not allow this combination afaict. main question 
from the standpoint of the protocol is whether we even want to consider this 
combination. if not, then lets document it with a dcheck.


http://gerrit.cloudera.org:8080/#/c/10289/3/be/src/statestore/statestore.cc
File be/src/statestore/statestore.cc:

http://gerrit.cloudera.org:8080/#/c/10289/3/be/src/statestore/statestore.cc@215
PS3, Line 215: -
if all is correct, the metric won't be negative. however, what happens if it 
does go negative?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I74a8ade8e498ac35cb56d3775d2c67a86988d9b6
Gerrit-Change-Number: 10289
Gerrit-PatchSet: 3
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 07 May 2018 19:22:53 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] [DOCS] Removed an extra p tag in known issues

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

Change subject: [DOCS] Removed an extra p tag in known issues
..


Patch Set 1: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieb4cf7b2ce0cd1f99420a75a0c26cd0097f1edb6
Gerrit-Change-Number: 10331
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 07 May 2018 19:21:08 +
Gerrit-HasComments: No


[Impala-ASF-CR] [DOCS] Removed an extra p tag in known issues

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

Change subject: [DOCS] Removed an extra p tag in known issues
..

[DOCS] Removed an extra p tag in known issues

Change-Id: Ieb4cf7b2ce0cd1f99420a75a0c26cd0097f1edb6
Cherry-picks: not for 2.x.
Reviewed-on: http://gerrit.cloudera.org:8080/10331
Reviewed-by: Alex Rodoni 
Tested-by: Impala Public Jenkins 
---
M docs/topics/impala_known_issues.xml
1 file changed, 0 insertions(+), 4 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ieb4cf7b2ce0cd1f99420a75a0c26cd0097f1edb6
Gerrit-Change-Number: 10331
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-6948,IMPALA-6962: add end-to-end tests

2018-05-07 Thread Dimitris Tsirogiannis (Code Review)
Dimitris Tsirogiannis has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10291 )

Change subject: IMPALA-6948,IMPALA-6962: add end-to-end tests
..


Patch Set 2:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/10291/2/tests/custom_cluster/test_metadata_replicas.py
File tests/custom_cluster/test_metadata_replicas.py:

http://gerrit.cloudera.org:8080/#/c/10291/2/tests/custom_cluster/test_metadata_replicas.py@39
PS2, Line 39: def test_load_all(self, cursor):
: """ Loads metadata for all tables and validates that they're 
all the same."""
: # Use describe to issue table loads.
: c_objects = 
self.cluster.catalogd.service.get_catalog_objects()
: for obj in c_objects:
:   if obj[1] == "TABLE": cursor.execute("describe %s" % obj[0])
: # Use a sentinel ddl with sync_ddl to make sure that all 
impalads
: # have receieved the effect of the previous ddl's.
: cursor.execute("set sync_ddl=true")
: cursor.execute("invalidate metadata functional.alltypes")
:
: self.__validate_metadata()
This test is going to be very expensive and I am not sure how much extra 
coverage it adds compared to our existing metadata tests. Also, if we decide to 
keep it, you may want to consider using the load_catalog_in_background flag to 
trigger metadata load for all tables instead of running describe for each one 
of them.


http://gerrit.cloudera.org:8080/#/c/10291/2/tests/custom_cluster/test_metadata_replicas.py@69
PS2, Line 69: cursor.execute("invalidate metadata")
No need for the global one, you may do "invalidate metadata x".


http://gerrit.cloudera.org:8080/#/c/10291/2/tests/custom_cluster/test_metadata_replicas.py@79
PS2, Line 79: wait_time_s = specific_build_type_timeout(10, 
slow_build_timeout=20)
: sleep(wait_time_s)
I agree with the TODO and I think we should fix it now. Essentially, in order 
to ensure that we are in steady state and that the coordinators have been 
through the cycle of detecting the catalog restart, asking the full update and 
receiving it, we need to compare the current value of the catalog version in 
the catalog and the last received catalog version in the coordinator (the 
latter is already a metric).


http://gerrit.cloudera.org:8080/#/c/10291/2/tests/custom_cluster/test_metadata_replicas.py@81
PS2, Line 81: self.cluster.refresh()
Hm, why is this needed?


http://gerrit.cloudera.org:8080/#/c/10291/2/tests/custom_cluster/test_metadata_replicas.py@123
PS2, Line 123: def __dump_objects(self, catalog, impalads):
 : """ For debugging, prints all objects and their version."""
 : print "CATALOG OBJECTS"
 : for k, v in catalog.items():
 :   print "%s, %s, %d" % (k, v[0], v[1])
 :
 : for idx in xrange(0,len(impalads)):
 :   print "IMPALAD %d OBJECTS" % idx
 :   for k, v in impalads[idx].items():
 : print "%s, %s, %d" % (k, v[0], v[1])
Is this used anywhere?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic6c5b39e29b2885cd30fede18833cbf23fb755f5
Gerrit-Change-Number: 10291
Gerrit-PatchSet: 2
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 07 May 2018 19:15:05 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] [DOCS] Removed an extra p tag in known issues

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

Change subject: [DOCS] Removed an extra p tag in known issues
..


Patch Set 1:

Build started: https://jenkins.impala.io/job/gerrit-docs-submit/280/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieb4cf7b2ce0cd1f99420a75a0c26cd0097f1edb6
Gerrit-Change-Number: 10331
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 07 May 2018 19:12:14 +
Gerrit-HasComments: No


[Impala-ASF-CR] [DOCS] Removed an extra p tag in known issues

2018-05-07 Thread Alex Rodoni (Code Review)
Alex Rodoni has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10331


Change subject: [DOCS] Removed an extra p tag in known issues
..

[DOCS] Removed an extra p tag in known issues

Change-Id: Ieb4cf7b2ce0cd1f99420a75a0c26cd0097f1edb6
Cherry-picks: not for 2.x.
---
M docs/topics/impala_known_issues.xml
1 file changed, 0 insertions(+), 4 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ieb4cf7b2ce0cd1f99420a75a0c26cd0097f1edb6
Gerrit-Change-Number: 10331
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 


[Impala-ASF-CR] [DOCS] Removed an extra p tag in known issues

2018-05-07 Thread Alex Rodoni (Code Review)
Alex Rodoni has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10331 )

Change subject: [DOCS] Removed an extra p tag in known issues
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieb4cf7b2ce0cd1f99420a75a0c26cd0097f1edb6
Gerrit-Change-Number: 10331
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Comment-Date: Mon, 07 May 2018 19:11:47 +
Gerrit-HasComments: No


[Impala-ASF-CR](asf-site) [DOCS] Impala doc site update for 3.0

2018-05-07 Thread Michael Brown (Code Review)
Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10322 )

Change subject: [DOCS] Impala doc site update for 3.0
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10322/2/impala-docs.html
File impala-docs.html:

http://gerrit.cloudera.org:8080/#/c/10322/2/impala-docs.html@143
PS2, Line 143: HTML 
Documentation for Impala 2.12
This doesn't point to 2.12 docs. It points to the same path as the 3.0 docs at 
L131. We need either a different tree for 2.x vs. 3.x HTML docs, or to only 
have HTML docs for 3.x latest. The diff you have here moves us toward the 
latter.

This also raises questions of procedure as it pertains to documenting future 
2.x releases. If we use the latter, then for future 2.x releases, need we only 
upload a PDF?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf5927efa7baa965095a3ff2fd4ec7411313342d
Gerrit-Change-Number: 10322
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Mon, 07 May 2018 19:10:38 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] test-with-docker: exit properly on failures

2018-05-07 Thread Philip Zeyliger (Code Review)
Philip Zeyliger has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10318 )

Change subject: test-with-docker: exit properly on failures
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10318/1/docker/entrypoint.sh
File docker/entrypoint.sh:

http://gerrit.cloudera.org:8080/#/c/10318/1/docker/entrypoint.sh@381
PS1, Line 381:   # The "| cat" here avoids "set -e"/errexit from exiting the
 :   # script right away.
 :   "${CMD}" "$@" | cat
 :   ret=${PIPESTATUS[0]}
> Let me double-check my understanding:
Yep: as soon as you're inside of an "if", set -e loses all potency. We want set 
-e to work inside of the function we're calling, so we avoid the if.

The | cat is all so that I can print line 386 and then exit.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I285e2f4d07e34898d73beba857e9ac325ed4e6db
Gerrit-Change-Number: 10318
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Mon, 07 May 2018 18:53:40 +
Gerrit-HasComments: Yes


[Impala-ASF-CR](asf-site) [DOCS] Impala doc site update for 3.0

2018-05-07 Thread Alex Rodoni (Code Review)
Hello Michael Brown, Ambreen Kazi, Sailesh Mukil,

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

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

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

Change subject: [DOCS] Impala doc site update for 3.0
..

[DOCS] Impala doc site update for 3.0

Commit hash: 1eedafed6ae147829e84c3a08a1bf54d7ab4b1fb

Change-Id: Icf5927efa7baa965095a3ff2fd4ec7411313342d
---
M docs/build/html/index.html
M docs/build/html/topics/impala_abort_on_error.html
M docs/build/html/topics/impala_adls.html
M docs/build/html/topics/impala_admin.html
M docs/build/html/topics/impala_admission.html
M docs/build/html/topics/impala_aggregate_functions.html
M docs/build/html/topics/impala_aliases.html
M docs/build/html/topics/impala_allow_unsupported_formats.html
M docs/build/html/topics/impala_alter_table.html
M docs/build/html/topics/impala_alter_view.html
M docs/build/html/topics/impala_analytic_functions.html
M docs/build/html/topics/impala_appx_count_distinct.html
M docs/build/html/topics/impala_appx_median.html
M docs/build/html/topics/impala_array.html
M docs/build/html/topics/impala_auditing.html
M docs/build/html/topics/impala_authentication.html
M docs/build/html/topics/impala_authorization.html
M docs/build/html/topics/impala_avg.html
M docs/build/html/topics/impala_avro.html
M docs/build/html/topics/impala_batch_size.html
M docs/build/html/topics/impala_bigint.html
M docs/build/html/topics/impala_bit_functions.html
M docs/build/html/topics/impala_boolean.html
M docs/build/html/topics/impala_breakpad.html
M docs/build/html/topics/impala_buffer_pool_limit.html
M docs/build/html/topics/impala_char.html
M docs/build/html/topics/impala_comments.html
M docs/build/html/topics/impala_complex_types.html
M docs/build/html/topics/impala_components.html
M docs/build/html/topics/impala_compression_codec.html
M docs/build/html/topics/impala_compute_stats.html
M docs/build/html/topics/impala_compute_stats_min_sample_size.html
M docs/build/html/topics/impala_concepts.html
M docs/build/html/topics/impala_conditional_functions.html
M docs/build/html/topics/impala_config.html
M docs/build/html/topics/impala_config_options.html
M docs/build/html/topics/impala_config_performance.html
M docs/build/html/topics/impala_connecting.html
M docs/build/html/topics/impala_conversion_functions.html
M docs/build/html/topics/impala_count.html
M docs/build/html/topics/impala_create_database.html
M docs/build/html/topics/impala_create_function.html
M docs/build/html/topics/impala_create_role.html
M docs/build/html/topics/impala_create_table.html
M docs/build/html/topics/impala_create_view.html
M docs/build/html/topics/impala_databases.html
M docs/build/html/topics/impala_datatypes.html
M docs/build/html/topics/impala_datetime_functions.html
M docs/build/html/topics/impala_ddl.html
M docs/build/html/topics/impala_debug_action.html
M docs/build/html/topics/impala_decimal.html
M docs/build/html/topics/impala_decimal_v2.html
M docs/build/html/topics/impala_default_join_distribution_mode.html
M docs/build/html/topics/impala_default_spillable_buffer_size.html
M docs/build/html/topics/impala_delegation.html
M docs/build/html/topics/impala_delete.html
M docs/build/html/topics/impala_describe.html
M docs/build/html/topics/impala_development.html
M docs/build/html/topics/impala_disable_codegen.html
M docs/build/html/topics/impala_disable_row_runtime_filtering.html
M docs/build/html/topics/impala_disable_streaming_preaggregations.html
M docs/build/html/topics/impala_disable_unsafe_spills.html
M docs/build/html/topics/impala_disk_space.html
M docs/build/html/topics/impala_distinct.html
M docs/build/html/topics/impala_dml.html
M docs/build/html/topics/impala_double.html
M docs/build/html/topics/impala_drop_database.html
M docs/build/html/topics/impala_drop_function.html
M docs/build/html/topics/impala_drop_role.html
M docs/build/html/topics/impala_drop_stats.html
M docs/build/html/topics/impala_drop_table.html
M docs/build/html/topics/impala_drop_view.html
M docs/build/html/topics/impala_exec_single_node_rows_threshold.html
M docs/build/html/topics/impala_exec_time_limit_s.html
M docs/build/html/topics/impala_explain.html
M docs/build/html/topics/impala_explain_level.html
M docs/build/html/topics/impala_explain_plan.html
M docs/build/html/topics/impala_faq.html
M docs/build/html/topics/impala_file_formats.html
M docs/build/html/topics/impala_fixed_issues.html
M docs/build/html/topics/impala_float.html
M docs/build/html/topics/impala_functions.html
M docs/build/html/topics/impala_functions_overview.html
M docs/build/html/topics/impala_grant.html
M docs/build/html/topics/impala_group_by.html
M docs/build/html/topics/impala_group_concat.html
M docs/build/html/topics/impala_hadoop.html
M docs/build/html/topics/impala_having.html
M docs/build/html/topics/impala_hbase.html
M docs/build/html/topics/impala_hbase_cache_blocks.html
M docs/build/html/topics/impala_hbase_caching.html
M 

[Impala-ASF-CR] IMPALA-6966: sort table memory by size in catalogd web UI

2018-05-07 Thread Dimitris Tsirogiannis (Code Review)
Dimitris Tsirogiannis has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10292 )

Change subject: IMPALA-6966: sort table memory by size in catalogd web UI
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I60dc253f862f5fde6fa96147f114d8765bb31a85
Gerrit-Change-Number: 10292
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Comment-Date: Mon, 07 May 2018 18:42:12 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6948: Delete catalog update topic entries upon catalog restart

2018-05-07 Thread Dimitris Tsirogiannis (Code Review)
Hello Alex Behm, Vuk Ercegovac,

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

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

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

Change subject: IMPALA-6948: Delete catalog update topic entries upon catalog 
restart
..

IMPALA-6948: Delete catalog update topic entries upon catalog restart

This commit fixes an issue where the statestore may end up with stale
entries in the catalog update topic that do not correspond to the
catalog objects stored in the catalog. This may occur if the catalog
server restarts and some catalog object (e.g. table) that was known to
the catalog before the restart no longer exists in the Hive Metastore
after the restart.

Fix:
The first update for the catalog update topic that is sent by the catalog
instructs the statestore to clear any entries it may have on this topic
before applying the first update. In that way, we guarantee that the
statestore entries are consistent with the catalog objects stored in the
catalog server. Any coordinator that detects the catalog restart will
receive from the statestore a full topic update that reflects the state
of the catalog server.

Testing:
Added statestore test.

Change-Id: I907509bf92da631ece5efd23c275a613ead00e91

Tmp

Change-Id: I74a8ade8e498ac35cb56d3775d2c67a86988d9b6
---
M be/src/catalog/catalog-server.cc
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
M common/thrift/StatestoreService.thrift
M fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
M tests/statestore/test_statestore.py
6 files changed, 91 insertions(+), 7 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I74a8ade8e498ac35cb56d3775d2c67a86988d9b6
Gerrit-Change-Number: 10289
Gerrit-PatchSet: 3
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Vuk Ercegovac 


[Impala-ASF-CR] IMPALA-6948: Delete catalog update topic entries upon catalog restart

2018-05-07 Thread Dimitris Tsirogiannis (Code Review)
Dimitris Tsirogiannis has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10289 )

Change subject: IMPALA-6948: Delete catalog update topic entries upon catalog 
restart
..


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/10289/1/be/src/statestore/statestore.cc
File be/src/statestore/statestore.cc:

http://gerrit.cloudera.org:8080/#/c/10289/1/be/src/statestore/statestore.cc@213
PS1, Line 213:   topic_update_log_.clear();
> makes sense to not update. pls add a comment, perhaps in the header, to sta
Done


http://gerrit.cloudera.org:8080/#/c/10289/1/be/src/statestore/statestore.cc@665
PS1, Line 665: 
subscriber->SetLastTopicVersionProcessed(topic_it->first, update.from_version);
> basically asking if from_version can be set here and would that worth check
I see what you mean. Ideally, I want to keep that operation (deleting the topic 
entries) separate from managing the subscriber state. Makes sense?


http://gerrit.cloudera.org:8080/#/c/10289/2/be/src/statestore/statestore.cc
File be/src/statestore/statestore.cc:

http://gerrit.cloudera.org:8080/#/c/10289/2/be/src/statestore/statestore.cc@216
PS2, Line 216: key_size_metric_->SetValue(0);
 :   topic_size_metric_->SetValue(0);
 :   value_size_metric_->SetValue(0);
> these are shared across topics, so this should more accurately subtract wha
Correct. Fixed it.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I74a8ade8e498ac35cb56d3775d2c67a86988d9b6
Gerrit-Change-Number: 10289
Gerrit-PatchSet: 2
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 07 May 2018 18:37:30 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] test-with-docker: exit properly on failures

2018-05-07 Thread Joe McDonnell (Code Review)
Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10318 )

Change subject: test-with-docker: exit properly on failures
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10318/1/docker/entrypoint.sh
File docker/entrypoint.sh:

http://gerrit.cloudera.org:8080/#/c/10318/1/docker/entrypoint.sh@381
PS1, Line 381:   # The "| cat" here avoids "set -e"/errexit from exiting the
 :   # script right away.
 :   "${CMD}" "$@" | cat
 :   ret=${PIPESTATUS[0]}
Let me double-check my understanding:
The "set -e" didn't work with the CMD inside an if.
When it is not inside the if, "set -e" does what we want and causes the CMD 
function to exit if any command in the function evaluates false.
However, we don't want it exiting main() at that point, hence the cat + 
PIPESTATUS.
Is this right?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I285e2f4d07e34898d73beba857e9ac325ed4e6db
Gerrit-Change-Number: 10318
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Mon, 07 May 2018 18:37:51 +
Gerrit-HasComments: Yes


[Impala-ASF-CR](asf-site) [DOCS] Impala doc site update for 3.0

2018-05-07 Thread Ambreen Kazi (Code Review)
Ambreen Kazi has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10322 )

Change subject: [DOCS] Impala doc site update for 3.0
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10322/1/docs/build/html/topics/impala_known_issues.html
File docs/build/html/topics/impala_known_issues.html:

http://gerrit.cloudera.org:8080/#/c/10322/1/docs/build/html/topics/impala_known_issues.html@36
PS1, Line 36: 
:   c
: 
Is this incomplete?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf5927efa7baa965095a3ff2fd4ec7411313342d
Gerrit-Change-Number: 10322
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Mon, 07 May 2018 18:37:15 +
Gerrit-HasComments: Yes


[Impala-ASF-CR](2.x) IMPALA-6908: IsConnResetTException() should include ECONNRESET

2018-05-07 Thread Dan Hecht (Code Review)
Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10265 )

Change subject: IMPALA-6908: IsConnResetTException() should include ECONNRESET
..


Patch Set 3: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10265/3/be/src/rpc/thrift-util.cc
File be/src/rpc/thrift-util.cc:

http://gerrit.cloudera.org:8080/#/c/10265/3/be/src/rpc/thrift-util.cc@178
PS3, Line 178: IsSendFailTException
is that talking about IsConnResetTException?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I1bb997a833917e5166c9ca192da219f50f4439e2
Gerrit-Change-Number: 10265
Gerrit-PatchSet: 3
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Mon, 07 May 2018 18:35:48 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6946: handle negative counts in RLE decoder

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

Change subject: IMPALA-6946: handle negative counts in RLE decoder
..


Patch Set 7:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2419/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If75ef3fb12494209918c100f26407cd93b17addb
Gerrit-Change-Number: 10233
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 07 May 2018 18:11:12 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6907: Close stale connections to removed cluster members

2018-05-07 Thread Michael Ho (Code Review)
Michael Ho has uploaded a new patch set (#3). ( 
http://gerrit.cloudera.org:8080/10327 )

Change subject: IMPALA-6907: Close stale connections to removed cluster members
..

IMPALA-6907: Close stale connections to removed cluster members

Previously, ImpalaServer::MembershipCallback() is used by each
Impala backend node to update cluster membership. It also removes
stale connections to nodes which are no longer members of the cluster.
However, the way it detects removed member is flawed as it relies
on query_locations_ to determine whether stale connections may
exist to the removed members. query_locations_ is a map of host
name to a set of queries running on that host. A entry for a remote
node only exists in query_locations_ if an Impalad node has acted
as coordinator of a query with fragment instances scheduled to run
on that remote node.

This change fixes this problem by closing connections to remote
hosts which are removed from the cluster regardless of whether
it can be found in query_locations_. Also added a new test which
exercises this path by restarting Impalad backend nodes between
queries.

Change-Id: I41b7297cf665bf291b09b23524d19b1d10ab281d
---
M be/src/service/impala-server.cc
M tests/common/impala_cluster.py
M tests/custom_cluster/test_restart_services.py
3 files changed, 56 insertions(+), 35 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I41b7297cf665bf291b09b23524d19b1d10ab281d
Gerrit-Change-Number: 10327
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Ho 


[Impala-ASF-CR] IMPALA-5216: Make admission control queuing async

2018-05-07 Thread Bikramjeet Vig (Code Review)
Hello Tim Armstrong, Dan Hecht,

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

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

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

Change subject: IMPALA-5216: Make admission control queuing async
..

IMPALA-5216: Make admission control queuing async

Implement asynchronous admission control queuing. This is achieved by
running the admission control code-path in a separate thread. Major
changes include: propagating cancellation to the admission control
thread and dequeuing thread, and adding a new Query Operation State
called "PENDING" that represents the state between completion of
planning and starting of query execution.

Testing:
- Added a deterministic end to end test
- Ran multiple stress tests successfully with a cancellation probability
of 60% and with different values for the following parameters:
max_requests, queue_wait_timeout_ms. Ensured that the impalad was in a
valid state afterwards (no orphan fragments or wrong metrics).
- Ran all exhaustive tests and ASAN core tests successfully.

TODO: change terminology of "in_flight_query" to "submitted_queries".
  Need to identify all references of this terminology, eg. in
  comments, tests, variable names, etc.

Change-Id: I989cf5b259afb8f5bc5c35590c94961c81ce88bf
---
M be/src/common/atomic.h
M be/src/common/logging.h
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
M be/src/scheduling/query-schedule.cc
M be/src/scheduling/query-schedule.h
M be/src/service/client-request-state.cc
M be/src/service/client-request-state.h
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-http-handler.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/util/debug-util.cc
M be/src/util/debug-util.h
M be/src/util/promise-test.cc
M be/src/util/promise.h
M common/thrift/ImpalaService.thrift
M testdata/workloads/functional-query/queries/QueryTest/runtime_row_filters.test
M tests/authorization/test_authorization.py
M tests/beeswax/impala_beeswax.py
M tests/common/impala_connection.py
M tests/custom_cluster/test_admission_controller.py
M tests/custom_cluster/test_krpc_mem_usage.py
M tests/hs2/hs2_test_suite.py
M tests/hs2/test_hs2.py
M tests/query_test/test_observability.py
M www/query_backends.tmpl
30 files changed, 603 insertions(+), 206 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I989cf5b259afb8f5bc5c35590c94961c81ce88bf
Gerrit-Change-Number: 10060
Gerrit-PatchSet: 7
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-5216: Make admission control queuing async

2018-05-07 Thread Bikramjeet Vig (Code Review)
Bikramjeet Vig has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10060 )

Change subject: IMPALA-5216: Make admission control queuing async
..


Patch Set 5:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/10060/5/be/src/service/client-request-state.h
File be/src/service/client-request-state.h:

http://gerrit.cloudera.org:8080/#/c/10060/5/be/src/service/client-request-state.h@151
PS5, Line 151:   /// that these updates reach the coordinator object after 
Exec() has been called on it.
> Are these comments stale? It seems like these functions are just straightfo
not stale, but you are right, they can be worded better.


http://gerrit.cloudera.org:8080/#/c/10060/5/be/src/service/client-request-state.cc
File be/src/service/client-request-state.cc:

http://gerrit.cloudera.org:8080/#/c/10060/5/be/src/service/client-request-state.cc@735
PS5, Line 735:   || operation_state_ < operation_state)
> Need braces for multi-line if.
Done


http://gerrit.cloudera.org:8080/#/c/10060/5/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

http://gerrit.cloudera.org:8080/#/c/10060/5/be/src/service/impala-server.cc@673
PS5, Line 673: "Query id $0 has not started any backends yet.", 
PrintId(query_id));
> Does this interact with live_summary=1 in the shell at all?
nice catch. As discussed, returning an empty ExecSummary object instead


http://gerrit.cloudera.org:8080/#/c/10060/5/tests/authorization/test_authorization.py
File tests/authorization/test_authorization.py:

http://gerrit.cloudera.org:8080/#/c/10060/5/tests/authorization/test_authorization.py@299
PS5, Line 299: 
self.wait_for_admission_control(execute_statement_resp.operationHandle)
based on the change that an empty ExecSummary is returned if in PENDING state 
then just an OK response from GetExecSummary means that it was successful and 
user has access.
Therefore removing the need to wait for admission control here.


http://gerrit.cloudera.org:8080/#/c/10060/5/tests/authorization/test_authorization.py@330
PS5, Line 330:   def wait_for_admission_control(self, operation_handle):
 : """Waits for the admission control processing of the query 
to complete by polling
 :   GetOperationStatus"""
 : from tests.hs2.test_hs2 import TestHS2
 : while True:
 :   get_operation_status_req = 
TCLIService.TGetOperationStatusReq()
 :   get_operation_status_req.operationHandle = operation_handle
 :   get_operation_status_resp = \
 : 
self.hs2_client.GetOperationStatus(get_operation_status_req)
 :   TestHS2.check_response(get_operation_status_resp)
 :   if TCLIService.TOperationState.INITIALIZED_STATE < \
 :   get_operation_status_resp.operationState < \
 :   TCLIService.TOperationState.PENDING_STATE:
 : return get_operation_status_resp
 :   sleep(0.05)
removed due to reason mentioned above


http://gerrit.cloudera.org:8080/#/c/10060/5/tests/hs2/test_hs2.py
File tests/hs2/test_hs2.py:

http://gerrit.cloudera.org:8080/#/c/10060/5/tests/hs2/test_hs2.py@456
PS5, Line 456: # Wait for query to start running.
> Should we try fetching  the summary before admission control finishes? To e
based on the change that an empty ExecSummary is returned if in PENDING state, 
there will no longer be an error path. Instead, moving the 
wait_for_admission_control() call after Line 467 as before that the 
GetExecSummary() call should be successful in both PENDING and RUNNING state.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I989cf5b259afb8f5bc5c35590c94961c81ce88bf
Gerrit-Change-Number: 10060
Gerrit-PatchSet: 5
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 07 May 2018 17:48:46 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5216: Make admission control queuing async

2018-05-07 Thread Bikramjeet Vig (Code Review)
Hello Tim Armstrong, Dan Hecht,

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

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

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

Change subject: IMPALA-5216: Make admission control queuing async
..

IMPALA-5216: Make admission control queuing async

Implement asynchronous admission control queuing. This is achieved by
running the admission control code-path in a separate thread. Major
changes include: propagating cancellation to the admission control
thread and dequeuing thread, and adding a new Query Operation State
called "PENDING" that represents the state between completion of
planning and starting of query execution.

Testing:
- Added a deterministic end to end test
- Ran multiple stress tests successfully with a cancellation probability
of 60% and with different values for the following parameters:
max_requests, queue_wait_timeout_ms. Ensured that the impalad was in a
valid state afterwards (no orphan fragments or wrong metrics).
- Ran all exhaustive tests and ASAN core tests successfully.

TODO: change terminology of "in_flight_query" to "submitted_queries".
  Need to identify all references of this terminology, eg. in
  comments, tests, variable names, etc.

Change-Id: I989cf5b259afb8f5bc5c35590c94961c81ce88bf
---
M be/src/common/atomic.h
M be/src/common/logging.h
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
M be/src/scheduling/query-schedule.cc
M be/src/scheduling/query-schedule.h
M be/src/service/client-request-state.cc
M be/src/service/client-request-state.h
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-http-handler.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/util/debug-util.cc
M be/src/util/debug-util.h
M be/src/util/promise-test.cc
M be/src/util/promise.h
M common/thrift/ImpalaService.thrift
M testdata/workloads/functional-query/queries/QueryTest/runtime_row_filters.test
M tests/authorization/test_authorization.py
M tests/beeswax/impala_beeswax.py
M tests/common/impala_connection.py
M tests/custom_cluster/test_admission_controller.py
M tests/custom_cluster/test_krpc_mem_usage.py
M tests/hs2/hs2_test_suite.py
M tests/hs2/test_hs2.py
M tests/query_test/test_observability.py
M www/query_backends.tmpl
30 files changed, 604 insertions(+), 206 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I989cf5b259afb8f5bc5c35590c94961c81ce88bf
Gerrit-Change-Number: 10060
Gerrit-PatchSet: 6
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-6946: handle negative counts in RLE decoder

2018-05-07 Thread Dan Hecht (Code Review)
Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10233 )

Change subject: IMPALA-6946: handle negative counts in RLE decoder
..


Patch Set 7: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If75ef3fb12494209918c100f26407cd93b17addb
Gerrit-Change-Number: 10233
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 07 May 2018 17:34:39 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6946: handle negative counts in RLE decoder

2018-05-07 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10233 )

Change subject: IMPALA-6946: handle negative counts in RLE decoder
..


Patch Set 7: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10233/7/be/src/util/rle-test.cc
File be/src/util/rle-test.cc:

http://gerrit.cloudera.org:8080/#/c/10233/7/be/src/util/rle-test.cc@518
PS7, Line 518: Decoding failed above
> The message in this case is the justification for the check. I can just rem
I wanted to make sure I understood this correctly and I'm good leaving it as it 
is.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If75ef3fb12494209918c100f26407cd93b17addb
Gerrit-Change-Number: 10233
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 07 May 2018 17:27:04 +
Gerrit-HasComments: Yes


[Impala-ASF-CR](2.x) IMPALA-6908: IsConnResetTException() should include ECONNRESET

2018-05-07 Thread Michael Ho (Code Review)
Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10265 )

Change subject: IMPALA-6908: IsConnResetTException() should include ECONNRESET
..


Patch Set 3: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10265/3/be/src/rpc/thrift-util.cc
File be/src/rpc/thrift-util.cc:

http://gerrit.cloudera.org:8080/#/c/10265/3/be/src/rpc/thrift-util.cc@201
PS3, Line 201:  0.9.0, since in 0.9.3
Is there a way to validate this behavior ?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I1bb997a833917e5166c9ca192da219f50f4439e2
Gerrit-Change-Number: 10265
Gerrit-PatchSet: 3
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Mon, 07 May 2018 17:25:06 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6946: handle negative counts in RLE decoder

2018-05-07 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10233 )

Change subject: IMPALA-6946: handle negative counts in RLE decoder
..


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10233/7/be/src/util/rle-test.cc
File be/src/util/rle-test.cc:

http://gerrit.cloudera.org:8080/#/c/10233/7/be/src/util/rle-test.cc@518
PS7, Line 518: Decoding failed above
> This is the error that gets printed if the expect fails, no? The expect its
The message in this case is the justification for the check. I can just remove 
it if it's unhelpful.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If75ef3fb12494209918c100f26407cd93b17addb
Gerrit-Change-Number: 10233
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 07 May 2018 17:20:09 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6974: Use CMAKE POSITION INDEPENDENT CODE in backend

2018-05-07 Thread Dan Hecht (Code Review)
Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10267 )

Change subject: IMPALA-6974: Use CMAKE_POSITION_INDEPENDENT_CODE in backend
..


Patch Set 5: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id37bb5afa6a9b7909bb4efe1390a67f7d1469544
Gerrit-Change-Number: 10267
Gerrit-PatchSet: 5
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 07 May 2018 17:14:19 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6941: load more text scanner compression plugins

2018-05-07 Thread Dan Hecht (Code Review)
Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10165 )

Change subject: IMPALA-6941: load more text scanner compression plugins
..


Patch Set 8: Code-Review+2

(1 comment)

Please see if Alex/Dimitris wants to take a look but lgtm.

http://gerrit.cloudera.org:8080/#/c/10165/8/be/src/exec/hdfs-text-scanner.h
File be/src/exec/hdfs-text-scanner.h:

http://gerrit.cloudera.org:8080/#/c/10165/8/be/src/exec/hdfs-text-scanner.h@70
PS8, Line 70: has_builtin_support
should probably be HasBuiltinSupport()



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If2a9c4a4a11bed81df706e9e834400bfedfe48e6
Gerrit-Change-Number: 10165
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 07 May 2018 17:10:33 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6946: handle negative counts in RLE decoder

2018-05-07 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10233 )

Change subject: IMPALA-6946: handle negative counts in RLE decoder
..


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10233/7/be/src/util/rle-test.cc
File be/src/util/rle-test.cc:

http://gerrit.cloudera.org:8080/#/c/10233/7/be/src/util/rle-test.cc@518
PS7, Line 518: Decoding failed above
This is the error that gets printed if the expect fails, no? The expect itself 
checks that GetValues() failed so the error description gets printed when 
decoding succeeded but should have failed. Am I missing something?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If75ef3fb12494209918c100f26407cd93b17addb
Gerrit-Change-Number: 10233
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 07 May 2018 16:54:44 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6946: handle negative counts in RLE decoder

2018-05-07 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10233 )

Change subject: IMPALA-6946: handle negative counts in RLE decoder
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10233/5/be/src/util/rle-test.cc
File be/src/util/rle-test.cc:

http://gerrit.cloudera.org:8080/#/c/10233/5/be/src/util/rle-test.cc@517
PS5, Line 517: decoder.GetValues(1, );
> This looks to me like we're reading from a partially initialized buffer. We
Good point. The buffer is zero-initialised so we'd expect it to return 
all-zeros if anything. Updated to reflect expected behaviour



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If75ef3fb12494209918c100f26407cd93b17addb
Gerrit-Change-Number: 10233
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 07 May 2018 16:43:52 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6946: handle negative counts in RLE decoder

2018-05-07 Thread Tim Armstrong (Code Review)
Hello Lars Volker, Csaba Ringhofer,

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

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

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

Change subject: IMPALA-6946: handle negative counts in RLE decoder
..

IMPALA-6946: handle negative counts in RLE decoder

This improves the handling of out-of-range values to avoid hitting various
DCHECKs, including the one in the JIRA. repeat_count_ and literal_count_
are int32_ts. Avoid setting them to a negative value directly or by
integer overflow.

Switch to using uint32_t for "VLQ" encoding, which should be ULEB-128
encoding according to the Parquet standard. This fixes an infinite loop
in PutVlqInt() for negative values - the bug was that shifting right
sign-extended the signed value.

Testing:
Added backend test to exercise handling of large literal and repeat
counts that don't fit in an int32_t. Before these fixes it could trigger
several DCHECKs.

Change-Id: If75ef3fb12494209918c100f26407cd93b17addb
---
M be/src/util/bit-stream-utils.h
M be/src/util/bit-stream-utils.inline.h
M be/src/util/dict-encoding.h
M be/src/util/rle-encoding.h
M be/src/util/rle-test.cc
5 files changed, 90 insertions(+), 39 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If75ef3fb12494209918c100f26407cd93b17addb
Gerrit-Change-Number: 10233
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] Move admission-controller and catalog metrics into own groups

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

Change subject: Move admission-controller and catalog metrics into own groups
..


Patch Set 2:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2418/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I17dbbcebc01cc1f5b8e94e593873cdc3dc5e36df
Gerrit-Change-Number: 10302
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 07 May 2018 16:39:01 +
Gerrit-HasComments: No


[Impala-ASF-CR] Move admission-controller and catalog metrics into own groups

2018-05-07 Thread Philip Zeyliger (Code Review)
Philip Zeyliger has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10302 )

Change subject: Move admission-controller and catalog metrics into own groups
..


Patch Set 2: Code-Review+2

Works for me.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I17dbbcebc01cc1f5b8e94e593873cdc3dc5e36df
Gerrit-Change-Number: 10302
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 07 May 2018 16:33:27 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6970: race with decreasing scanner reservation

2018-05-07 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10314 )

Change subject: IMPALA-6970: race with decreasing scanner reservation
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10314/2/be/src/runtime/bufferpool/buffer-pool-internal.h
File be/src/runtime/bufferpool/buffer-pool-internal.h:

http://gerrit.cloudera.org:8080/#/c/10314/2/be/src/runtime/bufferpool/buffer-pool-internal.h@256
PS2, Line 256:   Status DecreaseReservationTo(int64_t max_decrease, int64_t 
target_bytes) WARN_UNUSED_RESULT;
long line



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I653225c981bf674d2b2b947f3a3cb4d8f382d36b
Gerrit-Change-Number: 10314
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 07 May 2018 16:32:08 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6970: race with decreasing scanner reservation

2018-05-07 Thread Tim Armstrong (Code Review)
Tim Armstrong has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10314


Change subject: IMPALA-6970: race with decreasing scanner reservation
..

IMPALA-6970: race with decreasing scanner reservation

The usage pattern of DecreaseReservationTo() in
ReturnReservationFromScannerThread() can decrease the reservation by
more than 'bytes' if there is a concurrent IncreaseReservation() call.

The API is changed to fit the requirements of the scanner, which
does not want to release more than the memory reserved for that
scanner thread.

Testing:
Added some backend tests (including concurrency) for the new API.

Change-Id: I653225c981bf674d2b2b947f3a3cb4d8f382d36b
---
M be/src/benchmarks/bloom-filter-benchmark.cc
M be/src/exec/exec-node.cc
M be/src/exec/exec-node.h
M be/src/exec/hdfs-scan-node.cc
M be/src/runtime/bufferpool/buffer-pool-internal.h
M be/src/runtime/bufferpool/buffer-pool-test.cc
M be/src/runtime/bufferpool/buffer-pool.cc
M be/src/runtime/bufferpool/buffer-pool.h
8 files changed, 76 insertions(+), 35 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I653225c981bf674d2b2b947f3a3cb4d8f382d36b
Gerrit-Change-Number: 10314
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 


[Impala-ASF-CR] Move admission-controller and catalog metrics into own groups

2018-05-07 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10302 )

Change subject: Move admission-controller and catalog metrics into own groups
..


Patch Set 2:

I'm not aware of any tools that depend on the group structure. The current 
organisation is a total mess - there's no alignment between the metric groups 
and the metric names - so I think any sane tool would just be walking the 
structure and picking out metrics by the full name rather than depending on the 
structure (or using the /jsonmetrics endpoint)

Ideally we'd rework this so that the metric names and groups were automatically 
consistent but I'm just scratching my own itch here.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I17dbbcebc01cc1f5b8e94e593873cdc3dc5e36df
Gerrit-Change-Number: 10302
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 07 May 2018 16:29:24 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5842: Write page index in Parquet files

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

Change subject: IMPALA-5842: Write page index in Parquet files
..


Patch Set 13:

(15 comments)

http://gerrit.cloudera.org:8080/#/c/9693/2/be/src/exec/hdfs-parquet-table-writer.cc
File be/src/exec/hdfs-parquet-table-writer.cc:

http://gerrit.cloudera.org:8080/#/c/9693/2/be/src/exec/hdfs-parquet-table-writer.cc@1195
PS2, Line 1195:   // Populate RowGroup::sorting_columns with all columns 
specified by the Frontend.
> I didn't follow up on this. It looks like the code only ever writes one row
Unfortunately it wouldn't work, because based on the description of the Parquet 
page indexes 
(https://github.com/apache/parquet-format/blob/master/PageIndex.md) we should 
write all the page indexes of all the row groups together, just before the 
footer. In other words, we have to flush all the row groups, then we can write 
all the page indexes.

Currently Impala writes only one row group per Parquet file, so I just add a 
DCHECK here. If Impala's behavior changes in the future in this regard, we'll 
need to update the write path of the page indexes.


http://gerrit.cloudera.org:8080/#/c/9693/13/be/src/exec/hdfs-parquet-table-writer.cc
File be/src/exec/hdfs-parquet-table-writer.cc:

http://gerrit.cloudera.org:8080/#/c/9693/13/be/src/exec/hdfs-parquet-table-writer.cc@107
PS13, Line 107:   page_index_mem_tracker_(-1, "Parquet Page Index",
> We can have many HdfsTableWriters per HdfsTableSink if we're doing non-clus
I track directly against HdfsTableSink.


http://gerrit.cloudera.org:8080/#/c/9693/13/be/src/exec/hdfs-parquet-table-writer.cc@1212
PS13, Line 1212:   if (file_metadata_.row_groups.size() != 1) return 
Status::OK();
> This
Answered it at PS2.


http://gerrit.cloudera.org:8080/#/c/9693/13/be/src/exec/hdfs-parquet-table-writer.cc@1214
PS13, Line 1214:   parquet::RowGroup* row_group = 
&(file_metadata_.row_groups[0]);
> Can't we use current_row_group_ here?
We can't, detailed answer is at PS2.


http://gerrit.cloudera.org:8080/#/c/9693/13/be/src/exec/hdfs-parquet-table-writer.cc@1216
PS13, Line 1216:   for (int i = 0; i < columns_.size(); i++) {
> I injected an error here by forgetting to write out one of the pages and th
Thanks for checking.


http://gerrit.cloudera.org:8080/#/c/9693/13/be/src/exec/hdfs-parquet-table-writer.cc@1241
PS13, Line 1241: row_group->columns[i].__set_offset_index_offset(file_pos_);
> I added some errors to the offsets and lengths - looks like the tests caugh
Thanks


http://gerrit.cloudera.org:8080/#/c/9693/13/be/src/exec/parquet-column-stats.inline.h
File be/src/exec/parquet-column-stats.inline.h:

http://gerrit.cloudera.org:8080/#/c/9693/13/be/src/exec/parquet-column-stats.inline.h@31
PS13, Line 31:   ascending_boundary_order_ = true;
> I tried injecting a bug by initializing these to "false". The tests didn't
Currently we don't invoke Reset() on the ColumnStatsBase object where it would 
matter.

In HdfsParquetTableWriter::BaseColumnWriter there are two ColumnStatsBase 
members: page_stats_base_ and row_group_stats_base_.

For each new page, we invoke Reset() on page_stats_base_, but that doesn't 
corrupt the ordering, because we only consider the state of 
row_group_stats_base_ for that, and we never call Reset() on it.

Even in BaseColumnWriter::Reset(), we throw away the old row_group_stats_base_ 
and create a brand new one.


http://gerrit.cloudera.org:8080/#/c/9693/13/be/src/exec/parquet-column-stats.inline.h@56
PS13, Line 56: false
> I tried injecting a bug by flipping the value of this and the tests caught
Thanks!


http://gerrit.cloudera.org:8080/#/c/9693/13/be/src/exec/parquet-column-stats.inline.h@60
PS13, Line 60: if (MinMaxTrait::Compare(prev_page_max_value_, 
cs->max_value_) < 0 ||
> I tried injecting a bug by removing the first condition and the tests caugh
Thanks!


http://gerrit.cloudera.org:8080/#/c/9693/13/be/src/exec/parquet-column-stats.inline.h@68
PS13, Line 68: max_value_
> I tried switching this to cs->min_value. The tests caught it.
Thanks!


http://gerrit.cloudera.org:8080/#/c/9693/13/be/src/exec/parquet-column-stats.inline.h@69
PS13, Line 69: prev_page_min_buffer_.Clear();
> I tried removing this line. The tests didn't catch it, even under ASAN.
Turned out that the _validate_ordering function was too loose in 
test_parquet_page_index.py. We could remove both prev_page_min_buffer_ and 
prev_page_max_buffer_ without noticing it.

Now _validate_ordering implements the same logic from parquet-column-stats to 
determine the expected ordering.

Interestingly the tests only catch the case when we remove 
prev_page_max_buffer_.Clear(). The lack of prev_page_min_buffer.Clear() 
remained unnoticed. I tried to run the tests on a couple of existing tables, 
but without any luck.

The reason is a bit subtle, basically prev_page_min_value_.ptr is "lucky" 
enough to point at memory 

[Impala-ASF-CR] IMPALA-5706: Parallelise read I/O in sorter

2018-05-07 Thread Csaba Ringhofer (Code Review)
Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9943 )

Change subject: IMPALA-5706: Parallelise read I/O in sorter
..


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9943/7/be/src/runtime/sorter.cc
File be/src/runtime/sorter.cc:

http://gerrit.cloudera.org:8080/#/c/9943/7/be/src/runtime/sorter.cc@1730
PS7, Line 1730: return (sorted_runs_.size() + 1) / 2;
If the goal is to minimize the number of "extra merges" per row, then it is 
optimal for the final merge to always merge as much runs as possible, so this 
line could return
sorted_runs_.size() - max_runs_per_intermediate_merge

An example when this would result in less merges:
max_runs_per_intermediate_merge: 3
number of runs: 7
(the numbers are the number of original runs merged into a run)

1 1 1 1 1 1 1

1 1 1 1 3 - current logic decides to merge (5+1/2)=3 runs

1 3 3 - ready for final merge after merging 6 runs

1 1 1 1 1 1 1

1 1 1 1 3 - new logic would decide to merge 5-3=2 runs

1 1 3 2 - ready for final merge after merging 5 runs

On the other hand, merging more runs in the final merge means that the buffers 
will be released later, so I am not completely sure that maximizing it is a 
good idea.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I74857c1694802e81f1cfc765d2b4e8bc644387f9
Gerrit-Change-Number: 9943
Gerrit-PatchSet: 7
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 07 May 2018 15:59:02 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5842: Write page index in Parquet files

2018-05-07 Thread Zoltan Borok-Nagy (Code Review)
Hello Lars Volker, Anonymous Coward #248, Tim Armstrong, Csaba Ringhofer,

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

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

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

Change subject: IMPALA-5842: Write page index in Parquet files
..

IMPALA-5842: Write page index in Parquet files

This commit builds on the previous work of
Pooja Nilangekar: https://gerrit.cloudera.org/#/c/7464/

The commit implements the write path of PARQUET-922:
"Add column indexes to parquet.thrift". As specified in the
parquet-format, Impala writes the page indexes just before
the footer. This allows much more efficient page filtering
than using the same information from the 'statistics' field
of DataPageHeader.

I updated Pooja's python tests as well.

Change-Id: Icbacf7fe3b7672e3ce719261ecef445b16f8dec9
---
M be/src/exec/hdfs-parquet-table-writer.cc
M be/src/exec/hdfs-parquet-table-writer.h
M be/src/exec/parquet-column-stats.h
M be/src/exec/parquet-column-stats.inline.h
M be/src/util/CMakeLists.txt
A be/src/util/string-util-test.cc
A be/src/util/string-util.cc
A be/src/util/string-util.h
M common/thrift/parquet.thrift
M testdata/bin/load-dependent-tables.sql
M tests/query_test/test_chars.py
A tests/query_test/test_parquet_page_index.py
M tests/util/get_parquet_metadata.py
13 files changed, 923 insertions(+), 48 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/93/9693/15
--
To view, visit http://gerrit.cloudera.org:8080/9693
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icbacf7fe3b7672e3ce719261ecef445b16f8dec9
Gerrit-Change-Number: 9693
Gerrit-PatchSet: 15
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Anonymous Coward #248
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-5842: Write page index in Parquet files

2018-05-07 Thread Zoltan Borok-Nagy (Code Review)
Hello Lars Volker, Anonymous Coward #248, Tim Armstrong, Csaba Ringhofer,

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

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

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

Change subject: IMPALA-5842: Write page index in Parquet files
..

IMPALA-5842: Write page index in Parquet files

This commit builds on the previous work of
Pooja Nilangekar: https://gerrit.cloudera.org/#/c/7464/

The commit implements the write path of PARQUET-922:
"Add column indexes to parquet.thrift". As specified in the
parquet-format, Impala writes the page indexes just before
the footer. This allows much more efficient page filtering
than using the same information from the 'statistics' field
of DataPageHeader.

I updated Pooja's python tests as well.

Change-Id: Icbacf7fe3b7672e3ce719261ecef445b16f8dec9
---
M be/src/exec/hdfs-parquet-table-writer.cc
M be/src/exec/hdfs-parquet-table-writer.h
M be/src/exec/parquet-column-stats.h
M be/src/exec/parquet-column-stats.inline.h
M be/src/util/CMakeLists.txt
A be/src/util/string-util-test.cc
A be/src/util/string-util.cc
A be/src/util/string-util.h
M common/thrift/parquet.thrift
M testdata/bin/load-dependent-tables.sql
M tests/query_test/test_chars.py
A tests/query_test/test_parquet_page_index.py
M tests/util/get_parquet_metadata.py
13 files changed, 923 insertions(+), 48 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/93/9693/14
--
To view, visit http://gerrit.cloudera.org:8080/9693
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icbacf7fe3b7672e3ce719261ecef445b16f8dec9
Gerrit-Change-Number: 9693
Gerrit-PatchSet: 14
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Anonymous Coward #248
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-6131: Track time of last statistics update in metadata

2018-05-07 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10116 )

Change subject: IMPALA-6131: Track time of last statistics update in metadata
..


Patch Set 8: Code-Review+1

Alex, do you have time to look at this?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I59a671ac29d352bd92ce40d5cb6662bb23f146b5
Gerrit-Change-Number: 10116
Gerrit-PatchSet: 8
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 07 May 2018 15:42:15 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6946: handle negative counts in RLE decoder

2018-05-07 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10233 )

Change subject: IMPALA-6946: handle negative counts in RLE decoder
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10233/5/be/src/util/rle-test.cc
File be/src/util/rle-test.cc:

http://gerrit.cloudera.org:8080/#/c/10233/5/be/src/util/rle-test.cc@517
PS5, Line 517: decoder.GetValues(1, );
This looks to me like we're reading from a partially initialized buffer. We 
wrote the header, but no values. Is this supported? Should we add a check that 
val is what we expect, i.e. 0?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If75ef3fb12494209918c100f26407cd93b17addb
Gerrit-Change-Number: 10233
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 07 May 2018 15:40:53 +
Gerrit-HasComments: Yes