[Impala-ASF-CR] IMPALA-13108: Update version to 4.5.0-SNAPSHOT

2024-05-29 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21460 )

Change subject: IMPALA-13108: Update version to 4.5.0-SNAPSHOT
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7803fe523406dbdd1ac066a35bb31d21765a244
Gerrit-Change-Number: 21460
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Smith 
Gerrit-Comment-Date: Wed, 29 May 2024 18:45:52 +
Gerrit-HasComments: No


[Impala-ASF-CR](asf-site) Add DOAP file

2024-05-29 Thread Michael Smith (Code Review)
Michael Smith has removed a vote on this change.

Change subject: Add DOAP file
..


Removed Code-Review+2 by Michael Smith 
--
To view, visit http://gerrit.cloudera.org:8080/21449
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ib1a47c68345769281449dd377a6f82a7257cee07
Gerrit-Change-Number: 21449
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 


[Impala-ASF-CR](asf-site) Add DOAP file

2024-05-29 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21449 )

Change subject: Add DOAP file
..


Patch Set 3: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib1a47c68345769281449dd377a6f82a7257cee07
Gerrit-Change-Number: 21449
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Comment-Date: Wed, 29 May 2024 18:45:38 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12562: Cast double and float to string with exact presicion

2024-05-29 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21441 )

Change subject: IMPALA-12562: Cast double and float to string with exact 
presicion
..


Patch Set 5: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
Gerrit-Change-Number: 21441
Gerrit-PatchSet: 5
Gerrit-Owner: Yifan Zhang 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Yifan Zhang 
Gerrit-Comment-Date: Wed, 29 May 2024 18:42:39 +
Gerrit-HasComments: No


[Impala-ASF-CR](asf-site) Add DOAP file

2024-05-29 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21449 )

Change subject: Add DOAP file
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib1a47c68345769281449dd377a6f82a7257cee07
Gerrit-Change-Number: 21449
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Comment-Date: Wed, 29 May 2024 18:40:56 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-13107: Don't start query on executor if instance number equals 0

2024-05-29 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21458 )

Change subject: IMPALA-13107: Don't start query on executor if instance number 
equals 0
..


Patch Set 5: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie92ee120f1e9369f8dc2512792a05b7f8be5f007
Gerrit-Change-Number: 21458
Gerrit-PatchSet: 5
Gerrit-Owner: Wenzhe Zhou 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Comment-Date: Wed, 29 May 2024 17:49:46 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-13107: Don't start query on executor if instance number equals 0

2024-05-29 Thread Michael Smith (Code Review)
Michael Smith has removed a vote on this change.

Change subject: IMPALA-13107: Don't start query on executor if instance number 
equals 0
..


Removed Code-Review+1 by Michael Smith 
--
To view, visit http://gerrit.cloudera.org:8080/21458
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ie92ee120f1e9369f8dc2512792a05b7f8be5f007
Gerrit-Change-Number: 21458
Gerrit-PatchSet: 5
Gerrit-Owner: Wenzhe Zhou 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Wenzhe Zhou 


[Impala-ASF-CR] IMPALA-13107: Don't start query on executor if instance number equals 0

2024-05-29 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21458 )

Change subject: IMPALA-13107: Don't start query on executor if instance number 
equals 0
..


Patch Set 5: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie92ee120f1e9369f8dc2512792a05b7f8be5f007
Gerrit-Change-Number: 21458
Gerrit-PatchSet: 5
Gerrit-Owner: Wenzhe Zhou 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Comment-Date: Wed, 29 May 2024 17:26:06 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12939: Bound IMPALA BUILD THREADS for cgroups and memory

2024-05-23 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21200 )

Change subject: IMPALA-12939: Bound IMPALA_BUILD_THREADS for cgroups and memory
..


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21200/8/bin/impala-config.sh
File bin/impala-config.sh:

http://gerrit.cloudera.org:8080/#/c/21200/8/bin/impala-config.sh@1067
PS8, Line 1067: AVAILABLE_MEM
> missing $ sign?
Not needed in $(()), as done in the line below. All the others were though, 
it's an odd syntax.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I87994d0464073fe2d91bc2f7c2592c012e42de71
Gerrit-Change-Number: 21200
Gerrit-PatchSet: 8
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Fri, 24 May 2024 00:11:29 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12939: Bound IMPALA BUILD THREADS for cgroups and memory

2024-05-23 Thread Michael Smith (Code Review)
Hello Laszlo Gaal, Riza Suminto, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-12939: Bound IMPALA_BUILD_THREADS for cgroups and memory
..

IMPALA-12939: Bound IMPALA_BUILD_THREADS for cgroups and memory

Updates IMPALA_BUILD_THREADS to bound it based on guideline of 2 GB
memory per core during builds. Computes cores and memory from cgroup
limits if applicable; memory is used as a bound on physical memory, as
sometimes cgroups will report a larger limit than available physical
memory.

Uses IMPALA_BUILD_THREADS for load-data.

Adds a default in case USER is unset during bootstrap, which can occur
in devcontainer.

Change-Id: I87994d0464073fe2d91bc2f7c2592c012e42de71
---
M bin/bootstrap_development.sh
M bin/bootstrap_system.sh
M bin/impala-config.sh
M bin/load-data.py
4 files changed, 73 insertions(+), 9 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I87994d0464073fe2d91bc2f7c2592c012e42de71
Gerrit-Change-Number: 21200
Gerrit-PatchSet: 8
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Riza Suminto 


[Impala-ASF-CR] IMPALA-12939: Bound IMPALA BUILD THREADS for cgroups and memory

2024-05-23 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21200 )

Change subject: IMPALA-12939: Bound IMPALA_BUILD_THREADS for cgroups and memory
..


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21200/7/bin/impala-config.sh
File bin/impala-config.sh:

http://gerrit.cloudera.org:8080/#/c/21200/7/bin/impala-config.sh@1059
PS7, Line 1059:   echo "Detected $AVAILABLE_MEM GB memory from cgroups v1"
It seems sometimes this is also set to something near INT64_MAX

  Detected 8589934591 GB memory from cgroups v1

We should probably bound that by whatever physical memory we detected.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I87994d0464073fe2d91bc2f7c2592c012e42de71
Gerrit-Change-Number: 21200
Gerrit-PatchSet: 7
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Thu, 23 May 2024 23:41:33 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12562: Cast double and float to string with exact presicion

2024-05-23 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21441 )

Change subject: IMPALA-12562: Cast double and float to string with exact 
presicion
..


Patch Set 4:

> Patch Set 4: Verified-1
>
> Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10665/

Tests show some real failures: 
https://jenkins.impala.io/job/ubuntu-20.04-from-scratch/2665/testReport/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
Gerrit-Change-Number: 21441
Gerrit-PatchSet: 4
Gerrit-Owner: Yifan Zhang 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Yifan Zhang 
Gerrit-Comment-Date: Thu, 23 May 2024 23:01:16 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-13034: Add logs and counters for HTTP profile requests blocking client fetches

2024-05-23 Thread Michael Smith (Code Review)
Michael Smith has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/21412 )

Change subject: IMPALA-13034: Add logs and counters for HTTP profile requests 
blocking client fetches
..

IMPALA-13034: Add logs and counters for HTTP profile requests blocking client 
fetches

There are several endpoints in WebUI that can dump a query profile:
/query_profile, /query_profile_encoded, /query_profile_plain_text,
/query_profile_json. The HTTP handler thread goes into
ImpalaServer::GetRuntimeProfileOutput() which acquires lock of the
ClientRequestState. This could block client requests in fetching query
results.

To help identify this issue, this patch adds warning logs when such
profile dumping requests run slow and the query is still in-flight. Also
adds a profile counter, GetInFlightProfileTimeStats, for the summary
stats of this time. Dumping the profiles after the query is archived
(e.g. closed) won't be tracked.

Logs for slow http responses are also added. The thresholds are defined
by two new flags, slow_profile_dump_warning_threshold_ms, and
slow_http_response_warning_threshold_ms.

Note that dumping the profile in-flight won't always block the query,
e.g. if there are no client fetch requests or if the coordinator
fragment is idle waiting for executor fragment instances. So a long time
shown in GetInFlightProfileTimeStats doesn't mean it's hitting the
issue.

To better identify this issue, this patch adds another profile counter,
ClientFetchLockWaitTimer, as the cumulative time client fetch requests
waiting for locks.

Also fixes false positive logs for complaining invalid query handles.
Such logs are added in GetQueryHandle() when the query is not found in
the active query map, but it could still exist in the query log. This
removes the logs in GetQueryHandle() and lets the callers decide whether
to log the error.

Tests:
 - Added e2e test
 - Ran CORE tests

Change-Id: I538ebe914f70f460bc8412770a8f7a1cc8b505dc
Reviewed-on: http://gerrit.cloudera.org:8080/21412
Reviewed-by: Impala Public Jenkins 
Tested-by: Michael Smith 
---
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/util/webserver.cc
M tests/query_test/test_observability.py
8 files changed, 101 insertions(+), 13 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I538ebe914f70f460bc8412770a8f7a1cc8b505dc
Gerrit-Change-Number: 21412
Gerrit-PatchSet: 6
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Kurt Deschler 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 


[Impala-ASF-CR] IMPALA-13034: Add logs and counters for HTTP profile requests blocking client fetches

2024-05-23 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21412 )

Change subject: IMPALA-13034: Add logs and counters for HTTP profile requests 
blocking client fetches
..


Patch Set 5: Verified+1

Ran into https://issues.apache.org/jira/browse/IMPALA-12266.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I538ebe914f70f460bc8412770a8f7a1cc8b505dc
Gerrit-Change-Number: 21412
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Kurt Deschler 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Comment-Date: Thu, 23 May 2024 18:32:36 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-13034: Add logs and counters for HTTP profile requests blocking client fetches

2024-05-23 Thread Michael Smith (Code Review)
Michael Smith has removed a vote on this change.

Change subject: IMPALA-13034: Add logs and counters for HTTP profile requests 
blocking client fetches
..


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I538ebe914f70f460bc8412770a8f7a1cc8b505dc
Gerrit-Change-Number: 21412
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Kurt Deschler 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 


[Impala-ASF-CR] IMPALA-13057: Incorporate tuple/slot information into tuple cache key

2024-05-23 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21398 )

Change subject: IMPALA-13057: Incorporate tuple/slot information into tuple 
cache key
..


