[Impala-ASF-CR] IMPALA-12515: Build modules for extra pythons

2023-10-24 Thread Michael Smith (Code Review)
Michael Smith has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/20617


Change subject: IMPALA-12515: Build modules for extra pythons
..

IMPALA-12515: Build modules for extra pythons

Adds IMPALA_EXTRA_PACKAGE_PYTHONS to build impala-shell tarball
dependencies for additional Python targets. That can be used to build a
tarball that supports multiple Python 3 minor versions at once.

Updates the impala-shell script to provide a clear error message when
attempting to use the tarball with a Python version that it hasn't been
built for.

Change-Id: I13720a9e3c50f348bef41f5e91f810204e416f13
---
R bin/cmake_aux/create_virtualenv.sh
M bin/impala-config.sh
M shell/CMakeLists.txt
M shell/impala-shell
M shell/make_shell_tarball.sh
5 files changed, 88 insertions(+), 81 deletions(-)



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

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


[Impala-ASF-CR] IMPALA-12515: Build modules for extra pythons

2023-10-24 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20617 )

Change subject: IMPALA-12515: Build modules for extra pythons
..


Patch Set 1:

Build Successful

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I13720a9e3c50f348bef41f5e91f810204e416f13
Gerrit-Change-Number: 20617
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Comment-Date: Tue, 24 Oct 2023 18:04:30 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12018: Consider runtime filter for cardinality reduction

2023-10-24 Thread Riza Suminto (Code Review)
Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20498 )

Change subject: IMPALA-12018: Consider runtime filter for cardinality reduction
..


Patch Set 3:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/20498/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/20498/3//COMMIT_MSG@15
PS3, Line 15: This patch applies runtime filter selectivity to lower cardinality
: estimates of scan nodes and certain join nodes above them after 
runtime
: filter generation and before resource requirement computation.
> I think that ideally we shouldn't use join node selectivity, but the NDV of
I think understand your concern. As conclusion, is it correct that I should 
make similar implementation as JoinNode.getGenericJoinCardinality(), but 
against incoming runtime filters instead of join conjuncts?
https://github.com/apache/impala/blob/c244aadcf367360e52807a84e7fba8b6237651fd/fe/src/main/java/org/apache/impala/planner/JoinNode.java#L404-L411


http://gerrit.cloudera.org:8080/#/c/20498/3//COMMIT_MSG@34
PS3, Line 34: Testing:
> It would be nice to have some targeted tests for edge cases, e.g. missing s
Maybe I should consider enabling this for all situation rather than exclusive 
on COMPUTE_PROCESSING_COST=1 to ease testing. I will look around.


http://gerrit.cloudera.org:8080/#/c/20498/3/fe/src/main/java/org/apache/impala/planner/Planner.java
File fe/src/main/java/org/apache/impala/planner/Planner.java:

http://gerrit.cloudera.org:8080/#/c/20498/3/fe/src/main/java/org/apache/impala/planner/Planner.java@538
PS3, Line 538: CardinalityRefinerVisitor
> I don't have a clear plan, but couldn't this be turned into a recursive fun
Last time I tried to implement recursive calls over PlanNodes tree, I was hit 
by StackOverflow error.
It could be a flaw in my implementation though. I'll try again making it 
recursive.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I033789c9b63a8188484e3afde8e646563918b3e1
Gerrit-Change-Number: 20498
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Tue, 24 Oct 2023 21:03:40 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12513: Reset metadata when the CatalogD become active

2023-10-24 Thread Wenzhe Zhou (Code Review)
Wenzhe Zhou has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20614 )

Change subject: IMPALA-12513: Reset metadata when the CatalogD become active
..


Patch Set 2:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/20614/2//COMMIT_MSG@9
PS2, Line 9: the loaded metadata in the candidate
   : CatalogD may not be the latest.
What are the causes for the loaded metadata in standby catalogd legging behind? 
How about adding a flag to keep metadata ready on standby catalogd if the flag 
is enabled, otherwise reset the metadata when it becomes active.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b54f36f96e7901499105e51790d8e2300d7fea9
Gerrit-Change-Number: 20614
Gerrit-PatchSet: 2
Gerrit-Owner: ttz <2433038...@qq.com>
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: ttz <2433038...@qq.com>
Gerrit-Comment-Date: Tue, 24 Oct 2023 19:33:32 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12515: Build modules for extra pythons

2023-10-24 Thread Wenzhe Zhou (Code Review)
Wenzhe Zhou has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20617 )

Change subject: IMPALA-12515: Build modules for extra pythons
..


Patch Set 1:

What's impact for impala-shell published to pypi?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I13720a9e3c50f348bef41f5e91f810204e416f13
Gerrit-Change-Number: 20617
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Comment-Date: Tue, 24 Oct 2023 21:27:18 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

2023-10-24 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
..


Patch Set 19:

Build Successful

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 19
Gerrit-Owner: Wenzhe Zhou 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: Yida Wu 
Gerrit-Comment-Date: Tue, 24 Oct 2023 17:42:28 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

2023-10-24 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
..


Patch Set 19:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 19
Gerrit-Owner: Wenzhe Zhou 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: Yida Wu 
Gerrit-Comment-Date: Tue, 24 Oct 2023 18:11:03 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12514: Remove dependency on jetty-server

2023-10-24 Thread Michael Smith (Code Review)
Michael Smith has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/20616


Change subject: IMPALA-12514: Remove dependency on jetty-server
..

IMPALA-12514: Remove dependency on jetty-server

Removes the direct dependency on jetty-server. It's still needed and
included via hive-standalone-metastore, which is expected to update its
own dependencies to address CVEs.

Change-Id: I6cd2879ee88e0636186a27c3e16843a30ee9a738
---
M fe/pom.xml
1 file changed, 0 insertions(+), 5 deletions(-)



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

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


[Impala-ASF-CR] IMPALA-12514: Remove dependency on jetty-server

2023-10-24 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20616 )

Change subject: IMPALA-12514: Remove dependency on jetty-server
..


Patch Set 1:

Build Successful

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6cd2879ee88e0636186a27c3e16843a30ee9a738
Gerrit-Change-Number: 20616
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Comment-Date: Tue, 24 Oct 2023 17:56:12 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

2023-10-24 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
..


Patch Set 19: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 19
Gerrit-Owner: Wenzhe Zhou 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: Yida Wu 
Gerrit-Comment-Date: Tue, 24 Oct 2023 21:34:39 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

2023-10-24 Thread Wenzhe Zhou (Code Review)
Wenzhe Zhou has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
..


Patch Set 19:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py@786
PS17, Line 786:   def start_statestore(self):
> I'm curious why this implementation looks so different from start_catalogd.
For catalogd HA, the two catalog instances don't communicate each other. The 
active statestored decide which catalogd instance is active. For statestored 
HA, two statestore instance communicate to negotiate the active role, the 
active statestored send heartbeat to standby statestored. There are additional 
port settings for statestore HA.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 19
Gerrit-Owner: Wenzhe Zhou 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: Yida Wu 
Gerrit-Comment-Date: Tue, 24 Oct 2023 17:56:42 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

2023-10-24 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
..


Patch Set 19:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py@786
PS17, Line 786:   def start_statestore(self):
> For catalogd HA, the two catalog instances don't communicate each other. Th
Ack



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 19
Gerrit-Owner: Wenzhe Zhou 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: Yida Wu 
Gerrit-Comment-Date: Tue, 24 Oct 2023 18:10:38 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12515: Build modules for extra pythons

2023-10-24 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20617 )

Change subject: IMPALA-12515: Build modules for extra pythons
..


Patch Set 1:

> Patch Set 1:
>
> What's impact for impala-shell published to pypi?

