You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Riza Suminto (Code Review)" <ge...@cloudera.org> on 2023/08/22 18:09:49 UTC

[Impala-ASF-CR] IMPALA-12387: PartialUpdates is misleading for LOCAL filter

Riza Suminto has uploaded this change for review. ( http://gerrit.cloudera.org:8080/20397


Change subject: IMPALA-12387: PartialUpdates is misleading for LOCAL filter
......................................................................

IMPALA-12387: PartialUpdates is misleading for LOCAL filter

In filter table, PartialUpdates is intended to mark if coordinator
receive only partial update from contributing fragments. This can be
misleading for LOCAL filter in column "Min value", "Max value", and
"In-list size", because LOCAL filter does not aggregate in coordinator
anymore. Thus, coordinator does not receive any filter update.

This patch mark such column value as "LOCAL" if no global aggregation is
expected in coordinator.

Testing:
- Pass core tests.

Change-Id: I56078a458799671246ff90b831e5ecebd04a78e8
---
M be/src/runtime/coordinator.cc
M testdata/workloads/functional-query/queries/QueryTest/in_list_filters.test
2 files changed, 12 insertions(+), 6 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I56078a458799671246ff90b831e5ecebd04a78e8
Gerrit-Change-Number: 20397
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>

[Impala-ASF-CR] IMPALA-12387: PartialUpdates is misleading for LOCAL filter

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20397 )

Change subject: IMPALA-12387: PartialUpdates is misleading for LOCAL filter
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/13812/ : 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/20397
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I56078a458799671246ff90b831e5ecebd04a78e8
Gerrit-Change-Number: 20397
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 22 Aug 2023 18:30:03 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12387: PartialUpdates is misleading for LOCAL filter

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/20397 )

Change subject: IMPALA-12387: PartialUpdates is misleading for LOCAL filter
......................................................................


Patch Set 2:

(1 comment)

Thank you for the +2!

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

http://gerrit.cloudera.org:8080/#/c/20397/1/testdata/workloads/functional-query/queries/QueryTest/in_list_filters.test@181
PS1, Line 181: LOCAL
> Sorry, I mean the size of the IN-list filter, i.e. how many items in the li
I'll try to look at it separately next time.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I56078a458799671246ff90b831e5ecebd04a78e8
Gerrit-Change-Number: 20397
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 25 Aug 2023 16:07:42 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12387: PartialUpdates is misleading for LOCAL filter

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/20397 )

Change subject: IMPALA-12387: PartialUpdates is misleading for LOCAL filter
......................................................................


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/20397/1/testdata/workloads/functional-query/queries/QueryTest/in_list_filters.test@181
PS1, Line 181: LOCAL
> This is ok to me. Just curious, is it doable to get the stats if the fragme
For global filter aggregating in coordinator, I think column "Pending (Expected)" is quite representative to show how many total contributors for a filter.
For more granular tracing, we'll need to add new field to do the counting at least at PublishFilterParamsPB.
Not sure if its worth to hack since it will be only interesting for filters that quickly become ALL_TRUE / ALL_FALSE.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I56078a458799671246ff90b831e5ecebd04a78e8
Gerrit-Change-Number: 20397
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 24 Aug 2023 22:59:57 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12387: PartialUpdates is misleading for LOCAL filter

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20397 )

Change subject: IMPALA-12387: PartialUpdates is misleading for LOCAL filter
......................................................................


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I56078a458799671246ff90b831e5ecebd04a78e8
Gerrit-Change-Number: 20397
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 25 Aug 2023 16:07:16 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12387: PartialUpdates is misleading for LOCAL filter

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20397 )

Change subject: IMPALA-12387: PartialUpdates is misleading for LOCAL filter
......................................................................


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I56078a458799671246ff90b831e5ecebd04a78e8
Gerrit-Change-Number: 20397
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 25 Aug 2023 16:07:17 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12387: PartialUpdates is misleading for LOCAL filter

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/20397 )

Change subject: IMPALA-12387: PartialUpdates is misleading for LOCAL filter
......................................................................


Patch Set 1: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I56078a458799671246ff90b831e5ecebd04a78e8
Gerrit-Change-Number: 20397
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 24 Aug 2023 17:40:05 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12387: PartialUpdates is misleading for LOCAL filter

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20397 )

Change subject: IMPALA-12387: PartialUpdates is misleading for LOCAL filter
......................................................................


Patch Set 1: Code-Review+1

(1 comment)

LGTM

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

http://gerrit.cloudera.org:8080/#/c/20397/1/testdata/workloads/functional-query/queries/QueryTest/in_list_filters.test@181
PS1, Line 181: LOCAL
This is ok to me. Just curious, is it doable to get the stats if the fragment also runs on coordinator? We have details in the per node sections. It'd be nice if we can show them in this table.

    Per Node Profiles:
      quanlong-OptiPlex-BJ:27000:
        Filter 0 arrival with 1 items: 333ms
        Filter 1 arrival with 8 items: 339ms



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I56078a458799671246ff90b831e5ecebd04a78e8
Gerrit-Change-Number: 20397
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 23 Aug 2023 12:25:17 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12387: PartialUpdates is misleading for LOCAL filter

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20397 )

Change subject: IMPALA-12387: PartialUpdates is misleading for LOCAL filter
......................................................................


Patch Set 1: Code-Review+2

(1 comment)

Carrying Wenzhe's +1.

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

http://gerrit.cloudera.org:8080/#/c/20397/1/testdata/workloads/functional-query/queries/QueryTest/in_list_filters.test@181
PS1, Line 181: LOCAL
> For global filter aggregating in coordinator, I think column "Pending (Expe
Sorry, I mean the size of the IN-list filter, i.e. how many items in the list. For MinMax filters, I think we can also get the min/max values. It's an improvement that we can do it in future.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I56078a458799671246ff90b831e5ecebd04a78e8
Gerrit-Change-Number: 20397
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 25 Aug 2023 00:04:28 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12387: PartialUpdates is misleading for LOCAL filter

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20397 )

Change subject: IMPALA-12387: PartialUpdates is misleading for LOCAL filter
......................................................................


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I56078a458799671246ff90b831e5ecebd04a78e8
Gerrit-Change-Number: 20397
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 25 Aug 2023 20:15:43 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12387: PartialUpdates is misleading for LOCAL filter

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/20397 )

Change subject: IMPALA-12387: PartialUpdates is misleading for LOCAL filter
......................................................................

IMPALA-12387: PartialUpdates is misleading for LOCAL filter

In filter table, PartialUpdates is intended to mark if coordinator
receive only partial update from contributing fragments. This can be
misleading for LOCAL filter in column "Min value", "Max value", and
"In-list size", because LOCAL filter does not aggregate in coordinator
anymore. Thus, coordinator does not receive any filter update.

This patch mark such column value as "LOCAL" if no global aggregation is
expected in coordinator.

Testing:
- Pass core tests.

Change-Id: I56078a458799671246ff90b831e5ecebd04a78e8
Reviewed-on: http://gerrit.cloudera.org:8080/20397
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/runtime/coordinator.cc
M testdata/workloads/functional-query/queries/QueryTest/in_list_filters.test
2 files changed, 12 insertions(+), 6 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I56078a458799671246ff90b831e5ecebd04a78e8
Gerrit-Change-Number: 20397
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>