Patch Set 6: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21398/4/fe/src/main/java/org/apache/impala/analysis/Expr.java
File fe/src/main/java/org/apache/impala/analysis/Expr.java:

http://gerrit.cloudera.org:8080/#/c/21398/4/fe/src/main/java/org/apache/impala/analysis/Expr.java@946
PS4, Line 946:   result.add(expr.treeToThrift(serialCtx));
> Ooops, that is a mistake. Fixed this to pass in serialCtx.
Ack



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f5278e9dbb976cbebdc6a21a6e66bc90ce06c6c
Gerrit-Change-Number: 21398
Gerrit-PatchSet: 6
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Kurt Deschler 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Yida Wu 
Gerrit-Comment-Date: Thu, 23 May 2024 18:22:00 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12939: Bound IMPALA BUILD THREADS for cgroups and memory

2024-05-23 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21200 )

Change subject: IMPALA-12939: Bound IMPALA_BUILD_THREADS for cgroups and memory
..


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21200/5/bin/impala-config.sh
File bin/impala-config.sh:

http://gerrit.cloudera.org:8080/#/c/21200/5/bin/impala-config.sh@1042
PS5, Line 1042: # ASAN needs a matching version of llvm-symbolizer to symbolize 
stack tr
> Just as CORES, can we set reasonable AVAILABLE_MEM by default if /proc/memi
Added macOS and cgroups handling.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I87994d0464073fe2d91bc2f7c2592c012e42de71
Gerrit-Change-Number: 21200
Gerrit-PatchSet: 7
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Thu, 23 May 2024 18:08:41 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12939: Bound IMPALA BUILD THREADS for cgroups and memory

2024-05-23 Thread Michael Smith (Code Review)
Hello Laszlo Gaal, Riza Suminto, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-12939: Bound IMPALA_BUILD_THREADS for cgroups and memory
..

IMPALA-12939: Bound IMPALA_BUILD_THREADS for cgroups and memory

Updates IMPALA_BUILD_THREADS to bound it based on guideline of 2 GB
memory per core during builds. Computes cores and memory from cgroup
limits if applicable. Uses IMPALA_BUILD_THREADS for load-data.

Adds a default in case USER is unset during bootstrap, which can occur
in devcontainer.

Change-Id: I87994d0464073fe2d91bc2f7c2592c012e42de71
---
M bin/bootstrap_development.sh
M bin/bootstrap_system.sh
M bin/impala-config.sh
M bin/load-data.py
4 files changed, 69 insertions(+), 9 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I87994d0464073fe2d91bc2f7c2592c012e42de71
Gerrit-Change-Number: 21200
Gerrit-PatchSet: 7
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Riza Suminto 


[Impala-ASF-CR] IMPALA-12562: Cast double and float to string with exact presicion

2024-05-23 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21441 )

Change subject: IMPALA-12562: Cast double and float to string with exact 
presicion
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
Gerrit-Change-Number: 21441
Gerrit-PatchSet: 3
Gerrit-Owner: Yifan Zhang 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Yifan Zhang 
Gerrit-Comment-Date: Thu, 23 May 2024 17:30:48 +
Gerrit-HasComments: No


[Impala-ASF-CR](asf-site) Add DOAP file

2024-05-23 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21449 )

Change subject: Add DOAP file
..


Patch Set 2: Code-Review+1

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21449/1/doap_Impala.rdf
File doap_Impala.rdf:

http://gerrit.cloudera.org:8080/#/c/21449/1/doap_Impala.rdf@29
PS1, Line 29: https://impala.apache.org; />
> Good point. I think we can update the chapter there.
Ack


http://gerrit.cloudera.org:8080/#/c/21449/1/doap_Impala.rdf@41
PS1, Line 41: 4.3.0
> Yes, if we want to show them in this page: https://projects.apache.org/proj
Ack



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib1a47c68345769281449dd377a6f82a7257cee07
Gerrit-Change-Number: 21449
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Comment-Date: Thu, 23 May 2024 16:43:49 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-13093: Fix failure in inserting into OBS tables

2024-05-23 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21438 )

Change subject: IMPALA-13093: Fix failure in inserting into OBS tables
..


Patch Set 2: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2441da0fc521b4bbed10c8edceb937bde481
Gerrit-Change-Number: 21438
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Xiang Yang 
Gerrit-Reviewer: Zihao Ye 
Gerrit-Comment-Date: Thu, 23 May 2024 15:45:52 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12680: Fix NullPointerException during AlterTableAddPartitions

2024-05-22 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21430 )

Change subject: IMPALA-12680: Fix NullPointerException during 
AlterTableAddPartitions
..


Patch Set 1: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I730fed311ebc09762dccc152d9583d5394b0b9b3
Gerrit-Change-Number: 21430
Gerrit-PatchSet: 1
Gerrit-Owner: Sai Hemanth Gantasala 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Comment-Date: Wed, 22 May 2024 23:14:26 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-13093: Fix failure in inserting into OBS tables

2024-05-22 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21438 )

Change subject: IMPALA-13093: Fix failure in inserting into OBS tables
..


Patch Set 1: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21438/1/be/src/exec/table-sink-base.cc
File be/src/exec/table-sink-base.cc:

http://gerrit.cloudera.org:8080/#/c/21438/1/be/src/exec/table-sink-base.cc@326
PS1, Line 326:   IsOBSPath(tmp_hdfs_file_name_cstr) ||
It'd make more sense at this point to invert this check: if IsHDFSPath.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2441da0fc521b4bbed10c8edceb937bde481
Gerrit-Change-Number: 21438
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Xiang Yang 
Gerrit-Reviewer: Zihao Ye 
Gerrit-Comment-Date: Wed, 22 May 2024 22:04:00 +
Gerrit-HasComments: Yes


[Impala-ASF-CR](asf-site) Add DOAP file

2024-05-22 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21449 )

Change subject: Add DOAP file
..


Patch Set 1:

(2 comments)

Validates at https://www.w3.org/RDF/Validator/ and looks reasonable to me.

http://gerrit.cloudera.org:8080/#/c/21449/1/doap_Impala.rdf
File doap_Impala.rdf:

http://gerrit.cloudera.org:8080/#/c/21449/1/doap_Impala.rdf@29
PS1, Line 29: https://impala.apache.org; />
We'll probably also want to update 
https://svn.apache.org/repos/asf/comdev/projects.apache.org/trunk/data/committees/impala.rdf.


http://gerrit.cloudera.org:8080/#/c/21449/1/doap_Impala.rdf@41
PS1, Line 41: 4.3.0
We'll need to update this as part of the release process?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib1a47c68345769281449dd377a6f82a7257cee07
Gerrit-Change-Number: 21449
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Comment-Date: Wed, 22 May 2024 17:38:04 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-13020 (part 1): Change thrift rpc max message size to int64 t

2024-05-17 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21367 )

Change subject: IMPALA-13020 (part 1): Change thrift_rpc_max_message_size to 
int64_t
..


Patch Set 5: Code-Review+1

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/21367/5/be/src/rpc/thrift-util.cc@63
PS5, Line 63: "Default to the upper limit of 2147483647 bytes (~2GB). "
> "Default to the upper limit" seems misleading, as it's no longer the upper
This is all reworded in the next patch, so it's probably fine.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
Gerrit-Change-Number: 21367
Gerrit-PatchSet: 5
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Comment-Date: Fri, 17 May 2024 17:49:14 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-13020 (part 2): Split out external vs internal Thrift max message size

2024-05-17 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21420 )

Change subject: IMPALA-13020 (part 2): Split out external vs internal Thrift 
max message size
..


Patch Set 3: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21420/3/testdata/scale_test_metadata/create-wide-table.sql
File testdata/scale_test_metadata/create-wide-table.sql:

http://gerrit.cloudera.org:8080/#/c/21420/3/testdata/scale_test_metadata/create-wide-table.sql@a1005
PS3, Line 1005:
> This was a latent issue. The table definition needs to line up with the dir
Ack



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9a649ef49a8a99c7bd9a1b73c37c4c621661311
Gerrit-Change-Number: 21420
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Fri, 17 May 2024 17:47:50 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12562: Cast double and float to string with exact presicion

2024-05-17 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21441 )

Change subject: IMPALA-12562: Cast double and float to string with exact 
presicion
..


Patch Set 1: Code-Review+1

(1 comment)

Thanks for fixing this!

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

http://gerrit.cloudera.org:8080/#/c/21441/1/testdata/workloads/functional-query/queries/QueryTest/exprs.test@3300
PS1, Line 3300: select cast(round(cast(1.33 as double), 2) as string)
A few more test cases around the limits of large/small number of decimals would 
be nice.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
Gerrit-Change-Number: 21441
Gerrit-PatchSet: 1
Gerrit-Owner: Yifan Zhang 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Michael Smith 
Gerrit-Comment-Date: Fri, 17 May 2024 17:47:25 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-13020 (part 2): Split out external vs internal Thrift max message size

2024-05-17 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21420 )

Change subject: IMPALA-13020 (part 2): Split out external vs internal Thrift 
max message size
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21420/3/testdata/scale_test_metadata/create-wide-table.sql
File testdata/scale_test_metadata/create-wide-table.sql:

http://gerrit.cloudera.org:8080/#/c/21420/3/testdata/scale_test_metadata/create-wide-table.sql@a1005
PS3, Line 1005:
Why did this change?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9a649ef49a8a99c7bd9a1b73c37c4c621661311
Gerrit-Change-Number: 21420
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Fri, 17 May 2024 17:23:09 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-13034: Add logs and counters for HTTP profile requests blocking client fetches

2024-05-17 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21412 )

Change subject: IMPALA-13034: Add logs and counters for HTTP profile requests 
blocking client fetches
..


Patch Set 3: Code-Review+1

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/21412/2/be/src/service/client-request-state.h@498
PS2, Line 498:
> Done. Implicitly inline them by defining them here (inside the declaration
Done


http://gerrit.cloudera.org:8080/#/c/21412/2/tests/query_test/test_observability.py
File tests/query_test/test_observability.py:

http://gerrit.cloudera.org:8080/#/c/21412/2/tests/query_test/test_observability.py@974
PS2, Line 974: impalad = cluster.get_first_impalad()
> Didn't dig deeper into this. We can use handle.get_handle().id here.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I538ebe914f70f460bc8412770a8f7a1cc8b505dc
Gerrit-Change-Number: 21412
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Kurt Deschler 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Comment-Date: Fri, 17 May 2024 17:18:55 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12935: First pass on Calcite planner functions

2024-05-16 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21357 )