None. It uses a generated impala-shell script to invoke impala_shell_main 
directly.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I13720a9e3c50f348bef41f5e91f810204e416f13
Gerrit-Change-Number: 20617
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Comment-Date: Tue, 24 Oct 2023 21:28:41 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-3825: Delegate runtime filter aggregation to some executors

2023-10-24 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20612 )

Change subject: IMPALA-3825: Delegate runtime filter aggregation to some 
executors
..


Patch Set 3:

(18 comments)

http://gerrit.cloudera.org:8080/#/c/20612/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/20612/3//COMMIT_MSG@43
PS3, Line 43: This patch currently targets the boom filter produced by 
partitioned
bloom filter, not boom filter


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

http://gerrit.cloudera.org:8080/#/c/20612/3/be/src/runtime/coordinator.cc@488
PS3, Line 488:   int agg_idx = agg_info.aggregator_idx_to_report(i);
aggregator_idx_to_report producing an agg_idx is confusing me. There's 
something about the names I'm missing.


http://gerrit.cloudera.org:8080/#/c/20612/3/be/src/runtime/runtime-filter-bank.h
File be/src/runtime/runtime-filter-bank.h:

http://gerrit.cloudera.org:8080/#/c/20612/3/be/src/runtime/runtime-filter-bank.h@71
PS3, Line 71:   bool need_subaggregegation = false;
nit: should be need_subaggregation


http://gerrit.cloudera.org:8080/#/c/20612/3/be/src/runtime/runtime-filter-bank.h@227
PS3, Line 227: // Pointer to runtime filter that hold the merge resut of 
all remote updates.
nit: result


http://gerrit.cloudera.org:8080/#/c/20612/3/be/src/runtime/runtime-filter-bank.h@235
PS3, Line 235: inline int AllRemainingProducers() { return pending_remotes 
+ pending_producers; }
These should all probably have function comments.


http://gerrit.cloudera.org:8080/#/c/20612/3/be/src/runtime/runtime-filter-bank.cc
File be/src/runtime/runtime-filter-bank.cc:

http://gerrit.cloudera.org:8080/#/c/20612/3/be/src/runtime/runtime-filter-bank.cc@196
PS3, Line 196:   DCHECK_EQ(res->status().status_code(), TErrorCode::OK);
This feels weird to me because you've already guaranteed that it's false at 
line 188. Maybe simplify with

  if (res->status().status_code() != TErrorCode::OK) {
// ... never sat an error status
DCHECK(is_remote_update);
...
  }


http://gerrit.cloudera.org:8080/#/c/20612/3/be/src/runtime/runtime-filter-bank.cc@217
PS3, Line 217: // Late RPC might come while filter bank is closing.
It'd be helpful to VLOG_RPC that the message was ignored.


http://gerrit.cloudera.org:8080/#/c/20612/3/be/src/runtime/runtime-filter-bank.cc@252
PS3, Line 252:   VLOG(3) << "filter_id=" << params.filter_id()
You use VLOG(3) enough it might warrant its own define in logging.h to name 
this logging scenario. VLOG_FILTER?


http://gerrit.cloudera.org:8080/#/c/20612/3/be/src/runtime/runtime-filter-bank.cc@256
PS3, Line 256:   produced_filter.pending_remotes = 0;
Wouldn't this + line 286 cause pending_remotes to be negative?


http://gerrit.cloudera.org:8080/#/c/20612/3/be/src/runtime/runtime-filter-bank.cc@281
PS3, Line 281: target->Or(params.bloom_filter(), sidecar_slice);
Is there a case where this produces a filter that's always true and we would 
want to stop waiting for other remotes? If FalsePositiveProb == 1.0, then 
presumably we should just use an ALWAYS_TRUE_FILTER.


http://gerrit.cloudera.org:8080/#/c/20612/3/be/src/runtime/runtime-filter-bank.cc@690
PS3, Line 690:   bool try_wait_aggregation = !cancelled_;
Can cancelled_ be updated externally? That seems like a problem, because I 
don't see a lock or atomic.


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

