You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Michael Smith (Code Review)" <ge...@cloudera.org> on 2026/04/01 23:36:40 UTC
[Impala-ASF-CR] IMPALA-14855: Make number of query map shards configurable
Michael Smith has uploaded this change for review. ( http://gerrit.cloudera.org:8080/24161
Change subject: IMPALA-14855: Make number of query map shards configurable
......................................................................
IMPALA-14855: Make number of query map shards configurable
Allow configuring the number of query map shards used for tracking
queries with `--num_query_map_shards`. Retains the default of 4.
Removes unused and buggy ScopedShardedMapPBRef.
Change-Id: Icacb7fc541171546925798c2c652d60052bb774b
---
M be/src/util/sharded-query-map-util.cc
M be/src/util/sharded-query-map-util.h
2 files changed, 41 insertions(+), 24 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/24161/1
--
To view, visit http://gerrit.cloudera.org:8080/24161
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Icacb7fc541171546925798c2c652d60052bb774b
Gerrit-Change-Number: 24161
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
[Impala-ASF-CR] IMPALA-14855: Make number of query map shards configurable
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/24161 )
Change subject: IMPALA-14855: Make number of query map shards configurable
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/24161/1/be/src/util/sharded-query-map-util.cc
File be/src/util/sharded-query-map-util.cc:
http://gerrit.cloudera.org:8080/#/c/24161/1/be/src/util/sharded-query-map-util.cc@30
PS1, Line 30: DEFINE_validator(num_query_map_shards, [](const char* name, uint32_t value) {
: if (value == 0) {
: std::cerr << "Invalid value for --" << name << ": " << value
: << " (must be > 0)" << std::endl;
: return false;
: }
: return true;
: });
> There are a few validators in `util/gflag-validator-util.h`, including `ge_
I'll use gt_zero.
There's some other cleanup that would be nice to do; catalog-server.cc defines both gt_0 and ValidatePositiveInt, and I believe both are identical to gt_zero. Also found ValidateBiggerThanZero in jwt-util.cc and several variants in backend-gflag-util.cc.
--
To view, visit http://gerrit.cloudera.org:8080/24161
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icacb7fc541171546925798c2c652d60052bb774b
Gerrit-Change-Number: 24161
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Thu, 02 Apr 2026 22:07:20 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14855: Make number of query map shards configurable
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/24161 )
Change subject: IMPALA-14855: Make number of query map shards configurable
......................................................................
IMPALA-14855: Make number of query map shards configurable
Allow configuring the number of query map shards used for tracking
queries with `--num_query_map_shards`. Retains the default of 4.
Removes unused and buggy ScopedShardedMapPBRef.
Change-Id: Icacb7fc541171546925798c2c652d60052bb774b
Reviewed-on: http://gerrit.cloudera.org:8080/24161
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/util/sharded-query-map-util.cc
M be/src/util/sharded-query-map-util.h
2 files changed, 35 insertions(+), 24 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/24161
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Icacb7fc541171546925798c2c652d60052bb774b
Gerrit-Change-Number: 24161
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
[Impala-ASF-CR] IMPALA-14855: Make number of query map shards configurable
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24161 )
Change subject: IMPALA-14855: Make number of query map shards configurable
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/22067/ : 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/24161
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icacb7fc541171546925798c2c652d60052bb774b
Gerrit-Change-Number: 24161
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 02 Apr 2026 00:01:42 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14855: Make number of query map shards configurable
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24161 )
Change subject: IMPALA-14855: Make number of query map shards configurable
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/24161
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icacb7fc541171546925798c2c652d60052bb774b
Gerrit-Change-Number: 24161
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Mon, 06 Apr 2026 18:13:50 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14855: Make number of query map shards configurable
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/24161 )
Change subject: IMPALA-14855: Make number of query map shards configurable
......................................................................
Patch Set 1: Code-Review+1
(2 comments)
http://gerrit.cloudera.org:8080/#/c/24161/1/be/src/util/sharded-query-map-util.h
File be/src/util/sharded-query-map-util.h:
http://gerrit.cloudera.org:8080/#/c/24161/1/be/src/util/sharded-query-map-util.h@47
PS1, Line 47: GenericShardedQueryMap
out of scope for this change, but it would nice to replace this with a wrapper around a library concurrent hash map, e.g. https://www.boost.org/doc/libs/latest/libs/unordered/doc/html/unordered/reference/concurrent_flat_map.html
the only extra feature I saw here are metric collection and error messages
http://gerrit.cloudera.org:8080/#/c/24161/1/be/src/util/sharded-query-map-util.cc
File be/src/util/sharded-query-map-util.cc:
http://gerrit.cloudera.org:8080/#/c/24161/1/be/src/util/sharded-query-map-util.cc@30
PS1, Line 30: DEFINE_validator(num_query_map_shards, [](const char* name, uint32_t value) {
: if (value == 0) {
: std::cerr << "Invalid value for --" << name << ": " << value
: << " (must be > 0)" << std::endl;
: return false;
: }
: return true;
: });
There is a function for this in catalog_server.cc, e.g DEFINE_validator(reset_metadata_lock_duration_ms, gt_0);
We could have a global one like gflag_gt_0 as this is pretty common.
--
To view, visit http://gerrit.cloudera.org:8080/24161
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icacb7fc541171546925798c2c652d60052bb774b
Gerrit-Change-Number: 24161
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 02 Apr 2026 09:33:43 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14855: Make number of query map shards configurable
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/24161 )
Change subject: IMPALA-14855: Make number of query map shards configurable
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/24161
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icacb7fc541171546925798c2c652d60052bb774b
Gerrit-Change-Number: 24161
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Fri, 03 Apr 2026 16:16:27 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14855: Make number of query map shards configurable
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/24161 )
Change subject: IMPALA-14855: Make number of query map shards configurable
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/24161/1/be/src/util/sharded-query-map-util.h
File be/src/util/sharded-query-map-util.h:
http://gerrit.cloudera.org:8080/#/c/24161/1/be/src/util/sharded-query-map-util.h@a184
PS1, Line 184:
This signature was wrong, it should have taken a UniqueIdPB and ShardedQueryPBMap. But it's also obviously unused, so I removed the code instead.
--
To view, visit http://gerrit.cloudera.org:8080/24161
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icacb7fc541171546925798c2c652d60052bb774b
Gerrit-Change-Number: 24161
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Thu, 02 Apr 2026 15:52:03 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14855: Make number of query map shards configurable
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24161 )
Change subject: IMPALA-14855: Make number of query map shards configurable
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/22075/ : 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/24161
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icacb7fc541171546925798c2c652d60052bb774b
Gerrit-Change-Number: 24161
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Fri, 03 Apr 2026 00:38:05 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14855: Make number of query map shards configurable
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24161 )
Change subject: IMPALA-14855: Make number of query map shards configurable
......................................................................
Patch Set 3:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/13302/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/24161
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icacb7fc541171546925798c2c652d60052bb774b
Gerrit-Change-Number: 24161
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Mon, 06 Apr 2026 18:13:51 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14855: Make number of query map shards configurable
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24161 )
Change subject: IMPALA-14855: Make number of query map shards configurable
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/24161
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icacb7fc541171546925798c2c652d60052bb774b
Gerrit-Change-Number: 24161
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Mon, 06 Apr 2026 22:37:24 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14855: Make number of query map shards configurable
Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/24161 )
Change subject: IMPALA-14855: Make number of query map shards configurable
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/24161/1/be/src/util/sharded-query-map-util.cc
File be/src/util/sharded-query-map-util.cc:
http://gerrit.cloudera.org:8080/#/c/24161/1/be/src/util/sharded-query-map-util.cc@30
PS1, Line 30: DEFINE_validator(num_query_map_shards, [](const char* name, uint32_t value) {
: if (value == 0) {
: std::cerr << "Invalid value for --" << name << ": " << value
: << " (must be > 0)" << std::endl;
: return false;
: }
: return true;
: });
> There is a function for this in catalog_server.cc, e.g DEFINE_validator(res
There are a few validators in `util/gflag-validator-util.h`, including `ge_zero`.
--
To view, visit http://gerrit.cloudera.org:8080/24161
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icacb7fc541171546925798c2c652d60052bb774b
Gerrit-Change-Number: 24161
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Comment-Date: Thu, 02 Apr 2026 15:02:31 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14855: Make number of query map shards configurable
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Jason Fehr, Joe McDonnell, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/24161
to look at the new patch set (#2).
Change subject: IMPALA-14855: Make number of query map shards configurable
......................................................................
IMPALA-14855: Make number of query map shards configurable
Allow configuring the number of query map shards used for tracking
queries with `--num_query_map_shards`. Retains the default of 4.
Removes unused and buggy ScopedShardedMapPBRef.
Change-Id: Icacb7fc541171546925798c2c652d60052bb774b
---
M be/src/util/sharded-query-map-util.cc
M be/src/util/sharded-query-map-util.h
2 files changed, 35 insertions(+), 24 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/24161/2
--
To view, visit http://gerrit.cloudera.org:8080/24161
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icacb7fc541171546925798c2c652d60052bb774b
Gerrit-Change-Number: 24161
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
[Impala-ASF-CR] IMPALA-14855: Make number of query map shards configurable
Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/24161 )
Change subject: IMPALA-14855: Make number of query map shards configurable
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/24161
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icacb7fc541171546925798c2c652d60052bb774b
Gerrit-Change-Number: 24161
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Mon, 06 Apr 2026 15:52:26 +0000
Gerrit-HasComments: No