Change subject: IMPALA-12935: First pass on Calcite planner functions
..


Patch Set 10:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/21357/10/java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/AnalyzedFunctionCallExpr.java
File 
java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/AnalyzedFunctionCallExpr.java:

http://gerrit.cloudera.org:8080/#/c/21357/10/java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/AnalyzedFunctionCallExpr.java@69
PS10, Line 69:   //TODO: Don't throw runtime exception
If you're planning to leave TODOs in when we commit, please add the JIRA ticket 
where you plan to address them.


http://gerrit.cloudera.org:8080/#/c/21357/10/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaOperatorTable.java
File 
java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaOperatorTable.java:

http://gerrit.cloudera.org:8080/#/c/21357/10/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaOperatorTable.java@76
PS10, Line 76: // There shouldn't be more than one opName with our usage, 
so throw an exception
Why is that? How is "our usage" unique?


http://gerrit.cloudera.org:8080/#/c/21357/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeConverter.java
File 
java/calcite-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeConverter.java:

http://gerrit.cloudera.org:8080/#/c/21357/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeConverter.java@119
PS2, Line 119:* Get the normalized RelDataType given an impala type.
> Unused, deleted it.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2dd4e402d69ee10547abeeafe893164ffd789b88
Gerrit-Change-Number: 21357
Gerrit-PatchSet: 10
Gerrit-Owner: Steve Carlin 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Steve Carlin 
Gerrit-Comment-Date: Thu, 16 May 2024 21:09:06 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-13020 (part 2): Split out external vs internal Thrift max message size

2024-05-16 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21420 )

Change subject: IMPALA-13020 (part 2): Split out external vs internal Thrift 
max message size
..


Patch Set 2: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9a649ef49a8a99c7bd9a1b73c37c4c621661311
Gerrit-Change-Number: 21420
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Thu, 16 May 2024 17:40:50 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-13020 (part 2): Split out external vs internal Thrift max message size

2024-05-16 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21420 )

Change subject: IMPALA-13020 (part 2): Split out external vs internal Thrift 
max message size
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21420/1/be/src/catalog/catalogd-main.cc
File be/src/catalog/catalogd-main.cc:

http://gerrit.cloudera.org:8080/#/c/21420/1/be/src/catalog/catalogd-main.cc@87
PS1, Line 87:   // Mark this as an internal service to use a more permissive 
Thrift max message size
> Done
Ack


http://gerrit.cloudera.org:8080/#/c/21420/1/be/src/common/init.cc
File be/src/common/init.cc:

http://gerrit.cloudera.org:8080/#/c/21420/1/be/src/common/init.cc@548
PS1, Line 548:ThriftDefaultMaxMessageSize()));
> Ok, ThriftDefaultMaxMessageSize() is the minimum value. Make sense to me.
Ack



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9a649ef49a8a99c7bd9a1b73c37c4c621661311
Gerrit-Change-Number: 21420
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Thu, 16 May 2024 16:06:47 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-13057: Incorporate tuple/slot information into tuple cache key

2024-05-14 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21398 )

Change subject: IMPALA-13057: Incorporate tuple/slot information into tuple 
cache key
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21398/4/fe/src/main/java/org/apache/impala/analysis/Expr.java
File fe/src/main/java/org/apache/impala/analysis/Expr.java:

http://gerrit.cloudera.org:8080/#/c/21398/4/fe/src/main/java/org/apache/impala/analysis/Expr.java@946
PS4, Line 946:   result.add(expr.treeToThrift());
Why doesn't this pass the serialCtx?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f5278e9dbb976cbebdc6a21a6e66bc90ce06c6c
Gerrit-Change-Number: 21398
Gerrit-PatchSet: 4
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Kurt Deschler 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Yida Wu 
Gerrit-Comment-Date: Tue, 14 May 2024 21:47:46 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-13034: Add logs and counters for HTTP profile requests blocking client fetches

2024-05-14 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21412 )

Change subject: IMPALA-13034: Add logs and counters for HTTP profile requests 
blocking client fetches
..


Patch Set 2:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/21412/2/be/src/service/client-request-state.h@498
PS2, Line 498:   void UpdateGetInFlightProfileTime(int64_t elapsed_time_ns);
nit: we usually include a comment describing the function. Might be nice to 
reference the visible metric name it updates. Also might make sense as inline 
functions since they're so simple.


http://gerrit.cloudera.org:8080/#/c/21412/2/tests/query_test/test_observability.py
File tests/query_test/test_observability.py:

http://gerrit.cloudera.org:8080/#/c/21412/2/tests/query_test/test_observability.py@974
PS2, Line 974: query_id = query_id_search.group(1)
Doesn't handle.query_id have the query ID? Not sure why you're parsing the 
profile.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I538ebe914f70f460bc8412770a8f7a1cc8b505dc
Gerrit-Change-Number: 21412
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Kurt Deschler 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Comment-Date: Tue, 14 May 2024 21:16:08 +
Gerrit-HasComments: Yes


[native-toolchain-CR] IMPALA-13072: Add retries for s3 uploads to combat flakiness

2024-05-14 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21421 )

Change subject: IMPALA-13072: Add retries for s3 uploads to combat flakiness
..


Patch Set 1: Code-Review+1


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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95d858c99e965730303c2bfd90478ac5f68acf83
Gerrit-Change-Number: 21421
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Smith 
Gerrit-Comment-Date: Tue, 14 May 2024 19:13:05 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-13020 (part 2): Split out external vs internal Thrift max message size

2024-05-14 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21420 )

Change subject: IMPALA-13020 (part 2): Split out external vs internal Thrift 
max message size
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21420/1/be/src/common/init.cc
File be/src/common/init.cc:

http://gerrit.cloudera.org:8080/#/c/21420/1/be/src/common/init.cc@548
PS1, Line 548: FLAGS_thrift_external_rpc_max_message_size < 
ThriftDefaultMaxMessageSize()
> The intention is to enforce a minimum for the rpc max message size. Someone
Is there a possibility it would parse as unsigned but then get used as signed? 
Should at least check that.

I see this is the error message added previously; I guess a little rationale 
for why would be nice. The option description mentions what happens with 0 or 
negative, but doesn't mention anything about a minimum positive value.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9a649ef49a8a99c7bd9a1b73c37c4c621661311
Gerrit-Change-Number: 21420
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Tue, 14 May 2024 19:12:33 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-13020 (part 2): Split out external vs internal Thrift max message size

2024-05-14 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21420 )

Change subject: IMPALA-13020 (part 2): Split out external vs internal Thrift 
max message size
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21420/1/be/src/catalog/catalogd-main.cc
File be/src/catalog/catalogd-main.cc:

http://gerrit.cloudera.org:8080/#/c/21420/1/be/src/catalog/catalogd-main.cc@87
PS1, Line 87:   // Mark this as an internal service to use a more permission 
Thrift max message size
nit: permissive


http://gerrit.cloudera.org:8080/#/c/21420/1/be/src/common/init.cc
File be/src/common/init.cc:

http://gerrit.cloudera.org:8080/#/c/21420/1/be/src/common/init.cc@548
PS1, Line 548: FLAGS_thrift_external_rpc_max_message_size < 
ThriftDefaultMaxMessageSize()
> This comment should go for FLAGS_thrift_rpc_max_message_size instead.
I don't understand these checks and error message. Why do we force the 
configuration to be -1/0 or exceed DEFAULT_MAX_MESSAGE_SIZE?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9a649ef49a8a99c7bd9a1b73c37c4c621661311
Gerrit-Change-Number: 21420
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Tue, 14 May 2024 18:55:58 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-13020 (part 1): Change thrift rpc max message size to int64 t

2024-05-14 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21367 )

Change subject: IMPALA-13020 (part 1): Change thrift_rpc_max_message_size to 
int64_t
..


Patch Set 5:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/21367/5/be/src/rpc/thrift-util.cc@63
PS5, Line 63: "Default to the upper limit of 2147483647 bytes (~2GB). "
"Default to the upper limit" seems misleading, as it's no longer the upper 
limit for this option.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
Gerrit-Change-Number: 21367
Gerrit-PatchSet: 5
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Comment-Date: Tue, 14 May 2024 17:29:40 +
Gerrit-HasComments: Yes


[native-toolchain-CR] IMPALA-13072: Add retries for s3 uploads to combat flakiness

2024-05-14 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21421 )

Change subject: IMPALA-13072: Add retries for s3 uploads to combat flakiness
..


Patch Set 1:

https://gerrit.cloudera.org/c/21341/ was also added to try and address this.


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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95d858c99e965730303c2bfd90478ac5f68acf83
Gerrit-Change-Number: 21421
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Smith 
Gerrit-Comment-Date: Tue, 14 May 2024 16:57:06 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote

2024-05-08 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21383 )

Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
..


Patch Set 5: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21383/5/be/src/runtime/query-state.h
File be/src/runtime/query-state.h:

http://gerrit.cloudera.org:8080/#/c/21383/5/be/src/runtime/query-state.h@328
PS5, Line 328: BackendExecState exec_state = backend_exec_state_;
> backend_exec_state_ can only transition to terminal state once and wont rev
Ack



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9
Gerrit-Change-Number: 21383
Gerrit-PatchSet: 5
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Kurt Deschler 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Wed, 08 May 2024 20:41:53 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote

2024-05-08 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21383 )

Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21383/5/be/src/runtime/query-state.h
File be/src/runtime/query-state.h:

http://gerrit.cloudera.org:8080/#/c/21383/5/be/src/runtime/query-state.h@328
PS5, Line 328: BackendExecState exec_state = backend_exec_state_;
Shouldn't this be atomic if we need to worry about it's value changing? Or take 
backend_exec_state_lock_?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9
Gerrit-Change-Number: 21383
Gerrit-PatchSet: 5
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Kurt Deschler 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Wed, 08 May 2024 20:21:25 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote

2024-05-08 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21383 )

Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21383/4/be/src/util/network-util.h
File be/src/util/network-util.h:

http://gerrit.cloudera.org:8080/#/c/21383/4/be/src/util/network-util.h@124
PS4, Line 124: comp = lhs.uds_address().compare(rhs.uds_address());
What if only one has_uds_address? Seems like they should be unequal then.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9
Gerrit-Change-Number: 21383
Gerrit-PatchSet: 4
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Kurt Deschler 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Wed, 08 May 2024 18:19:28 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters

2024-05-08 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21131 )

Change subject: IMPALA-11499: Refactor UrlEncode function to handle special 
characters
..