http://gerrit.cloudera.org:8080/#/c/20612/3/be/src/runtime/runtime-filter.h@151
PS3, Line 151:   bool IsReportToPeerRpc() const {
These would benefit from function comments.


http://gerrit.cloudera.org:8080/#/c/20612/3/be/src/scheduling/scheduler.cc
File be/src/scheduling/scheduler.cc:

http://gerrit.cloudera.org:8080/#/c/20612/3/be/src/scheduling/scheduler.cc@288
PS3, Line 288:   // Walk the instances and pick two random krpc backend for 
intermediate runtime
Why 2?


http://gerrit.cloudera.org:8080/#/c/20612/3/be/src/scheduling/scheduler.cc@291
PS3, Line 291:   vector>> instance_groups;
I'm not entirely clear what these pairs represent.


http://gerrit.cloudera.org:8080/#/c/20612/3/be/src/scheduling/scheduler.cc@298
PS3, Line 298:   if (i == 0
Please add a comment describing what this is preventing.


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

http://gerrit.cloudera.org:8080/#/c/20612/3/be/src/service/data-stream-service.cc@139
PS3, Line 139: LOG(INFO) << err_msg;
When would it happen. Should this be a warning?


http://gerrit.cloudera.org:8080/#/c/20612/3/fe/src/main/java/org/apache/impala/planner/PlanFragment.java
File fe/src/main/java/org/apache/impala/planner/PlanFragment.java:


[Impala-ASF-CR] IMPALA-3825: Delegate runtime filter aggregation to some executors

2023-10-24 Thread Riza Suminto (Code Review)
Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20612 )

Change subject: IMPALA-3825: Delegate runtime filter aggregation to some 
executors
..


Patch Set 5:

(18 comments)

http://gerrit.cloudera.org:8080/#/c/20612/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/20612/3//COMMIT_MSG@43
PS3, Line 43: This patch currently targets the bloom filter produced by 
partitioned
> bloom filter, not boom filter
Done


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

http://gerrit.cloudera.org:8080/#/c/20612/3/be/src/runtime/coordinator.cc@488
PS3, Line 488:   const RuntimeFilterAggregatorInfoPB& agg_info =
> aggregator_idx_to_report producing an agg_idx is confusing me. There's some
This is the translation from RuntimeFilterAggregatorInfoPB 
(FragmentExecParamsPB.filter_agg_info) to TRuntimeFilterAggDesc 
(TRuntimeFilterSource.aggregator_desc).

aggregator_idx_to_report id s list belonging to RuntimeFilterAggregatorInfoPB.
agg_idx is an indices of aggregator_idx_to_report.

Rewritten this code to clarify.


http://gerrit.cloudera.org:8080/#/c/20612/3/be/src/runtime/runtime-filter-bank.h
File be/src/runtime/runtime-filter-bank.h:

http://gerrit.cloudera.org:8080/#/c/20612/3/be/src/runtime/runtime-filter-bank.h@71
PS3, Line 71:   bool need_subaggregation = false;
> nit: should be need_subaggregation
Done


http://gerrit.cloudera.org:8080/#/c/20612/3/be/src/runtime/runtime-filter-bank.h@227
PS3, Line 227: /// Pointer to runtime filter that hold the merge result of 
all remote updates.
> nit: result
Done


http://gerrit.cloudera.org:8080/#/c/20612/3/be/src/runtime/runtime-filter-bank.h@235
PS3, Line 235: /// Return number of remaining filter producers, both remote 
and local.
> These should all probably have function comments.
Done


http://gerrit.cloudera.org:8080/#/c/20612/3/be/src/runtime/runtime-filter-bank.cc
File be/src/runtime/runtime-filter-bank.cc:

http://gerrit.cloudera.org:8080/#/c/20612/3/be/src/runtime/runtime-filter-bank.cc@196
PS3, Line 196:   {
> This feels weird to me because you've already guaranteed that it's false at
Done


http://gerrit.cloudera.org:8080/#/c/20612/3/be/src/runtime/runtime-filter-bank.cc@217
PS3, Line 217: return;
> It'd be helpful to VLOG_RPC that the message was ignored.
Done


http://gerrit.cloudera.org:8080/#/c/20612/3/be/src/runtime/runtime-filter-bank.cc@252
PS3, Line 252:   << " This filter is now disabled.";
> You use VLOG(3) enough it might warrant its own define in logging.h to name
Done


http://gerrit.cloudera.org:8080/#/c/20612/3/be/src/runtime/runtime-filter-bank.cc@256
PS3, Line 256:   VLOG_FILTER << "filter_id=" << params.filter_id()
> Wouldn't this + line 286 cause pending_remotes to be negative?
Changed assignment to 1.


http://gerrit.cloudera.org:8080/#/c/20612/3/be/src/runtime/runtime-filter-bank.cc@281
PS3, Line 281: DCHECK(!pending_merge_filter->AlwaysFalse());
> Is there a case where this produces a filter that's always true and we woul
That will require checking the underlying buffer if all the bits have become 
all 1, and there seems no fast way to do so.
Hence, there is separate always_true() flag for this purpose, which checked in 
previous branch.


http://gerrit.cloudera.org:8080/#/c/20612/3/be/src/runtime/runtime-filter-bank.cc@690
PS3, Line 690:   if (query_state_->query_options().runtime_filter_wait_time_ms 
> 0) {
> Can cancelled_ be updated externally? That seems like a problem, because I
cancelled_ can turn from False to True once through CancelLocked(). Once it is 
True, it stays True.
This method returns early and stop sending the remaining incomplete filters if 
query is cancelled midway.


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

http://gerrit.cloudera.org:8080/#/c/20612/3/be/src/runtime/runtime-filter.h@151
PS3, Line 151:
> These would benefit from function comments.
Done


http://gerrit.cloudera.org:8080/#/c/20612/3/be/src/scheduling/scheduler.cc
File be/src/scheduling/scheduler.cc:

http://gerrit.cloudera.org:8080/#/c/20612/3/be/src/scheduling/scheduler.cc@288
PS3, Line 288:
> Why 2?
Typo, it is removed.


http://gerrit.cloudera.org:8080/#/c/20612/3/be/src/scheduling/scheduler.cc@291
PS3, Line 291:   // src_state->instance_states organize fragment instances as 
one dimension vector
> I'm not entirely clear what these pairs represent.
Replaced with typedef and add comments.


http://gerrit.cloudera.org:8080/#/c/20612/3/be/src/scheduling/scheduler.cc@298
PS3, Line 298:   for (int i = 0; i < src_state->instance_states.size(); ++i) {
> Please add a comment describing what this is preventing.
Done


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


[Impala-ASF-CR] IMPALA-3825: Delegate runtime filter aggregation to some executors

2023-10-24 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20612 )

Change subject: IMPALA-3825: Delegate runtime filter aggregation to some 
executors
..


Patch Set 4:

Build Successful

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I11d38ed0f223d6e5b32a19ebe725af7738ee4ab0
Gerrit-Change-Number: 20612
Gerrit-PatchSet: 4
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Kurt Deschler 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Wed, 25 Oct 2023 03:18:44 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

2023-10-24 Thread Wenzhe Zhou (Code Review)
Wenzhe Zhou has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
..

IMPALA-12156: Support High Availability for Statestore

To support statestore HA, we allow two statestored instances in an
Active-Passive HA pair to be added to an Impala cluster. We add the
preemptive behavior for statestored. When HA is enabled, the preemptive
behavior allows the statestored with the higher priority to become
active and the paired statestored becomes standby. The active
statestored acts as the owner of Impala cluster and provides statestore
service for the cluster members.

To enable catalog HA for a cluster, two statestoreds in the HA pair and
all subscribers must be started with starting flag
"enable_statestored_ha" as true.

This patch makes following changes:
- Defined new service for Statestore HA.
- Statestored negotiates the role for HA with its peer statestore
  instance on startup.
- Create HA monitor thread:
  Active statestored sends heartbeat to standby statestored.
  Standby statestored monitors peer's connection states with their
  subscribers.
- Standby statestored sends heartbeat to subscribers with request
  for connection state between active statestore and subscribers.
  Standby statestored saves the connection state as failure detecter.
- When standby statestored lost connection with active statestore,
  it checks the connection states for active statestore, and takes over
  active role if majority of subscribers lost connections with active
  statestore.
- New active statestored sends RPC notification to all subscribers
  for new active statestored and active catalogd elected by the new
  active statestored.
- New active statestored starts sending heartbeat to its peer when it
  receives handshake from its peer.
- Active statestored enters recovery mode if it lost connections with
  its peer statestored and all subscribers. It keeps sending HA
  handshake to its peer until receiving response.
- All subscribers (impalad/catalogd/admissiond) register to two
  statestoreds.
- Subscribers report connection state for the requests from standby
  statestore.
- Subscribers switch to new active statestore when receiving RPC
  notifications from new active statestored.
- Only active statestored sends topic update messages to subscribers.
- Add a new option "enable_statestored_ha" in script
  bin/start-impala-cluster.py for starting Impala mini-cluster with
  statestored HA enabled.
- Add a new Thrift API in statestore service to disable network
  for statestored. It's only used for unit-test to simulate network
  failure. For safety, it's only working when the debug action is set
  in starting flags.

Testings:
 - Added end-to-end unit tests for statestored HA.
 - Passed core tests

Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Reviewed-on: http://gerrit.cloudera.org:8080/20372
Reviewed-by: Michael Smith 
Tested-by: Impala Public Jenkins 
---
M be/generated-sources/gen-cpp/CMakeLists.txt
M be/src/catalog/catalog-server.cc
M be/src/common/global-flags.cc
M be/src/rpc/thrift-server-test.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/admissiond-env.cc
M be/src/statestore/statestore-service-client-wrapper.h
M be/src/statestore/statestore-subscriber-catalog.cc
M be/src/statestore/statestore-subscriber-catalog.h
M be/src/statestore/statestore-subscriber-client-wrapper.h
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/statestore/statestore-test.cc
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
M be/src/statestore/statestored-main.cc
M bin/start-impala-cluster.py
M common/thrift/StatestoreService.thrift
M common/thrift/metrics.json
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_statestored_ha.py
23 files changed, 2,541 insertions(+), 114 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 20
Gerrit-Owner: Wenzhe Zhou 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: Yida Wu 


[Impala-ASF-CR] IMPALA-3825: Delegate runtime filter aggregation to some executors

2023-10-24 Thread Riza Suminto (Code Review)
Hello Kurt Deschler, Abhishek Rawat, Csaba Ringhofer, Michael Smith, Impala 
Public Jenkins,

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

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

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

Change subject: IMPALA-3825: Delegate runtime filter aggregation to some 
executors
..

IMPALA-3825: Delegate runtime filter aggregation to some executors

IMPALA-4400 improve the runtime filter by aggregating runtime filters
locally before sending filter update to the coordinator and sharing a
single RuntimeFilterBank for all fragment instances in a query. However,
local filter aggregation is still insufficient if the number of nodes in
an impala cluster is large. For example, in a cluster of around 700
impalad backends, aggregation of 1 MB bloom filter updates in the
coordinator can exceed more than 1 second.

This patch aims to reduce coordinator load and speed up runtime filter
aggregation by doing intermediate aggregation in a few designated impala
backends before doing final aggregation and publishing in the
coordinator. Query option MAX_NUM_FILTER_AGGREGATOR is added to control
this feature. Given N as the number of backend executors excluding the
coordinator, the selected number of intermediate aggregators M =
min(MAX_NUM_FILTER_AGGREGATOR, N / 2). Setting MAX_NUM_FILTER_AGGREGATOR
<= 0 will disable the intermediate aggregator feature. In the backend
scheduler, M impalad will be selected randomly as the intermediate
aggregator for that runtime filter. Information of this M selected
impalad then passed from scheduler to coordinator as a
RuntimeFilterAggregatorInfoPB. The coordinator then converts the
RuntimeFilterAggregatorInfoPB into a filter routing information
TRuntimeFilterAggDesc that is piggy-backed in TRuntimeFilterSource.

A new RPC endpoint named UpdateFilterFromRemote is added in
data_stream_service.proto to handle filter updates from fellow impalad
executor to the designated aggregator impalad. This RPC will merge
filter updates into 'pending_remote_filter'. The intermediate aggregator
will then combine 'pending_remote_filter' with
'pending_merge_filter' (from local aggregation) into 'result_filter'
which then send to the coordinator. Note that due to the random
selection of M impalad, an additional memory reservation for
'pending_remote_filter' is added for all impalad, even when only M
impalad is doing the intermediate aggregation per runtime filter.

This patch currently targets the boom filter produced by partitioned
join build only. Another kind of runtime filter is still efficient to
aggregate in coordinator only, while the bloom filter from broadcast
join only requires 1 valid filter update for publishing.

test_runtime_filters.py is modified to clarify the exec_options
dimension, test matrix constraints, and reduce pytest.skip() calls on
each test. runtime_filters.test is also changed to use counter
aggregation and assert on ExecSummary table so that they stay valid
irrespective of the number of fragment instances.

We benchmark the aggregation speed of 1 MB runtime filter aggregation on
20 executor nodes cluster with MT_DOP=36 that is instrumented to disable
local aggregation, simulating 720 runtime filter updates. The speed is
approximated as the duration between the earliest time a filter update
is made and the time that the coordinator publishes the complete filter.
The result is following:

+---++
| MAX_NUM_FILTER_AGGREGATOR | Aggregation speed (ms) |
+---++
| 0 |   1296 |
| 1 |   1229 |
| 2 |608 |
| 4 |329 |
| 8 |205 |
+---++

Testing:
- Exercise MAX_NUM_FILTER_AGGREGATOR in test_runtime_filters.py and
  query-options-test.cc
- Add custom_cluster/test_runtime_filter_aggregation.py.
- Pass exhaustive end-to-end and custom-cluster tests.

Change-Id: I11d38ed0f223d6e5b32a19ebe725af7738ee4ab0
---
M be/src/common/logging.h
M be/src/runtime/coordinator.cc
M be/src/runtime/data-stream-test.cc
M be/src/runtime/query-state.cc
M be/src/runtime/query-state.h
M be/src/runtime/runtime-filter-bank.cc
M be/src/runtime/runtime-filter-bank.h
M be/src/runtime/runtime-filter.cc
M be/src/runtime/runtime-filter.h
M be/src/scheduling/scheduler.cc
M be/src/scheduling/scheduler.h
M be/src/service/data-stream-service.cc
M be/src/service/data-stream-service.h
M be/src/service/query-options-test.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M be/src/util/bloom-filter.cc
M be/src/util/bloom-filter.h
M be/src/util/runtime-profile-counters.h
M common/protobuf/admission_control_service.proto
M common/protobuf/data_stream_service.proto
M 

[Impala-ASF-CR] IMPALA-12509: Optimize the backend startup and planner time of large Iceberg table query

2023-10-24 Thread Fu Lili (Code Review)
Fu Lili has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20601 )

Change subject: IMPALA-12509: Optimize the backend startup and planner time of 
large Iceberg table query
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20601/2/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
File fe/src/main/java/org/apache/impala/catalog/IcebergTable.java:

http://gerrit.cloudera.org:8080/#/c/20601/2/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java@549
PS2, Line 549: 
desc.setHdfsTable(hdfsTable_.getTHdfsTable(ThriftObjectType.DESCRIPTOR_ONLY, 
null));
> Should we do the same optimization for IcebergTimeTravelTable? Could you al
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I46a68f21903c16db3b02681feddb61ddce57879a
Gerrit-Change-Number: 20601
Gerrit-PatchSet: 2
Gerrit-Owner: Fu Lili 
Gerrit-Reviewer: Fu Lili 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 25 Oct 2023 02:50:17 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12491: [DOCS] Add a note on the cache item

2023-10-24 Thread Quanlong Huang (Code Review)
Quanlong Huang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20611 )

Change subject: IMPALA-12491: [DOCS] Add a note on the cache item
..


Patch Set 1: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I508ce667181d635c17373c7336ea9f83984d7641
Gerrit-Change-Number: 20611
Gerrit-PatchSet: 1
Gerrit-Owner: Shajini Thayasingh 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Comment-Date: Wed, 25 Oct 2023 02:20:12 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-3825: Delegate runtime filter aggregation to some executors

2023-10-24 Thread Riza Suminto (Code Review)
Hello Kurt Deschler, Abhishek Rawat, Csaba Ringhofer, Michael Smith, Impala 
Public Jenkins,

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

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

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

Change subject: IMPALA-3825: Delegate runtime filter aggregation to some 
executors
..

IMPALA-3825: Delegate runtime filter aggregation to some executors

IMPALA-4400 improve the runtime filter by aggregating runtime filters
locally before sending filter update to the coordinator and sharing a
single RuntimeFilterBank for all fragment instances in a query. However,
local filter aggregation is still insufficient if the number of nodes in
an impala cluster is large. For example, in a cluster of around 700
impalad backends, aggregation of 1 MB bloom filter updates in the
coordinator can exceed more than 1 second.

This patch aims to reduce coordinator load and speed up runtime filter
aggregation by doing intermediate aggregation in a few designated impala
backends before doing final aggregation and publishing in the
coordinator. Query option MAX_NUM_FILTER_AGGREGATOR is added to control
this feature. Given N as the number of backend executors excluding the
coordinator, the selected number of intermediate aggregators M =
min(MAX_NUM_FILTER_AGGREGATOR, N / 2). Setting MAX_NUM_FILTER_AGGREGATOR
<= 0 will disable the intermediate aggregator feature. In the backend
scheduler, M impalad will be selected randomly as the intermediate
aggregator for that runtime filter. Information of this M selected
impalad then passed from scheduler to coordinator as a
RuntimeFilterAggregatorInfoPB. The coordinator then converts the
RuntimeFilterAggregatorInfoPB into a filter routing information
TRuntimeFilterAggDesc that is piggy-backed in TRuntimeFilterSource.

A new RPC endpoint named UpdateFilterFromRemote is added in
data_stream_service.proto to handle filter updates from fellow impalad
executor to the designated aggregator impalad. This RPC will merge
filter updates into 'pending_remote_filter'. The intermediate aggregator
will then combine 'pending_remote_filter' with
'pending_merge_filter' (from local aggregation) into 'result_filter'
which then send to the coordinator. Note that due to the random
selection of M impalad, an additional memory reservation for
'pending_remote_filter' is added for all impalad, even when only M
impalad is doing the intermediate aggregation per runtime filter.

This patch currently targets the bloom filter produced by partitioned
join build only. Another kind of runtime filter is still efficient to
aggregate in coordinator only, while the bloom filter from broadcast
join only requires 1 valid filter update for publishing.

test_runtime_filters.py is modified to clarify the exec_options
dimension, test matrix constraints, and reduce pytest.skip() calls on
each test. runtime_filters.test is also changed to use counter
aggregation and assert on ExecSummary table so that they stay valid
irrespective of the number of fragment instances.

We benchmark the aggregation speed of 1 MB runtime filter aggregation on
20 executor nodes cluster with MT_DOP=36 that is instrumented to disable
local aggregation, simulating 720 runtime filter updates. The speed is
approximated as the duration between the earliest time a filter update
is made and the time that the coordinator publishes the complete filter.
The result is following:

+---++
| MAX_NUM_FILTER_AGGREGATOR | Aggregation speed (ms) |
+---++
| 0 |   1296 |
| 1 |   1229 |
| 2 |608 |
| 4 |329 |
| 8 |205 |
+---++

Testing:
- Exercise MAX_NUM_FILTER_AGGREGATOR in test_runtime_filters.py and
  query-options-test.cc
- Add custom_cluster/test_runtime_filter_aggregation.py.
- Pass exhaustive end-to-end and custom-cluster tests.

Change-Id: I11d38ed0f223d6e5b32a19ebe725af7738ee4ab0
---
M be/src/common/logging.h
M be/src/runtime/coordinator.cc
M be/src/runtime/data-stream-test.cc
M be/src/runtime/query-state.cc
M be/src/runtime/query-state.h
M be/src/runtime/runtime-filter-bank.cc
M be/src/runtime/runtime-filter-bank.h
M be/src/runtime/runtime-filter.cc
M be/src/runtime/runtime-filter.h
M be/src/scheduling/scheduler.cc
M be/src/scheduling/scheduler.h
M be/src/service/data-stream-service.cc
M be/src/service/data-stream-service.h
M be/src/service/query-options-test.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M be/src/util/bloom-filter.cc
M be/src/util/bloom-filter.h
M be/src/util/runtime-profile-counters.h
M common/protobuf/admission_control_service.proto
M common/protobuf/data_stream_service.proto
M 

[Impala-ASF-CR] IMPALA-11996: Scanner change for Iceberg metadata querying

2023-10-24 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20010 )