Patch Set 27: Verified+1 Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb
Gerrit-Change-Number: 21131
Gerrit-PatchSet: 27
Gerrit-Owner: Anonymous Coward 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Zihao Ye 
Gerrit-Comment-Date: Wed, 08 May 2024 18:15:52 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters

2024-05-08 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21131 )

Change subject: IMPALA-11499: Refactor UrlEncode function to handle special 
characters
..


Patch Set 26:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21131/26/tests/query_test/test_insert.py
File tests/query_test/test_insert.py:

http://gerrit.cloudera.org:8080/#/c/21131/26/tests/query_test/test_insert.py@343
PS26, Line 343: =
> flake8: E225 missing whitespace around operator
This.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb
Gerrit-Change-Number: 21131
Gerrit-PatchSet: 26
Gerrit-Owner: Anonymous Coward 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Zihao Ye 
Gerrit-Comment-Date: Wed, 08 May 2024 18:01:35 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote

2024-05-07 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21383 )

Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
..


Patch Set 3: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9
Gerrit-Change-Number: 21383
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Tue, 07 May 2024 22:09:08 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote

2024-05-07 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21383 )

Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
..


Patch Set 2: Code-Review+1

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21383/2/be/src/service/data-stream-service.cc
File be/src/service/data-stream-service.cc:

http://gerrit.cloudera.org:8080/#/c/21383/2/be/src/service/data-stream-service.cc@141
PS2, Line 141:   int32_t min_wait_time_ms = 500;
nit: could be useful as a hidden config option.


http://gerrit.cloudera.org:8080/#/c/21383/2/tests/custom_cluster/test_runtime_filter_aggregation.py
File tests/custom_cluster/test_runtime_filter_aggregation.py:

http://gerrit.cloudera.org:8080/#/c/21383/2/tests/custom_cluster/test_runtime_filter_aggregation.py@115
PS2, Line 115: #The JOIN BUILD fragment in first anc third impalads 
immediately receive EOS
typo: anc -> and



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9
Gerrit-Change-Number: 21383
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Tue, 07 May 2024 21:25:56 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters

2024-05-07 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21131 )

Change subject: IMPALA-11499: Refactor UrlEncode function to handle special 
characters
..


Patch Set 25: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb
Gerrit-Change-Number: 21131
Gerrit-PatchSet: 25
Gerrit-Owner: Anonymous Coward 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Zihao Ye 
Gerrit-Comment-Date: Tue, 07 May 2024 20:34:57 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-13061: Create query live as external table

2024-05-07 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21401 )

Change subject: IMPALA-13061: Create query live as external table
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21401/3/tests/custom_cluster/test_query_live.py
File tests/custom_cluster/test_query_live.py:

http://gerrit.cloudera.org:8080/#/c/21401/3/tests/custom_cluster/test_query_live.py@175
PS3, Line 175: 
"--use_local_catalog=true",
 : 
catalogd_args="--enable_workload_mgmt "
 :
> I can't actually find a way to detect "transactional" from "DESCRIBE EXTEND
Switched to CREATE EXTERNAL TABLE, I think that makes sense for System Tables 
because Hive can't manage them, and with these options Hive was converting them 
to external table with purge=true.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie60a2bd03fabc63c85bcd9fa2489e9d47cd2aa65
Gerrit-Change-Number: 21401
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Tue, 07 May 2024 18:57:58 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-13061: Create query live as external table

2024-05-07 Thread Michael Smith (Code Review)
Hello Quanlong Huang, Andrew Sherman, Riza Suminto, Jason Fehr, Impala Public 
Jenkins,

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

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

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

Change subject: IMPALA-13061: Create query live as external table
..

IMPALA-13061: Create query live as external table

Impala determines whether a managed table is transactional based on the
'transactional' table property. It assumes any managed table with
transactional=true returns non-null getValidWriteIds.

When 'default_transactional_type=insert_only' is set at startup (via
default_query_options), impala_query_live is created as a managed table
with transactional=true, but SystemTables don't implement
getValidWriteIds and are not meant to be transactional.

DataSourceTable has a similar problem, and when a JDBC table is
created setJdbcDataSourceProperties sets transactional=false. This
patch uses CREATE EXTERNAL TABLE sys.impala_Query_live so that it is not
created as a managed table and 'transactional' is not set. That avoids
creating a SystemTable that Impala can't read (it encounters an
IllegalStateException).

Change-Id: Ie60a2bd03fabc63c85bcd9fa2489e9d47cd2aa65
---
M be/src/service/workload-management.cc
M tests/custom_cluster/test_query_live.py
2 files changed, 33 insertions(+), 5 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie60a2bd03fabc63c85bcd9fa2489e9d47cd2aa65
Gerrit-Change-Number: 21401
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 


[Impala-ASF-CR] IMPALA-13061: Set transactional�lse on query live table

2024-05-07 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21401 )

Change subject: IMPALA-13061: Set transactional=false on query live table
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21401/3/tests/custom_cluster/test_query_live.py
File tests/custom_cluster/test_query_live.py:

http://gerrit.cloudera.org:8080/#/c/21401/3/tests/custom_cluster/test_query_live.py@175
PS3, Line 175: esult = self.client.execute("select * from functional.alltypes",
 : fetch_profile_after_close=True)
 :
> Can you also assert that 'transactional'='false' in table properties via "D
I can't actually find a way to detect "transactional" from "DESCRIBE EXTENDED" 
output. But maybe the right thing to do is "CREATE EXTERNAL TABLE", which 
ensures its not a managed table.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie60a2bd03fabc63c85bcd9fa2489e9d47cd2aa65
Gerrit-Change-Number: 21401
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Tue, 07 May 2024 17:52:00 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-13061: Set transactional�lse on query live table

2024-05-07 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21401 )

Change subject: IMPALA-13061: Set transactional=false on query live table
..


Patch Set 3:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/21401/1//COMMIT_MSG@10
PS1, Line 10: 'transactional' table property. It assumes any table with
> Is the point here is that if default_transactional_type=insert_only then th
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie60a2bd03fabc63c85bcd9fa2489e9d47cd2aa65
Gerrit-Change-Number: 21401
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Tue, 07 May 2024 16:43:19 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-13061: Set transactional�lse on query live table

2024-05-07 Thread Michael Smith (Code Review)
Hello Quanlong Huang, Andrew Sherman, Riza Suminto, Jason Fehr, Impala Public 
Jenkins,

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

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

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

Change subject: IMPALA-13061: Set transactional=false on query live table
..

IMPALA-13061: Set transactional=false on query live table

Impala determines whether a table is transactional based solely on the
'transactional' table property. It assumes any table with
transactional=true returns non-null getValidWriteIds.

When 'default_transactional_type=insert_only' is set at startup (via
default_query_options), impala_query_live is created with
transactional=true, but SystemTables don't implement getValidWriteIds
and are not meant to be transactional.

DataSourceTable has a similar problem, and when a JDBC table is
created setJdbcDataSourceProperties sets transactional=false. This
patch sets 'transactional'='false' in tblproperties for
impala_query_live to avoid creating a table that Impala can't read
(it encounters an IllegalStateException).