Change subject: IMPALA-11996: Scanner change for Iceberg metadata querying
..


Patch Set 15:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7
Gerrit-Change-Number: 20010
Gerrit-PatchSet: 15
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Gergely Fürnstáhl 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 25 Oct 2023 05:50:05 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-11996: Scanner change for Iceberg metadata querying

2023-10-24 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#15). ( 
http://gerrit.cloudera.org:8080/20010 )

Change subject: IMPALA-11996: Scanner change for Iceberg metadata querying
..

IMPALA-11996: Scanner change for Iceberg metadata querying

This commit adds a scan node for querying Iceberg metadata tables. The
scan node creates a Java scanner object that creates and scans the
metadata table. The scanner uses the Iceberg API to scan the table after
that the scan node fetches the rows one by one and materialises them
into RowBatches. The Iceberg row reader on the backend does the
translation between Iceberg and Impala types.

There is only one fragment created to query the Iceberg metadata table
which is supposed to be executed on the coordinator node that already
has the Iceberg table loaded. This way there is no need for further
table loading on the executor side.

This change will not cover nested column types, these slots are set to
NULL, it will be done in IMPALA-12205.

Testing:
 - Added e2e tests for querying metadata tables
 - Updated planner tests

Performance testing:
Created a table and inserted ~5500 rows one by one, this generated
~27 ALL_MANIFESTS metadata table records. This table is quite wide
and has a String column as well.

I only mention count(*) test on ALL_MANIFESTS, because every row is
materialized in every scenario currently:
  - Cold cache: 15.76s
- IcebergApiScanTime: 124.407ms
- MaterializeTupleTime: 8s368ms
  - Warm cache: 7.56s
- IcebergApiScanTime: 3.646ms
- MaterializeTupleTime: 7s477ms

Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7
---
M be/CMakeLists.txt
M be/src/exec/CMakeLists.txt
M be/src/exec/exec-node.cc
A be/src/exec/iceberg-metadata/CMakeLists.txt
A be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
A be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h
A be/src/exec/iceberg-metadata/iceberg-row-reader.cc
A be/src/exec/iceberg-metadata/iceberg-row-reader.h
M be/src/scheduling/scheduler.cc
M be/src/service/frontend.cc
M be/src/service/frontend.h
M be/src/service/impalad-main.cc
M be/src/util/jni-util.cc
M be/src/util/jni-util.h
M common/thrift/PlanNodes.thrift
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/IcebergMetadataTableRef.java
M fe/src/main/java/org/apache/impala/analysis/Path.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java
M fe/src/main/java/org/apache/impala/planner/IcebergMetadataScanNode.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
A fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
M 
testdata/workloads/functional-planner/queries/PlannerTest/iceberg-metadata-table-scan.test
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
M tests/authorization/test_ranger.py
M tests/query_test/test_iceberg.py
32 files changed, 1,419 insertions(+), 167 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7
Gerrit-Change-Number: 20010
Gerrit-PatchSet: 15
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Gergely Fürnstáhl 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-12018: Consider runtime filter for cardinality reduction

2023-10-24 Thread Csaba Ringhofer (Code Review)
Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20498 )

Change subject: IMPALA-12018: Consider runtime filter for cardinality reduction
..


Patch Set 3:

(3 comments)

The implementation looks good to me, but i have some high level questions.

http://gerrit.cloudera.org:8080/#/c/20498/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/20498/3//COMMIT_MSG@15
PS3, Line 15: This patch applies runtime filter selectivity to lower cardinality
: estimates of scan nodes and certain join nodes above them after 
runtime
: filter generation and before resource requirement computation.
I think that ideally we shouldn't use join node selectivity, but the NDV of the 
key on the build side. Generally we assume that selectivity on the join node 
will also reduce the NDV of the key, so the two becomes interchangeable, but I 
would prefer if this logic would stay within join node/runtime filter code, and 
not spread to other nodes.

This is how I see the steps that lead to cardinality reduction in the scan node:
1. ndv(key) is calculated in the join builder based on build side columns stats 
and estimated selectivity
2. a runtime filter is created for key, which also has an fpp based on ndv(key) 
and bloom filter size
3. the scan node can calculate bloom filter selectivity based on its own ndv 
(after applying other predicates), the ndv from the bloom filter and fpp of the 
bloom filter

This probably gives the same results as the current solution, but makes it 
easier to think about the effect of other predicates / runtime filters / 
duplicate keys on build side.

A good example where this could help is a join with huge number of duplicates + 
a very selective filter on the build side. We could assume that the build side 
ndv(key) is reduced due to the selective predicate, but 
getJoinNodeSelectivity() would be still > 0 due to the duplicate matches. So we 
wouldn't reduce scanner cardinality while we could assume that many rows are 
dropped based on the bloom filter.


http://gerrit.cloudera.org:8080/#/c/20498/3//COMMIT_MSG@34
PS3, Line 34: Testing:
It would be nice to have some targeted tests for edge cases, e.g. missing 
stats, duplicate matches in join, effect of other predicates.


http://gerrit.cloudera.org:8080/#/c/20498/3/fe/src/main/java/org/apache/impala/planner/Planner.java
File fe/src/main/java/org/apache/impala/planner/Planner.java:

http://gerrit.cloudera.org:8080/#/c/20498/3/fe/src/main/java/org/apache/impala/planner/Planner.java@538
PS3, Line 538: CardinalityRefinerVisitor
I don't have a clear plan, but couldn't this be turned into a recursive 
function in PlanNode?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I033789c9b63a8188484e3afde8e646563918b3e1
Gerrit-Change-Number: 20498
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Tue, 24 Oct 2023 09:17:30 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-11996: Scanner change for Iceberg metadata querying

2023-10-24 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20010 )

Change subject: IMPALA-11996: Scanner change for Iceberg metadata querying
..


Patch Set 15:

Build Successful

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7
Gerrit-Change-Number: 20010
Gerrit-PatchSet: 15
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Gergely Fürnstáhl 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Tue, 24 Oct 2023 09:17:39 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12481: Fix sorted-run-merger-ir.cc make dependency

2023-10-24 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20604 )