Change-Id: Ie60a2bd03fabc63c85bcd9fa2489e9d47cd2aa65
---
M be/src/service/workload-management.cc
M tests/custom_cluster/test_query_live.py
2 files changed, 18 insertions(+), 1 deletion(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie60a2bd03fabc63c85bcd9fa2489e9d47cd2aa65
Gerrit-Change-Number: 21401
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 


[Impala-ASF-CR] IMPALA-13061: Set transactional�lse on query live table

2024-05-07 Thread Michael Smith (Code Review)
Hello Quanlong Huang, Andrew Sherman, Riza Suminto, Jason Fehr, Impala Public 
Jenkins,

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

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

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

Change subject: IMPALA-13061: Set transactional=false on query live table
..

IMPALA-13061: Set transactional=false on query live table

Impala determines whether a table is transactional based solely on the
'transactional' table property. It assumes any table with
transactional=true returns non-null getValidWriteIds.

When 'default_transactional_type=insert_only' is set at startup (via
default_query_options), impala_query_live is created with
transactional=true, but SystemTables don't implement getValidWriteIds
and are not meant to be transactional.

DataSourceTables have a similar problem, and when a JDBC table is
created setJdbcDataSourceProperties sets transactional=false. This
patch sets 'transactional'='false' in tblproperties for
impala_query_live to avoid creating a table that Impala can't read
(it encounters an IllegalStateException).

Change-Id: Ie60a2bd03fabc63c85bcd9fa2489e9d47cd2aa65
---
M be/src/service/workload-management.cc
M tests/custom_cluster/test_query_live.py
2 files changed, 18 insertions(+), 1 deletion(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie60a2bd03fabc63c85bcd9fa2489e9d47cd2aa65
Gerrit-Change-Number: 21401
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 


[Impala-ASF-CR] IMPALA-13061: Set transactional�lse on query live table

2024-05-06 Thread Michael Smith (Code Review)
Michael Smith has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/21401


Change subject: IMPALA-13061: Set transactional=false on query live table
..

IMPALA-13061: Set transactional=false on query live table

Sets 'transactional'='false' in tblproperties for query live table to
avoid creating a table that Impala can't read. Transactional tables are
assumed to return valid getValidWriteIds, but SystemTables don't.

Change-Id: Ie60a2bd03fabc63c85bcd9fa2489e9d47cd2aa65
---
M be/src/service/workload-management.cc
M tests/custom_cluster/test_query_live.py
2 files changed, 18 insertions(+), 1 deletion(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie60a2bd03fabc63c85bcd9fa2489e9d47cd2aa65
Gerrit-Change-Number: 21401
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith 


[Impala-ASF-CR] IMPALA-13051: Speed up, refactor query log tests

2024-05-06 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21358 )

Change subject: IMPALA-13051: Speed up, refactor query log tests
..


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21358/6/tests/util/workload_management.py
File tests/util/workload_management.py:

http://gerrit.cloudera.org:8080/#/c/21358/6/tests/util/workload_management.py@26
PS6, Line 26: from SystemTables.ttypes import TQueryTableColumn
> While these changes simplify the code quite a bit (20% or so), they also re
The last addition (line 573) is there to ensure all columns are tested.

I agree that duplicating the column names helps reinforce they should not be 
changed, but feels kind of excessive. Protocol definitions usually have the 
ability to break things if you change existing fields.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1127ef041a3e024bf2b262767d56ec5f29bf3855
Gerrit-Change-Number: 21358
Gerrit-PatchSet: 6
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Mon, 06 May 2024 23:06:53 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-13051: Speed up, refactor query log tests

2024-05-06 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21358 )

Change subject: IMPALA-13051: Speed up, refactor query log tests
..


Patch Set 6:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/21358/5/tests/common/custom_cluster_test_suite.py
File tests/common/custom_cluster_test_suite.py:

http://gerrit.cloudera.org:8080/#/c/21358/5/tests/common/custom_cluster_test_suite.py@161
PS5, Line 161:   # IMPALA-13051: Add faster default graceful shutdown 
options before processing
> Can you mention the Jira? It may not be evident for the reader why this is
Done


http://gerrit.cloudera.org:8080/#/c/21358/5/tests/common/custom_cluster_test_suite.py@236
PS5, Line 236:
> I'll probably back this change out, it's weird but doesn't cause issues rig
Done


http://gerrit.cloudera.org:8080/#/c/21358/5/tests/custom_cluster/test_query_log.py
File tests/custom_cluster/test_query_log.py:

http://gerrit.cloudera.org:8080/#/c/21358/5/tests/custom_cluster/test_query_log.py@411
PS5, Line 411:   """Tests to assert that query_log_table_name works with 
non-default value."""
> nit: I was confused at first that this means that table table can be rename
Done


http://gerrit.cloudera.org:8080/#/c/21358/5/tests/util/workload_management.py
File tests/util/workload_management.py:

http://gerrit.cloudera.org:8080/#/c/21358/5/tests/util/workload_management.py@93
PS5, Line 93:   session_id = re.search(r'\n\s+Session ID:\s+(.*)\n', 
profile_text)
> An idea to make these even more compact:
Haven't had the bandwidth to look at this more. Seems reasonable, but also 
triggers a significant amount of other cleanup in this file that will take me a 
little while.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1127ef041a3e024bf2b262767d56ec5f29bf3855
Gerrit-Change-Number: 21358
Gerrit-PatchSet: 6
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Mon, 06 May 2024 22:09:17 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-13051: Speed up, refactor query log tests

2024-05-06 Thread Michael Smith (Code Review)
Hello Quanlong Huang, Andrew Sherman, Riza Suminto, Jason Fehr, Csaba 
Ringhofer, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-13051: Speed up, refactor query log tests
..

IMPALA-13051: Speed up, refactor query log tests

Sets faster default shutdown_grace_period_s and shutdown_deadline_s when
impalad_graceful_shutdown=True in tests. Impala waits until grace period
has passed and all queries are stopped (or deadline is exceeded) before
flushing the query log, so grace period of 0 is sufficient. Adds them in
setup_method to reduce duplication in test declarations.

Re-uses TQueryTableColumn Thrift definitions for testing.

Moves waiting for query log table to exist to setup_method rather than
as a side-effect of get_client.

Refactors workload management code to reduce if-clause nesting.

Change-Id: I1127ef041a3e024bf2b262767d56ec5f29bf3855
---
M be/src/service/workload-management.cc
M tests/common/custom_cluster_test_suite.py
M tests/custom_cluster/test_query_live.py
M tests/custom_cluster/test_query_log.py
M tests/util/workload_management.py
5 files changed, 332 insertions(+), 494 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1127ef041a3e024bf2b262767d56ec5f29bf3855
Gerrit-Change-Number: 21358
Gerrit-PatchSet: 6
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 


[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters

2024-05-06 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21131 )

Change subject: IMPALA-11499: Refactor UrlEncode function to handle special 
characters
..


Patch Set 20: Code-Review+1

(3 comments)

http://gerrit.cloudera.org:8080/#/c/21131/19/be/src/util/coding-util-test.cc
File be/src/util/coding-util-test.cc:

http://gerrit.cloudera.org:8080/#/c/21131/19/be/src/util/coding-util-test.cc@97
PS19, Line 97: 
"\x12\x13\x14\x15\x16\x17\x18\x19\x1A\x1B\x1C\x1D\x1E\x1F\x7F\"#%'"
> Why is the single quote escaped?
Done


http://gerrit.cloudera.org:8080/#/c/21131/19/tests/query_test/test_insert.py
File tests/query_test/test_insert.py:

http://gerrit.cloudera.org:8080/#/c/21131/19/tests/query_test/test_insert.py@302
PS19, Line 302:  
'\x12\x13\x14\x15\x16\x17\x18\x19\x1A\x1B\x1C\x1D\x1E\x1F\"\x7F''()%*/:=?{[]#')
> This looks like you forgot to escape the double quote. The rest are omitted
Done


http://gerrit.cloudera.org:8080/#/c/21131/20/tests/query_test/test_insert.py
File tests/query_test/test_insert.py:

http://gerrit.cloudera.org:8080/#/c/21131/20/tests/query_test/test_insert.py@306
PS20, Line 306:   self.execute_query("insert into {} partition(p='') 
values (0)".format(tbl))
Can you add a comment about why splitting this out was necessary?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb
Gerrit-Change-Number: 21131
Gerrit-PatchSet: 20
Gerrit-Owner: Anonymous Coward 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Zihao Ye 
Gerrit-Comment-Date: Mon, 06 May 2024 21:21:47 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-13020: Change thrift rpc max message size to int64 t

2024-05-06 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21367 )

Change subject: IMPALA-13020: Change thrift_rpc_max_message_size to int64_t
..


Patch Set 4:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/21367/4/be/src/rpc/thrift-util.cc@61
PS4, Line 61: DEFINE_int64(thrift_rpc_max_message_size, 4L * 1024 * 1024 * 1024,
> Is 8GB reasonable?
Could we split up those limits?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
Gerrit-Change-Number: 21367
Gerrit-PatchSet: 4
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Comment-Date: Mon, 06 May 2024 18:42:42 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-13054: Avoid revisiting children in QueryStateExpanded

2024-05-03 Thread Michael Smith (Code Review)
Hello Andrew Sherman, Riza Suminto, Jason Fehr, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-13054: Avoid revisiting children in QueryStateExpanded
..

IMPALA-13054: Avoid revisiting children in QueryStateExpanded

Avoids revisiting children in QueryStateExpanded due to duplicate
recursion. Since process_exec_profile visits children recursively, we
only want immediate children from GetChildren, not all children from
GetAllChildren.

Adjusts HDFS_SCAN_NODE test to look back at top of stack, rather than
depend on a specific depth that was caused by calling GetAllChildren
from the root.

This was identified by running the Impala E2E test suite with workload
management enabled, where
test_nested_types.py::TestMaxNestingDepth::test_max_nesting_depth took
multiple hours to run and blocked the FinishUnregisterQuery for other
tests.

Testing:
- Manual test with enable_workload_mgmt=true running
  test_max_nesting_depth.

Change-Id: Ia36d53e52095d487ffb3a162fc723f1e815bec11
---
M be/src/service/query-state-record.cc
1 file changed, 23 insertions(+), 11 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia36d53e52095d487ffb3a162fc723f1e815bec11
Gerrit-Change-Number: 21392
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Riza Suminto 


[Impala-ASF-CR] IMPALA-13051: Speed up, refactor query log tests

2024-05-03 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21358 )

Change subject: IMPALA-13051: Speed up, refactor query log tests
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21358/5/tests/common/custom_cluster_test_suite.py
File tests/common/custom_cluster_test_suite.py:

http://gerrit.cloudera.org:8080/#/c/21358/5/tests/common/custom_cluster_test_suite.py@236
PS5, Line 236: super(CustomClusterTestSuite, self).teardown_class()
> Should it be called the very last? I see couple custom cluster tests overri
I'll probably back this change out, it's weird but doesn't cause issues right 
now.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1127ef041a3e024bf2b262767d56ec5f29bf3855
Gerrit-Change-Number: 21358
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Fri, 03 May 2024 20:29:03 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-13054: Avoid revisiting children in QueryStateExpanded

2024-05-02 Thread Michael Smith (Code Review)
Michael Smith has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/21392


Change subject: IMPALA-13054: Avoid revisiting children in QueryStateExpanded
..

IMPALA-13054: Avoid revisiting children in QueryStateExpanded

Avoids revisiting children in QueryStateExpanded due to duplicate
recursion. Since process_exec_profile visits children recursively, we
only want immediate children from GetChildren, not all children from
GitAllChildren.

Adjusts HDFS_SCAN_NODE test to look back at top of stack, rather than
depend on a specific depth that was caused by calling GetAllChildren
from the root.

This was identified by running the Impala E2E test suite with workload
management enabled, where
test_nested_types.py::TestMaxNestingDepth::test_max_nesting_depth took
multiple hours to run and blocked the FinishUnregisterQuery for other
tests.

Testing:
- Manual test with enable_workload_mgmt=true running
  test_max_nesting_depth.

Change-Id: Ia36d53e52095d487ffb3a162fc723f1e815bec11
---
M be/src/service/query-state-record.cc
1 file changed, 23 insertions(+), 11 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia36d53e52095d487ffb3a162fc723f1e815bec11
Gerrit-Change-Number: 21392
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith 


[Impala-ASF-CR] IMPALA-13053: Update test to use ORC files

2024-05-02 Thread Michael Smith (Code Review)
Michael Smith has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/21391


Change subject: IMPALA-13053: Update test to use ORC files
..

IMPALA-13053: Update test to use ORC files

Updates test_max_nesting_depth to use precreated ORC files like the
Parquet version to reduce runtime rather than using Hive to generate ORC
from the Parquet files. This reduces each test run by almost 3 minutes.

Change-Id: I2f5bdbb86af0e651d189217a18882d5eda1098d5
---
M testdata/max_nesting_depth/README
A testdata/max_nesting_depth/orc/int_array/file.orc
A testdata/max_nesting_depth/orc/int_map/file.orc
A testdata/max_nesting_depth/orc/struct/file.orc
A testdata/max_nesting_depth/orc/struct_array/file.orc
A testdata/max_nesting_depth/orc/struct_map/file.orc
R testdata/max_nesting_depth/parquet/int_array/file.parq
R testdata/max_nesting_depth/parquet/int_map/file.parq
R testdata/max_nesting_depth/parquet/struct/file.parq
R testdata/max_nesting_depth/parquet/struct_array/file.parq
R testdata/max_nesting_depth/parquet/struct_map/file.parq
M tests/query_test/test_nested_types.py
12 files changed, 17 insertions(+), 24 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2f5bdbb86af0e651d189217a18882d5eda1098d5
Gerrit-Change-Number: 21391
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith 