Change subject: IMPALA-12481: Fix sorted-run-merger-ir.cc make dependency
..


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id39cc9613bec60f6308db05dde99f375c68166e0
Gerrit-Change-Number: 20604
Gerrit-PatchSet: 2
Gerrit-Owner: Noemi Pap-Takacs 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Comment-Date: Tue, 24 Oct 2023 09:17:10 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-11996: Scanner change for Iceberg metadata querying

2023-10-24 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20010 )

Change subject: IMPALA-11996: Scanner change for Iceberg metadata querying
..


Patch Set 15:

(2 comments)

Thank you for the comments!
Published PS15 and answered your questions/comments.

http://gerrit.cloudera.org:8080/#/c/20010/15/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
File 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test:

http://gerrit.cloudera.org:8080/#/c/20010/15/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@411
PS15, Line 411:  RESULTS
> In this test couldn't you assert on the exact values instead of checking re
I don't think I can test it without using the same code path to generate the 
expected results.

Also, I cannot pre-generate a table, because the metadata gets malformed when 
using avrotools (comment on PS7).


http://gerrit.cloudera.org:8080/#/c/20010/15/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@416
PS15, Line 416:
> Can we add a few more complex queries from https://www.apachecon.com/acna20
This commit does not resolve the nested type columns. I can add more complex 
queries with the nested type sub-task, IMPALA-12205.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7
Gerrit-Change-Number: 20010
Gerrit-PatchSet: 15
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Gergely Fürnstáhl 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Tue, 24 Oct 2023 08:46:02 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12481: Fix sorted-run-merger-ir.cc make dependency

2023-10-24 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20604 )

Change subject: IMPALA-12481: Fix sorted-run-merger-ir.cc make dependency
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id39cc9613bec60f6308db05dde99f375c68166e0
Gerrit-Change-Number: 20604
Gerrit-PatchSet: 2
Gerrit-Owner: Noemi Pap-Takacs 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Comment-Date: Tue, 24 Oct 2023 09:17:10 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

2023-10-24 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
..


Patch Set 17: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 17
Gerrit-Owner: Wenzhe Zhou 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: Yida Wu 
Gerrit-Comment-Date: Tue, 24 Oct 2023 06:38:57 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12373: Small String Optimization for StringValue

2023-10-24 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20496 )

Change subject: IMPALA-12373: Small String Optimization for StringValue
..


Patch Set 8:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I741c3a5f12ab620b6b64b57d4c89b5f8e056efd3
Gerrit-Change-Number: 20496
Gerrit-PatchSet: 8
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Tue, 24 Oct 2023 11:43:12 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12513: Reset metadata when the CatalogD become active

2023-10-24 Thread ttttttz (Code Review)
ttz has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/20614


Change subject: IMPALA-12513: Reset metadata when the CatalogD become active
..

IMPALA-12513: Reset metadata when the CatalogD become active

Change-Id: I2b54f36f96e7901499105e51790d8e2300d7fea9
---
M be/src/catalog/catalog-server.cc
1 file changed, 11 insertions(+), 0 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2b54f36f96e7901499105e51790d8e2300d7fea9
Gerrit-Change-Number: 20614
Gerrit-PatchSet: 1
Gerrit-Owner: ttz <2433038...@qq.com>


[Impala-ASF-CR] IMPALA-12373: Small String Optimization for StringValue

2023-10-24 Thread Zoltan Borok-Nagy (Code Review)
Hello Daniel Becker, Csaba Ringhofer, Michael Smith, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-12373: Small String Optimization for StringValue
..

IMPALA-12373: Small String Optimization for StringValue

This patch implements the Small String Optimization (SSO) for
StringValue objects. This is a well-known optimization in the C++
world that is used by the majority of various string implementations
(STL string, boost string, Folly string, etc.)

The old layout of the StringValue was:
  char* ptr;  // 8 bytes
  int len;// 4 bytes

We also add the __packed__ attribute to the StringValue class which
means there is no padding between 'ptr' and 'len', neither after 'len'.
I.e. StringValue objects take 12 bytes. This means with SSO we can use
11 bytes to store small strings. In this case the last byte is used to
store the length.

Small string layout:
  char[11] buf;
  unsigned char len;

We also need an indicator bit (which tells whether the long
representation or the small representation is active) in the last byte
that is the same bit of LONG_STRING.len and SMALL_STRING.len. On
little-endian architectures this is the most significant bit (MSB) of
both LONG_STRING.len and SMALL_STRING.len. On big endian architectures
this would be the least significant bit (LSB) of both LONG_STRING.len
and SMALL_STRING.len.

This patch adds SmallableString which implements the above on an
on-demand basis. I.e. all string objects start with the long
representation, then the string object can be explicitly asked to try
smallify itself. This is because I didn't want to introduce too much
change in behavior. This way we can try smallify only at certain points
(e.g. DeepCopy()), and we can also smallify all strings in a tuple at
once. The latter means if we've done that for a tuple, subsequent
smallifications can return on the first small string that is encountered
(because we can assume that all other string slots are also smallified).

Benefits:
 * lower memory and CPU cache consumption
 * smaller serialization buffers to compress
 * less data to send over the network
 * less data to spill to disk

Measurements:
I used TPCH(30) with the following query:

  select * from lineitem a, lineitem b
  where a.l_orderkey = b.l_orderkey and
a.l_orderkey * b.l_orderkey < 1000

The above query generates significant network traffic and does spilling.
The query selects all 16 columns out of which 6.5 columns contain small
strings. I.e. this kind of data is a good candidate for this
optimization but also not unrealistic.

This improves the following numbers:
Total query time: 5m16s--> 3m40s
Total CPU time:   12m9s--> 10m17s
Bytes sent over the network:  54.17 GB --> 41.76 GB
Data had to be spilled:   14.66 GB --> 9.42 GB

On the standard benchmarks I measured the followings:

TPC-H:
+--+---+-++++
| Workload | File Format   | Avg (s) | Delta(Avg) | GeoMean(s) | 
Delta(GeoMean) |
+--+---+-++++
| TPCH(42) | parquet / none / none | 3.57| -3.06% | 2.40   | -1.34% 
|
+--+---+-++++

TPC-DS:
+---+---+-++++
| Workload  | File Format   | Avg (s) | Delta(Avg) | GeoMean(s) | 
Delta(GeoMean) |
+---+---+-++++
| TPCDS(30) | parquet / none / none | 2.09| -0.92% | 0.76   | 
-1.17% |
+---+---+-++++

TODO:
* add specific BE and E2E tests for small strings
** also for long strings
* add ifdefs for big-endian
* update estimations in a follow-up Jira

Change-Id: I741c3a5f12ab620b6b64b57d4c89b5f8e056efd3
---
M be/src/benchmarks/atod-benchmark.cc
M be/src/benchmarks/atof-benchmark.cc
M be/src/benchmarks/atoi-benchmark.cc
M be/src/benchmarks/hash-benchmark.cc
M be/src/benchmarks/parse-timestamp-benchmark.cc
M be/src/benchmarks/string-benchmark.cc
M be/src/benchmarks/string-search-benchmark.cc
M be/src/codegen/codegen-anyval.cc
M be/src/codegen/gen_ir_descriptions.py
M be/src/codegen/impala-ir.cc
M be/src/codegen/llvm-codegen-test.cc
M be/src/exec/analytic-eval-node.cc
M be/src/exec/avro/hdfs-avro-scanner-ir.cc
M be/src/exec/avro/hdfs-avro-scanner-test.cc
M be/src/exec/data-source-scan-node.cc
M be/src/exec/file-metadata-utils.cc
M be/src/exec/grouping-aggregator.cc
M be/src/exec/hash-table.cc
M be/src/exec/hdfs-text-table-writer.cc
M be/src/exec/hdfs-text-table-writer.h
M be/src/exec/iceberg-delete-builder.cc
M 

[Impala-ASF-CR] IMPALA-12373: Small String Optimization for StringValue

2023-10-24 Thread Zoltan Borok-Nagy (Code Review)
Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20496 )

Change subject: IMPALA-12373: Small String Optimization for StringValue
..


Patch Set 7:

(2 comments)

Fixed a few bugs, mostly related to codegen.

http://gerrit.cloudera.org:8080/#/c/20496/7/be/src/exec/parquet/parquet-common.h
File be/src/exec/parquet/parquet-common.h:

http://gerrit.cloudera.org:8080/#/c/20496/7/be/src/exec/parquet/parquet-common.h@577
PS7, Line 577: fixed_len_size < str_len
> Are you sure it's correct? This is different from what we did before, and n
If fixed_len_size is greater than 0 then it still caps the length of 'v' or am 
I missing something?


http://gerrit.cloudera.org:8080/#/c/20496/7/be/src/runtime/smallable-string.h
File be/src/runtime/smallable-string.h:

http://gerrit.cloudera.org:8080/#/c/20496/7/be/src/runtime/smallable-string.h@55
PS7, Line 55: other
> Nit: could surround 'other' with (single) quotes. Same as on L57.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I741c3a5f12ab620b6b64b57d4c89b5f8e056efd3
Gerrit-Change-Number: 20496
Gerrit-PatchSet: 7
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Tue, 24 Oct 2023 11:42:36 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12373: Small String Optimization for StringValue

2023-10-24 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20496 )

Change subject: IMPALA-12373: Small String Optimization for StringValue
..


Patch Set 8:

Build Successful

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I741c3a5f12ab620b6b64b57d4c89b5f8e056efd3
Gerrit-Change-Number: 20496
Gerrit-PatchSet: 8
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Tue, 24 Oct 2023 12:10:55 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12481: Fix sorted-run-merger-ir.cc make dependency

2023-10-24 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/20604 )

Change subject: IMPALA-12481: Fix sorted-run-merger-ir.cc make dependency
..

IMPALA-12481: Fix sorted-run-merger-ir.cc make dependency

IMPALA-11477 added sorted-run-merger-ir.cc, and incorrectly
added it to the Runtime target, not RuntimeIr.
This patch fixes the issue by adding sorted-run-merger-ir.cc
to RuntimeIr instead in CMakeLists.txt.

Change-Id: Id39cc9613bec60f6308db05dde99f375c68166e0
Reviewed-on: http://gerrit.cloudera.org:8080/20604
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M be/src/runtime/CMakeLists.txt
1 file changed, 1 insertion(+), 1 deletion(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Id39cc9613bec60f6308db05dde99f375c68166e0
Gerrit-Change-Number: 20604
Gerrit-PatchSet: 3
Gerrit-Owner: Noemi Pap-Takacs 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 


[Impala-ASF-CR] IMPALA-12513: Reset metadata when the CatalogD become active

2023-10-24 Thread ttttttz (Code Review)
Hello Quanlong Huang, Wenzhe Zhou, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-12513: Reset metadata when the CatalogD become active
..

IMPALA-12513: Reset metadata when the CatalogD become active

When switching active CatalogD, the loaded metadata in the candidate
CatalogD may not be the latest. We should reset the metadata to ensure
that there are no metadata inconsistency exceptions after failover.

Change-Id: I2b54f36f96e7901499105e51790d8e2300d7fea9
---
M be/src/catalog/catalog-server.cc
1 file changed, 11 insertions(+), 0 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2b54f36f96e7901499105e51790d8e2300d7fea9
Gerrit-Change-Number: 20614
Gerrit-PatchSet: 2
Gerrit-Owner: ttz <2433038...@qq.com>
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: ttz <2433038...@qq.com>


[Impala-ASF-CR] IMPALA-12513: Reset metadata when the CatalogD become active

2023-10-24 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20614 )

Change subject: IMPALA-12513: Reset metadata when the CatalogD become active
..


Patch Set 1:

Build Successful

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b54f36f96e7901499105e51790d8e2300d7fea9
Gerrit-Change-Number: 20614
Gerrit-PatchSet: 1
Gerrit-Owner: ttz <2433038...@qq.com>
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: ttz <2433038...@qq.com>
Gerrit-Comment-Date: Tue, 24 Oct 2023 13:06:05 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12481: Fix sorted-run-merger-ir.cc make dependency

2023-10-24 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20604 )

Change subject: IMPALA-12481: Fix sorted-run-merger-ir.cc make dependency
..


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id39cc9613bec60f6308db05dde99f375c68166e0
Gerrit-Change-Number: 20604
Gerrit-PatchSet: 2
Gerrit-Owner: Noemi Pap-Takacs 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Comment-Date: Tue, 24 Oct 2023 13:32:28 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12373: Small String Optimization for StringValue

2023-10-24 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20496 )

Change subject: IMPALA-12373: Small String Optimization for StringValue
..


Patch Set 8: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I741c3a5f12ab620b6b64b57d4c89b5f8e056efd3
Gerrit-Change-Number: 20496
Gerrit-PatchSet: 8
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Tue, 24 Oct 2023 16:20:00 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

2023-10-24 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
..


Patch Set 17:

(1 comment)

Containerized tests seem to be consistently failing to start.

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py@379
PS17, Line 379: def build_statestored_arg_list(num_statestored, remap_ports):
I suspect something added to the return arg list for this function isn't a 
string, and that's causing __run_container__ to fail in containerized tests. 
I'd need to dig through it more to figure out where exactly that's happening.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 17
Gerrit-Owner: Wenzhe Zhou 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: Yida Wu 
Gerrit-Comment-Date: Tue, 24 Oct 2023 16:25:59 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