[Impala-ASF-CR] IMPALA-13044: Upgrade bouncycastle to 1.78

2024-05-02 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21371 )

Change subject: IMPALA-13044: Upgrade bouncycastle to 1.78
..


Patch Set 6: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8372916ab79b863e7a07d22e8333abd54492fa29
Gerrit-Change-Number: 21371
Gerrit-PatchSet: 6
Gerrit-Owner: Peter Rozsa 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Comment-Date: Thu, 02 May 2024 19:08:23 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-13051: Speed up, refactor query log tests

2024-05-02 Thread Michael Smith (Code Review)
Hello Riza Suminto, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-13051: Speed up, refactor query log tests
..

IMPALA-13051: Speed up, refactor query log tests

Sets faster default shutdown_grace_period_s and shutdown_deadline_s when
impalad_graceful_shutdown=True in tests. Impala waits until grace period
has passed and all queries are stopped (or deadline is exceeded) before
flushing the query log, so grace period of 0 is sufficient. Adds them in
setup_method to reduce duplication in test declarations.

Reorders CustomClusterTestSuite.teardown_method to stop clients, then
Impala, then restart Hive in reverse of the order they're setup in.

Re-uses TQueryTableColumn Thrift definitions for testing.

Moves waiting for query log table to exist to setup_method rather than
as a side-effect of get_client.

Refactors workload management code to reduce if-clause nesting.

Change-Id: I1127ef041a3e024bf2b262767d56ec5f29bf3855
---
M be/src/service/workload-management.cc
M tests/common/custom_cluster_test_suite.py
M tests/custom_cluster/test_query_live.py
M tests/custom_cluster/test_query_log.py
M tests/util/workload_management.py
5 files changed, 337 insertions(+), 502 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1127ef041a3e024bf2b262767d56ec5f29bf3855
Gerrit-Change-Number: 21358
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Riza Suminto 


[Impala-ASF-CR] IMPALA-13051: Speed up, refactor query log tests

2024-05-02 Thread Michael Smith (Code Review)
Hello Riza Suminto, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-13051: Speed up, refactor query log tests
..

IMPALA-13051: Speed up, refactor query log tests

Sets faster default shutdown_grace_period_s and shutdown_deadline_s when
impalad_graceful_shutdown=True in tests. Impala waits until grace period
has passed and all queries are stopped (or deadline is exceeded) before
flushing the query log, so grace period of 0 is sufficient. Adds them in
setup_method to reduce duplication in test declarations.

Reorders CustomClusterTestSuite.teardown_method to stop clients, then
Impala, then restart Hive in reverse of the order they're setup in.

Re-uses TQueryTableColumn Thrift definitions for testing.

Moves waiting for query log table to exist to setup_method rather than
as a side-effect of get_client.

Refactors workload management code to reduce if-clause nesting.

Change-Id: I1127ef041a3e024bf2b262767d56ec5f29bf3855
---
M be/src/service/workload-management.cc
M tests/common/custom_cluster_test_suite.py
M tests/custom_cluster/test_query_live.py
M tests/custom_cluster/test_query_log.py
M tests/util/workload_management.py
5 files changed, 336 insertions(+), 502 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1127ef041a3e024bf2b262767d56ec5f29bf3855
Gerrit-Change-Number: 21358
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Riza Suminto 


[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote

2024-05-02 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21383 )

Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21383/1/be/src/runtime/query-exec-mgr.cc
File be/src/runtime/query-exec-mgr.cc:

http://gerrit.cloudera.org:8080/#/c/21383/1/be/src/runtime/query-exec-mgr.cc@85
PS1, Line 85:   {FLAGS_krpc_port == 27000 ? "coordinator" : "executor"}));
> This test is tricky because:
Maybe we need to add a query option to fix those for testing so we can have 
deterministic testing?

We probably also want tests that are indeterministic since the feature itself 
is, and have probabilistic coverage. So test needs to be flexible enough to 
handle different scenarios.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9
Gerrit-Change-Number: 21383
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Thu, 02 May 2024 16:10:34 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote

2024-05-01 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21383 )

Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
..


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/21383/1/be/src/runtime/query-exec-mgr.cc
File be/src/runtime/query-exec-mgr.cc:

http://gerrit.cloudera.org:8080/#/c/21383/1/be/src/runtime/query-exec-mgr.cc@85
PS1, Line 85:   {FLAGS_krpc_port == 27000 ? "coordinator" : "executor"}));
This seems like an odd way to check when is_coordinator and is_executor exist.


http://gerrit.cloudera.org:8080/#/c/21383/1/be/src/service/data-stream-service.cc
File be/src/service/data-stream-service.cc:

http://gerrit.cloudera.org:8080/#/c/21383/1/be/src/service/data-stream-service.cc@126
PS1, Line 126: const UpdateFilterParamsPB* req, UpdateFilterResultPB* resp, 
RpcContext* context) {
What thread does this run in? Do we need to worry about it blocking a queue?


http://gerrit.cloudera.org:8080/#/c/21383/1/be/src/service/data-stream-service.cc@166
PS1, Line 166:   PrintId(ProtoToQueryId(req->query_id())), query_found ? 
"has done" : "not found",
I'm not clear what "Query State for query_id=... has done after N ms" would 
mean.

I guess "done" is a state? Usually not used in that tense, so maybe "no longer 
running".


http://gerrit.cloudera.org:8080/#/c/21383/1/tests/query_test/test_runtime_filters.py
File tests/query_test/test_runtime_filters.py:

http://gerrit.cloudera.org:8080/#/c/21383/1/tests/query_test/test_runtime_filters.py@555
PS1, Line 555: """Test that distributed runtime filter aggregation still 
works
Can we also test for the log message? assert_impalad_log_contains would work. 
Looks like it might need a separate test case with different parameters.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9
Gerrit-Change-Number: 21383
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Wed, 01 May 2024 23:05:28 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-13020: Change thrift rpc max message size to int64 t

2024-05-01 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21367 )

Change subject: IMPALA-13020: Change thrift_rpc_max_message_size to int64_t
..


Patch Set 4: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
Gerrit-Change-Number: 21367
Gerrit-PatchSet: 4
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Comment-Date: Wed, 01 May 2024 20:49:54 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-13020: Change thrift rpc max message size to int64 t

2024-05-01 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21367 )

Change subject: IMPALA-13020: Change thrift_rpc_max_message_size to int64_t
..


Patch Set 3: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21367/3/be/src/rpc/TAcceptQueueServer.cpp
File be/src/rpc/TAcceptQueueServer.cpp:

http://gerrit.cloudera.org:8080/#/c/21367/3/be/src/rpc/TAcceptQueueServer.cpp@94
PS3, Line 94:   std::string(ttx.what()).find("MaxMessageSize") != 
std::string::npos) {
Could we give a recommendation to increase thrift_rpc_max_message_size?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
Gerrit-Change-Number: 21367
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Comment-Date: Wed, 01 May 2024 20:16:52 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12934: Added Calcite parsing files to Impala

2024-05-01 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21194 )

Change subject: IMPALA-12934: Added Calcite parsing files to Impala
..


Patch Set 8: Code-Review+2

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/21194/5//COMMIT_MSG@14
PS5, Line 14: on top of the Parser.jj file or the config.fmpp file in the 
future are Impala
> Done
Ack



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If756b5ea8beb85661a30fb5d029e74ebb6719767
Gerrit-Change-Number: 21194
Gerrit-PatchSet: 8
Gerrit-Owner: Steve Carlin 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Steve Carlin 
Gerrit-Comment-Date: Wed, 01 May 2024 17:44:23 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-13044: Upgrade bouncycastle to 1.78

2024-05-01 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21371 )

Change subject: IMPALA-13044: Upgrade bouncycastle to 1.78
..


Patch Set 4: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8372916ab79b863e7a07d22e8333abd54492fa29
Gerrit-Change-Number: 21371
Gerrit-PatchSet: 4
Gerrit-Owner: Peter Rozsa 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Comment-Date: Wed, 01 May 2024 17:29:50 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12910: Support running TPCH/TPCDS queries for JDBC tables

2024-04-30 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21304 )

Change subject: IMPALA-12910: Support running TPCH/TPCDS queries for JDBC tables
..


Patch Set 18: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I44e8c1bb020e90559c7f22483a7ab7a151b8f48a
Gerrit-Change-Number: 21304
Gerrit-PatchSet: 18
Gerrit-Owner: Wenzhe Zhou 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: gaurav singh 
Gerrit-Comment-Date: Tue, 30 Apr 2024 23:36:17 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-13049: Add dependency management for log4j2 to use 2.18.0

2024-04-30 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21379 )

Change subject: IMPALA-13049: Add dependency management for log4j2 to use 2.18.0
..


Patch Set 1: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4f8485adadb90f66f354a5dedca29992c6d4e6f
Gerrit-Change-Number: 21379
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Comment-Date: Tue, 30 Apr 2024 21:00:06 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-13044: Upgrade bouncycastle to 1.78

2024-04-30 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21371 )

Change subject: IMPALA-13044: Upgrade bouncycastle to 1.78
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21371/3/fe/pom.xml
File fe/pom.xml:

http://gerrit.cloudera.org:8080/#/c/21371/3/fe/pom.xml@603
PS3, Line 603:   org.bouncycastle
Keycloak used a similar approach in 
https://github.com/keycloak/keycloak/pull/21543. I think this is probably ok if 
it passes tests. We seem to have some test coverage in test_saml2_sso.py.

However I think we need an entry for this in "enforce-banned-dependencies". 
When I look at

  mvn -f fe dependency:tree -Dscope=compile

it looks ok. Test dependencies look like they might still pull in some other 
versions though.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8372916ab79b863e7a07d22e8333abd54492fa29
Gerrit-Change-Number: 21371
Gerrit-PatchSet: 3
Gerrit-Owner: Peter Rozsa 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Comment-Date: Tue, 30 Apr 2024 20:44:02 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-13044: Upgrade bouncycastle to 1.78

2024-04-30 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21371 )

Change subject: IMPALA-13044: Upgrade bouncycastle to 1.78
..


Patch Set 2:

(1 comment)

I'd suggest running

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

http://gerrit.cloudera.org:8080/#/c/21371/1//COMMIT_MSG@11
PS1, Line 11: *-jdk18on.
> Thanks, I misspelled the artifact, and now I corrected it to jdk18on. I won
If this is a transitive dependency, I'd run 'mvn -f fe dependency:tree' and see 
what bouncycastle dependencies are used. If we want to actually force using 
newer bouncycastle we may need to explicitly exclude some references.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8372916ab79b863e7a07d22e8333abd54492fa29
Gerrit-Change-Number: 21371
Gerrit-PatchSet: 2
Gerrit-Owner: Peter Rozsa 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Comment-Date: Tue, 30 Apr 2024 17:35:03 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-13045: Wait for impala query live to exist

2024-04-30 Thread Michael Smith (Code Review)
Michael Smith has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/21372 )

Change subject: IMPALA-13045: Wait for impala_query_live to exist
..

IMPALA-13045: Wait for impala_query_live to exist

Waits for creation of 'sys.impala_query_live' in tests to ensure it has
been registered with HMS.

Change-Id: I5cc3fa3c43be7af9a5f097359a0d4f20d057a207
Reviewed-on: http://gerrit.cloudera.org:8080/21372
Reviewed-by: Impala Public Jenkins 
Tested-by: Michael Smith 
---
M tests/custom_cluster/test_query_live.py
1 file changed, 7 insertions(+), 0 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I5cc3fa3c43be7af9a5f097359a0d4f20d057a207
Gerrit-Change-Number: 21372
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Riza Suminto 


[Impala-ASF-CR] IMPALA-13045: Wait for impala query live to exist

2024-04-30 Thread Michael Smith (Code Review)
Michael Smith has removed a vote on this change.

Change subject: IMPALA-13045: Wait for impala_query_live to exist
..


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I5cc3fa3c43be7af9a5f097359a0d4f20d057a207
Gerrit-Change-Number: 21372
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Riza Suminto 


[Impala-ASF-CR] IMPALA-13045: Wait for impala query live to exist

2024-04-30 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21372 )

Change subject: IMPALA-13045: Wait for impala_query_live to exist
..


Patch Set 3: Verified+1

Ran into IMPALA-9441.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5cc3fa3c43be7af9a5f097359a0d4f20d057a207
Gerrit-Change-Number: 21372
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Tue, 30 Apr 2024 17:15:50 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-13045: Wait for impala query live to exist

2024-04-30 Thread Michael Smith (Code Review)
Michael Smith has removed a vote on this change.

Change subject: IMPALA-13045: Wait for impala_query_live to exist
..


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I5cc3fa3c43be7af9a5f097359a0d4f20d057a207
Gerrit-Change-Number: 21372
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Riza Suminto 


[Impala-ASF-CR] IMPALA-12910: Support running TPCH/TPCDS queries for JDBC tables

2024-04-29 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21304 )

Change subject: IMPALA-12910: Support running TPCH/TPCDS queries for JDBC tables
..


Patch Set 17: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I44e8c1bb020e90559c7f22483a7ab7a151b8f48a
Gerrit-Change-Number: 21304
Gerrit-PatchSet: 17
Gerrit-Owner: Wenzhe Zhou 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: gaurav singh 
Gerrit-Comment-Date: Mon, 29 Apr 2024 22:27:16 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-13045: Wait for impala query live to exist

2024-04-29 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21372 )

Change subject: IMPALA-13045: Wait for impala_query_live to exist
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21372/1/tests/custom_cluster/test_query_live.py
File tests/custom_cluster/test_query_live.py:

http://gerrit.cloudera.org:8080/#/c/21372/1/tests/custom_cluster/test_query_live.py@34
PS1, Line 34: def setup_method(self, method):
> That wouldn't hurt. It needs to run after the cluster has been started, whi
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5cc3fa3c43be7af9a5f097359a0d4f20d057a207
Gerrit-Change-Number: 21372
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Mon, 29 Apr 2024 20:56:33 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-13045: Wait for impala query live to exist

2024-04-29 Thread Michael Smith (Code Review)
Hello Andrew Sherman, Riza Suminto, Jason Fehr, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-13045: Wait for impala_query_live to exist
..

IMPALA-13045: Wait for impala_query_live to exist

Waits for creation of 'sys.impala_query_live' in tests to ensure it has
been registered with HMS.

Change-Id: I5cc3fa3c43be7af9a5f097359a0d4f20d057a207
---
M tests/custom_cluster/test_query_live.py
1 file changed, 7 insertions(+), 0 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5cc3fa3c43be7af9a5f097359a0d4f20d057a207
Gerrit-Change-Number: 21372
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Riza Suminto 


[Impala-ASF-CR] Refactor Workload Management

2024-04-29 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21358 )

Change subject: Refactor Workload Management
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21358/3/tests/custom_cluster/test_query_log.py
File tests/custom_cluster/test_query_log.py:

http://gerrit.cloudera.org:8080/#/c/21358/3/tests/custom_cluster/test_query_log.py@66
PS3, Line 66: create_re = r'\]\s+(\w+:\w+)\]\s+Analyzing query: CREATE 
TABLE IF NOT EXISTS {}' \
Put this in setup_method.

Clean up ordering in CustomClusterTestSuite.teardown_method.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1127ef041a3e024bf2b262767d56ec5f29bf3855
Gerrit-Change-Number: 21358
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Mon, 29 Apr 2024 20:55:37 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-13046: Update Iceberg mixed format deletes test

2024-04-29 Thread Michael Smith (Code Review)
Michael Smith has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/21373


Change subject: IMPALA-13046: Update Iceberg mixed format deletes test
..

IMPALA-13046: Update Iceberg mixed format deletes test

Updates iceberg-mixed-format-position-deletes.test for HIVE-28069. Newer
versions of Hive will now remove a data file if a delete would negate
all rows in the data file to reduce the number of small files produced.
The test now ensures every data file it expects to produce will have a
row after delete (or circumvent the merge logic by using different
formats).

Change-Id: I87c23cc541983223c6b766372f4e582c33ae6836
---
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-mixed-format-position-deletes.test
1 file changed, 6 insertions(+), 11 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I87c23cc541983223c6b766372f4e582c33ae6836
Gerrit-Change-Number: 21373
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith 


[Impala-ASF-CR] IMPALA-13045: Wait for impala query live to exist

2024-04-29 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21372 )

Change subject: IMPALA-13045: Wait for impala_query_live to exist
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21372/1/tests/custom_cluster/test_query_live.py
File tests/custom_cluster/test_query_live.py:

http://gerrit.cloudera.org:8080/#/c/21372/1/tests/custom_cluster/test_query_live.py@34
PS1, Line 34: def wait_for_create_table(self, table_name):
> Should this be an override of setup_method() instead, with table_name fixed
That wouldn't hurt. It needs to run after the cluster has been started, which 
seems doable as long as it calls the parent setup_method first.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5cc3fa3c43be7af9a5f097359a0d4f20d057a207
Gerrit-Change-Number: 21372
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Mon, 29 Apr 2024 20:07:38 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-10729: [DOCS] Document SHA1 and SHA2 functions

2024-04-29 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17551 )

Change subject: IMPALA-10729: [DOCS] Document SHA1 and SHA2 functions
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/17551/1/docs/topics/impala_functions.xml
File docs/topics/impala_functions.xml:

http://gerrit.cloudera.org:8080/#/c/17551/1/docs/topics/impala_functions.xml@70
PS1, Line 70:
Unnecessary indentation here. I cleaned it up.


http://gerrit.cloudera.org:8080/#/c/17551/3/docs/topics/impala_functions.xml
File docs/topics/impala_functions.xml:

http://gerrit.cloudera.org:8080/#/c/17551/3/docs/topics/impala_functions.xml@100
PS3, Line 100: 
It looks like we normally also add links to all functions in this table.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I379de6075af2168715e71edfec26265b6ff5eab7
Gerrit-Change-Number: 17551
Gerrit-PatchSet: 3
Gerrit-Owner: Shajini Thayasingh 
Gerrit-Reviewer: Amogh Margoor 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Comment-Date: Mon, 29 Apr 2024 19:39:48 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-10729: [DOCS] Document SHA1 and SHA2 functions

2024-04-29 Thread Michael Smith (Code Review)
Michael Smith has uploaded a new patch set (#3) to the change originally 
created by Shajini Thayasingh. ( http://gerrit.cloudera.org:8080/17551 )

Change subject: IMPALA-10729: [DOCS] Document SHA1 and SHA2 functions
..

IMPALA-10729: [DOCS] Document SHA1 and SHA2 functions

Introduced the new hash functions that were recently added.

Change-Id: I379de6075af2168715e71edfec26265b6ff5eab7
---
M docs/impala.ditamap
M docs/topics/impala_functions.xml
A docs/topics/impala_hash_functions.xml
3 files changed, 79 insertions(+), 0 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I379de6075af2168715e71edfec26265b6ff5eab7
Gerrit-Change-Number: 17551
Gerrit-PatchSet: 3
Gerrit-Owner: Shajini Thayasingh 
Gerrit-Reviewer: Amogh Margoor 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 


[Impala-ASF-CR] IMPALA-13045: Wait for impala query live to exist

2024-04-29 Thread Michael Smith (Code Review)
Michael Smith has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/21372


Change subject: IMPALA-13045: Wait for impala_query_live to exist
..

IMPALA-13045: Wait for impala_query_live to exist

Waits for creation of 'sys.impala_query_live' in tests where it may not
exist in HMS beforehand.

Change-Id: I5cc3fa3c43be7af9a5f097359a0d4f20d057a207
---
M tests/custom_cluster/test_query_live.py
1 file changed, 10 insertions(+), 0 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5cc3fa3c43be7af9a5f097359a0d4f20d057a207
Gerrit-Change-Number: 21372
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith 


[Impala-ASF-CR] PROTOTYPE: IMPALA-13020: Change thrift rpc max message size to int64 t

2024-04-29 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21367 )

Change subject: PROTOTYPE: IMPALA-13020: Change thrift_rpc_max_message_size to 
int64_t
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21367/2/fe/src/main/java/org/apache/impala/service/BackendConfig.java
File fe/src/main/java/org/apache/impala/service/BackendConfig.java:

http://gerrit.cloudera.org:8080/#/c/21367/2/fe/src/main/java/org/apache/impala/service/BackendConfig.java@411
PS2, Line 411: return (int) 
Math.min(backendCfg_.thrift_rpc_max_message_size, Integer.MAX_VALUE);
> No major reason. Mainly, I'm doing C++ because C++ is easy for us to patch
That'd be useful to clarify in the commit message.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
Gerrit-Change-Number: 21367
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Comment-Date: Mon, 29 Apr 2024 17:35:21 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-13044: Upgrade bouncycastle to 1.78

2024-04-29 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21371 )

Change subject: IMPALA-13044: Upgrade bouncycastle to 1.78
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/21371/1//COMMIT_MSG@11
PS1, Line 11: jdk15on18, which is the most similar to the previous release 
format.
Where did you find jdk15on18? I see jdk15to18, or jdk18on. jdk18on seems like a 
reasonable thing to upgrade to as we require Java 8+.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8372916ab79b863e7a07d22e8333abd54492fa29
Gerrit-Change-Number: 21371
Gerrit-PatchSet: 1
Gerrit-Owner: Peter Rozsa 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Comment-Date: Mon, 29 Apr 2024 17:34:45 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12934: Added Calcite parsing files to Impala

2024-04-29 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21194 )

Change subject: IMPALA-12934: Added Calcite parsing files to Impala
..


Patch Set 7: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If756b5ea8beb85661a30fb5d029e74ebb6719767
Gerrit-Change-Number: 21194
Gerrit-PatchSet: 7
Gerrit-Owner: Steve Carlin 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Steve Carlin 
Gerrit-Comment-Date: Mon, 29 Apr 2024 17:02:05 +
Gerrit-HasComments: No


[Impala-ASF-CR] PROTOTYPE: IMPALA-13020: Change thrift rpc max message size to int64 t

2024-04-29 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21367 )

Change subject: PROTOTYPE: IMPALA-13020: Change thrift_rpc_max_message_size to 
int64_t
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21367/2/fe/src/main/java/org/apache/impala/service/BackendConfig.java
File fe/src/main/java/org/apache/impala/service/BackendConfig.java:

http://gerrit.cloudera.org:8080/#/c/21367/2/fe/src/main/java/org/apache/impala/service/BackendConfig.java@411
PS2, Line 411: return (int) 
Math.min(backendCfg_.thrift_rpc_max_message_size, Integer.MAX_VALUE);
What's the rationale for not increasing it?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
Gerrit-Change-Number: 21367
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Comment-Date: Mon, 29 Apr 2024 16:47:43 +
Gerrit-HasComments: Yes


[native-toolchain-CR] IMPALA-13020: Use 64-bit integer for Thrift max message size on C++

2024-04-26 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21343 )

Change subject: IMPALA-13020: Use 64-bit integer for Thrift max message size on 
C++
..


Patch Set 3: Code-Review+1


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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I94fdd7b07fcc8dca0639839b40a9eff815090835
Gerrit-Change-Number: 21343
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Comment-Date: Sat, 27 Apr 2024 03:59:37 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-13012: Lower default query log max queued

2024-04-26 Thread Michael Smith (Code Review)
Michael Smith has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/21351 )

Change subject: IMPALA-13012: Lower default query_log_max_queued
..

IMPALA-13012: Lower default query_log_max_queued

Sets the query_log_max_queued default such that

  query_log_max_queued * num_columns(49) < statement_expression_limit

to avoid triggering e.g.

  AnalysisException: Exceeded the statement expression limit (25)
  Statement has 370039 expressions.

Also increases statement_expression_limit for insertion to avoid an
error if query_log_max_queued is changed.

Logs time taken to write to the queries table for help with debugging
and adds histogram "impala-server.completed-queries.write-durations".

Fixes InternalServer so it uses 'default_query_options'.

Change-Id: I6535675307d88cb65ba7d908f3c692e0cf3259d7
Reviewed-on: http://gerrit.cloudera.org:8080/21351
Reviewed-by: Michael Smith 
Tested-by: Michael Smith 
Reviewed-by: Riza Suminto 
---
M be/src/service/impala-server.h
M be/src/service/internal-server-test.cc
M be/src/service/internal-server.cc
M be/src/service/internal-server.h
M be/src/service/query-options.cc
M be/src/service/query-options.h
M be/src/service/workload-management-flags.cc
M be/src/service/workload-management.cc
M be/src/util/impalad-metrics.cc
M be/src/util/impalad-metrics.h
M common/thrift/SystemTables.thrift
M common/thrift/metrics.json
M tests/custom_cluster/test_query_log.py
13 files changed, 115 insertions(+), 71 deletions(-)

Approvals:
  Michael Smith: Looks good to me, but someone else must approve; Verified
  Riza Suminto: Looks good to me, approved

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I6535675307d88cb65ba7d908f3c692e0cf3259d7
Gerrit-Change-Number: 21351
Gerrit-PatchSet: 9
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Riza Suminto 


[Impala-ASF-CR] IMPALA-13012: Lower default query log max queued

2024-04-26 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21351 )

Change subject: IMPALA-13012: Lower default query_log_max_queued
..


Patch Set 8: Verified+1 Code-Review+1

Carry +1s after merging parent.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6535675307d88cb65ba7d908f3c692e0cf3259d7
Gerrit-Change-Number: 21351
Gerrit-PatchSet: 8
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Fri, 26 Apr 2024 23:24:36 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-13005: Create Query Live table in HMS

2024-04-26 Thread Michael Smith (Code Review)
Michael Smith has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/21302 )

Change subject: IMPALA-13005: Create Query Live table in HMS
..

IMPALA-13005: Create Query Live table in HMS

Creates the 'sys.impala_query_live' table in HMS using a similar 'CREATE
TABLE' command to 'sys.impala_query_log'. Updates frontend to identify a
System Table based on the '__IMPALA_SYSTEM_TABLE' property. Tables
improperly marked with '__IMPALA_SYSTEM_TABLE' will error when
attempting to scan them because no relevant scanner will be available.

Creating the table in HMS simplifies supporting 'SHOW CREATE TABLE' and
'DESCRIBE EXTENDED', so allows them for parity with Query Log.
Explicitly disables 'COMPUTE STATS' on system tables as it doesn't work
correctly.

Makes System Tables work with local catalog mode, fixing

  LocalCatalogException: Unknown table type for table sys.impala_query_live

Updates workload management implementation to rely more on
SystemTables.thrift definition, and adds DCHECKs to verify completeness
and ordering.

Testing:
- adds additional test cases for changes to introspection commands
- passes existing test_query_live and test_query_log suites

Change-Id: Idf302ee54a819fdee2db0ae582a5eeddffe4a5b4
Reviewed-on: http://gerrit.cloudera.org:8080/21302
Reviewed-by: Riza Suminto 
Tested-by: Impala Public Jenkins 
---
M be/generated-sources/gen-cpp/CMakeLists.txt
M be/src/exec/system-table-scanner.cc
M be/src/service/workload-management-fields.cc
M be/src/service/workload-management.cc
M be/src/service/workload-management.h
M common/thrift/CatalogObjects.thrift
M common/thrift/SystemTables.thrift
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowCreateTableStmt.java
A fe/src/main/java/org/apache/impala/analysis/SystemTableRef.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/Db.java
A fe/src/main/java/org/apache/impala/catalog/FeSystemTable.java
M fe/src/main/java/org/apache/impala/catalog/SystemTable.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
A fe/src/main/java/org/apache/impala/catalog/local/LocalSystemTable.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/main/java/org/apache/impala/planner/SystemTableScanNode.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/catalog/SystemTableTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
D 
testdata/workloads/functional-planner/queries/PlannerTest/impala-query-live.test
M tests/custom_cluster/test_query_live.py
26 files changed, 435 insertions(+), 294 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Idf302ee54a819fdee2db0ae582a5eeddffe4a5b4
Gerrit-Change-Number: 21302
Gerrit-PatchSet: 16
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Wenzhe Zhou 


[Impala-ASF-CR] IMPALA-12935: First pass on Calcite planner functions

2024-04-26 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21357 )

Change subject: IMPALA-12935: First pass on Calcite planner functions
..


Patch Set 2:

(2 comments)

Looks like this stack needs a rebase. I can't pull it locally either, I get an 
"internal server error".

http://gerrit.cloudera.org:8080/#/c/21357/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeConverter.java
File 
java/calcite-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeConverter.java:

http://gerrit.cloudera.org:8080/#/c/21357/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeConverter.java@119
PS2, Line 119:   public static List 
createRelDataTypesForArgs(List impalaTypes) {
How does this differ from createRelDataTypes at line 318?


http://gerrit.cloudera.org:8080/#/c/21357/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeConverter.java@130
PS2, Line 130:   public static RelDataType getRelDataType(Type impalaType) {
I'm not sure why this and createRelDataType both exist. They're very similar 
except this one doesn't handle Decimal types.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2dd4e402d69ee10547abeeafe893164ffd789b88
Gerrit-Change-Number: 21357
Gerrit-PatchSet: 2
Gerrit-Owner: Steve Carlin 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Comment-Date: Fri, 26 Apr 2024 22:37:07 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12934: Added Calcite parsing files to Impala

2024-04-26 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21194 )

Change subject: IMPALA-12934: Added Calcite parsing files to Impala
..


Patch Set 5: Code-Review+1

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/21194/5//COMMIT_MSG@14
PS5, Line 14: on top of the Parser.jj file or the config.fmpp file in the futre 
are Impala
typo: future



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If756b5ea8beb85661a30fb5d029e74ebb6719767
Gerrit-Change-Number: 21194
Gerrit-PatchSet: 5
Gerrit-Owner: Steve Carlin 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Comment-Date: Fri, 26 Apr 2024 22:26:42 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] Refactor Workload Management

2024-04-26 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21358 )

Change subject: Refactor Workload Management
..


Patch Set 3:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/21358/1//COMMIT_MSG@6
PS1, Line 6:
   : Refactor Workload Management
> Please assign JIRA number.
Will do, this refactor isn't urgent but I wanted to toss up some ideas.


http://gerrit.cloudera.org:8080/#/c/21358/1/tests/custom_cluster/test_query_log.py
File tests/custom_cluster/test_query_log.py:

http://gerrit.cloudera.org:8080/#/c/21358/1/tests/custom_cluster/test_query_log.py@255
PS1, Line 255:  
"--shutdown_grace_period_s=10 "
TODO: when 'impalad_graceful_shutdown=True', add reasonable default 
'shutdown_grace_period_s' and 'shutdown_deadline_s'. 
'shutdown_grace_period_s=0' is probably fine, as test queries are fast and 
ShutdownWorkloadManagement doesn't run until after the grace period.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1127ef041a3e024bf2b262767d56ec5f29bf3855
Gerrit-Change-Number: 21358
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Fri, 26 Apr 2024 22:07:28 +
Gerrit-HasComments: Yes


  1   2   3   4   5   6   7   8   9   10   >