2023-10-24 Thread Wenzhe Zhou (Code Review)
Wenzhe Zhou has uploaded a new patch set (#18). ( 
http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
..

IMPALA-12156: Support High Availability for Statestore

To support statestore HA, we allow two statestored instances in an
Active-Passive HA pair to be added to an Impala cluster. We add the
preemptive behavior for statestored. When HA is enabled, the preemptive
behavior allows the statestored with the higher priority to become
active and the paired statestored becomes standby. The active
statestored acts as the owner of Impala cluster and provides statestore
service for the cluster members.

To enable catalog HA for a cluster, two statestoreds in the HA pair and
all subscribers must be started with starting flag
"enable_statestored_ha" as true.

This patch makes following changes:
- Defined new service for Statestore HA.
- Statestored negotiates the role for HA with its peer statestore
  instance on startup.
- Create HA monitor thread:
  Active statestored sends heartbeat to standby statestored.
  Standby statestored monitors peer's connection states with their
  subscribers.
- Standby statestored sends heartbeat to subscribers with request
  for connection state between active statestore and subscribers.
  Standby statestored saves the connection state as failure detecter.
- When standby statestored lost connection with active statestore,
  it checks the connection states for active statestore, and takes over
  active role if majority of subscribers lost connections with active
  statestore.
- New active statestored sends RPC notification to all subscribers
  for new active statestored and active catalogd elected by the new
  active statestored.
- New active statestored starts sending heartbeat to its peer when it
  receives handshake from its peer.
- Active statestored enters recovery mode if it lost connections with
  its peer statestored and all subscribers. It keeps sending HA
  handshake to its peer until receiving response.
- All subscribers (impalad/catalogd/admissiond) register to two
  statestoreds.
- Subscribers report connection state for the requests from standby
  statestore.
- Subscribers switch to new active statestore when receiving RPC
  notifications from new active statestored.
- Only active statestored sends topic update messages to subscribers.
- Add a new option "enable_statestored_ha" in script
  bin/start-impala-cluster.py for starting Impala mini-cluster with
  statestored HA enabled.
- Add a new Thrift API in statestore service to disable network
  for statestored. It's only used for unit-test to simulate network
  failure. For safety, it's only working when the debug action is set
  in starting flags.

Testings:
 - Added end-to-end unit tests for statestored HA.
 - Passed core tests

Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
---
M be/generated-sources/gen-cpp/CMakeLists.txt
M be/src/catalog/catalog-server.cc
M be/src/common/global-flags.cc
M be/src/rpc/thrift-server-test.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/admissiond-env.cc
M be/src/statestore/statestore-service-client-wrapper.h
M be/src/statestore/statestore-subscriber-catalog.cc
M be/src/statestore/statestore-subscriber-catalog.h
M be/src/statestore/statestore-subscriber-client-wrapper.h
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/statestore/statestore-test.cc
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
M be/src/statestore/statestored-main.cc
M bin/start-impala-cluster.py
M common/thrift/StatestoreService.thrift
M common/thrift/metrics.json
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_statestored_ha.py
23 files changed, 2,538 insertions(+), 112 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 18
Gerrit-Owner: Wenzhe Zhou 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: Yida Wu 


[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

2023-10-24 Thread Wenzhe Zhou (Code Review)
Wenzhe Zhou has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
..


Patch Set 18:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py@379
PS17, Line 379: def build_statestored_arg_list(num_statestored, remap_ports):
> I suspect something added to the return arg list for this function isn't a
Found the cause. This function returns a list When calling this function from 
DockerMiniClusterOperations.start_statestore() with enable_statestored_ha not 
enabled, it should use the first element.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 18
Gerrit-Owner: Wenzhe Zhou 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: Yida Wu 
Gerrit-Comment-Date: Tue, 24 Oct 2023 16:33:01 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

2023-10-24 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
..


Patch Set 18:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py@395
PS17, Line 395: statestored_arg_list.append(args)
> Oh, is this building a list of lists? It should use extend.
Oh I see, that's intentional because it's a separate arg list for each 
instance. Now I understand your response at 379.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 18
Gerrit-Owner: Wenzhe Zhou 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: Yida Wu 
Gerrit-Comment-Date: Tue, 24 Oct 2023 16:39:34 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

2023-10-24 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
..


Patch Set 18:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py@379
PS17, Line 379: def build_statestored_arg_list(num_statestored, remap_ports):
> Found the cause. This function returns a list When calling this function fr
I'd like to see the function description updated reflecting that the return 
value format has changed.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 18
Gerrit-Owner: Wenzhe Zhou 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: Yida Wu 
Gerrit-Comment-Date: Tue, 24 Oct 2023 16:40:30 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

2023-10-24 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
..


Patch Set 18:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py@395
PS17, Line 395: statestored_arg_list.append(args)
Oh, is this building a list of lists? It should use extend.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 18
Gerrit-Owner: Wenzhe Zhou 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: Yida Wu 
Gerrit-Comment-Date: Tue, 24 Oct 2023 16:38:12 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

2023-10-24 Thread Wenzhe Zhou (Code Review)
Wenzhe Zhou has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
..


Patch Set 18:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py@395
PS17, Line 395: statestored_arg_list.append(args)
> Oh, is this building a list of lists? It should use extend.
Yes, it's building a list of lists.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 18
Gerrit-Owner: Wenzhe Zhou 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: Yida Wu 
Gerrit-Comment-Date: Tue, 24 Oct 2023 16:46:44 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

2023-10-24 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
..


Patch Set 18:

Build Successful

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 18
Gerrit-Owner: Wenzhe Zhou 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: Yida Wu 
Gerrit-Comment-Date: Tue, 24 Oct 2023 17:03:43 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

2023-10-24 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
..


Patch Set 19:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 19
Gerrit-Owner: Wenzhe Zhou 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: Yida Wu 
Gerrit-Comment-Date: Tue, 24 Oct 2023 17:13:44 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

2023-10-24 Thread Wenzhe Zhou (Code Review)
Wenzhe Zhou has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
..


Patch Set 19:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py@379
PS17, Line 379: def build_statestored_arg_list(num_statestored, remap_ports):
> I'd like to see the function description updated reflecting that the return
Updated description.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 19
Gerrit-Owner: Wenzhe Zhou 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: Yida Wu 
Gerrit-Comment-Date: Tue, 24 Oct 2023 17:13:30 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

2023-10-24 Thread Wenzhe Zhou (Code Review)
Wenzhe Zhou has uploaded a new patch set (#19). ( 
http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
..

IMPALA-12156: Support High Availability for Statestore

To support statestore HA, we allow two statestored instances in an
Active-Passive HA pair to be added to an Impala cluster. We add the
preemptive behavior for statestored. When HA is enabled, the preemptive
behavior allows the statestored with the higher priority to become
active and the paired statestored becomes standby. The active
statestored acts as the owner of Impala cluster and provides statestore
service for the cluster members.

To enable catalog HA for a cluster, two statestoreds in the HA pair and
all subscribers must be started with starting flag
"enable_statestored_ha" as true.

This patch makes following changes:
- Defined new service for Statestore HA.
- Statestored negotiates the role for HA with its peer statestore
  instance on startup.
- Create HA monitor thread:
  Active statestored sends heartbeat to standby statestored.
  Standby statestored monitors peer's connection states with their
  subscribers.
- Standby statestored sends heartbeat to subscribers with request
  for connection state between active statestore and subscribers.
  Standby statestored saves the connection state as failure detecter.
- When standby statestored lost connection with active statestore,
  it checks the connection states for active statestore, and takes over
  active role if majority of subscribers lost connections with active
  statestore.
- New active statestored sends RPC notification to all subscribers
  for new active statestored and active catalogd elected by the new
  active statestored.
- New active statestored starts sending heartbeat to its peer when it
  receives handshake from its peer.
- Active statestored enters recovery mode if it lost connections with
  its peer statestored and all subscribers. It keeps sending HA
  handshake to its peer until receiving response.
- All subscribers (impalad/catalogd/admissiond) register to two
  statestoreds.
- Subscribers report connection state for the requests from standby
  statestore.
- Subscribers switch to new active statestore when receiving RPC
  notifications from new active statestored.
- Only active statestored sends topic update messages to subscribers.
- Add a new option "enable_statestored_ha" in script
  bin/start-impala-cluster.py for starting Impala mini-cluster with
  statestored HA enabled.
- Add a new Thrift API in statestore service to disable network
  for statestored. It's only used for unit-test to simulate network
  failure. For safety, it's only working when the debug action is set
  in starting flags.

Testings:
 - Added end-to-end unit tests for statestored HA.
 - Passed core tests

Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
---
M be/generated-sources/gen-cpp/CMakeLists.txt
M be/src/catalog/catalog-server.cc
M be/src/common/global-flags.cc
M be/src/rpc/thrift-server-test.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/admissiond-env.cc
M be/src/statestore/statestore-service-client-wrapper.h
M be/src/statestore/statestore-subscriber-catalog.cc
M be/src/statestore/statestore-subscriber-catalog.h
M be/src/statestore/statestore-subscriber-client-wrapper.h
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/statestore/statestore-test.cc
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
M be/src/statestore/statestored-main.cc
M bin/start-impala-cluster.py
M common/thrift/StatestoreService.thrift
M common/thrift/metrics.json
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_statestored_ha.py
23 files changed, 2,541 insertions(+), 114 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 19
Gerrit-Owner: Wenzhe Zhou 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: Yida Wu 


[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

2023-10-24 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
..


Patch Set 19: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py@786
PS17, Line 786:   def start_statestore(self):
I'm curious why this implementation looks so different from start_catalogd.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 19
Gerrit-Owner: Wenzhe Zhou 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: Yida Wu 
Gerrit-Comment-Date: Tue, 24 Oct 2023 17:17:17 +
Gerrit-HasComments: Yes