You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Pooja Nilangekar (Code Review)" <ge...@cloudera.org> on 2019/02/08 01:22:06 UTC
[Impala-ASF-CR] IMPALA-8064: Improve observability of wait times for runtime filters
Pooja Nilangekar has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12401
Change subject: IMPALA-8064: Improve observability of wait times for runtime filters
......................................................................
IMPALA-8064: Improve observability of wait times for runtime filters
This change is a diagnostic fix to improve the wait times logged
for runtime filters. Previously, the wait time was calculated with
respect to the ScanNode::Init() function while duration was logged
with respect to the ScanNode::WaitForRuntimeFilters() function.
This change ensures that the wait time logged is the time elapsed
since filter registration.
From my analysis of the logs of the failed tests, I believe the
filters are actually waiting for the specified time but logging
the duration incorrectly. The solution would be increase the wait
time further. This change would help validate this hypothesis.
Change-Id: I28fd45e75c773bc01d424f5a179ae186ee9b7469
---
M be/src/exec/scan-node.cc
M be/src/runtime/runtime-filter.h
2 files changed, 11 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/12401/1
--
To view, visit http://gerrit.cloudera.org:8080/12401
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I28fd45e75c773bc01d424f5a179ae186ee9b7469
Gerrit-Change-Number: 12401
Gerrit-PatchSet: 1
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
[Impala-ASF-CR] IMPALA-8064: Improve observability of wait times for runtime filters
Posted by "Pooja Nilangekar (Code Review)" <ge...@cloudera.org>.
Pooja Nilangekar has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/12401 )
Change subject: IMPALA-8064: Improve observability of wait times for runtime filters
......................................................................
IMPALA-8064: Improve observability of wait times for runtime filters
This change is a diagnostic fix to improve the wait times logged
for runtime filters. The filter wait time counts against the
elapsed time since the filter's registration in ScanNode::Init()
while the duration logged in ScanNode::WaitForRuntimeFilters() is
the time spent in the function waiting for all the filters to
arrive. This could be misleading as it doesn't account for the
elapsed time spent between ScanNode::Init() and
ScanNode::WaitForRuntimeFilters(). This change logs the maximum
arrival delay for any runtime filter to arrive.
From my analysis of the logs of the failed tests, I believe the
filters are actually waiting for the specified time but logging
the duration incorrectly. The solution would be to increase the
wait time further. This change would help validate this
hypothesis.
Change-Id: I28fd45e75c773bc01d424f5a179ae186ee9b7469
---
M be/src/exec/scan-node.cc
M be/src/runtime/runtime-filter-bank.cc
M be/src/runtime/runtime-filter.h
3 files changed, 17 insertions(+), 12 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/12401/3
--
To view, visit http://gerrit.cloudera.org:8080/12401
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I28fd45e75c773bc01d424f5a179ae186ee9b7469
Gerrit-Change-Number: 12401
Gerrit-PatchSet: 3
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
[Impala-ASF-CR] IMPALA-8064: Improve observability of wait times for runtime filters
Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/12401 )
Change subject: IMPALA-8064: Improve observability of wait times for runtime filters
......................................................................
Patch Set 3: Code-Review+2
> > Patch Set 3:
> >
> > Totally forgot about testing, do you think it makes sense to add
> tests for both cases:
> > all filters arrive: arrival delay < runtime_filter_wait_time_ms
> > not all filters arrive: arrival delay >= runtime_filter_wait_time_ms
>
> Actually, I tried to test this change manually. For the "filters
> don't arrive on time" case, I set a low wait time. But that was not
> consistent, it somewhat depended on the system load and running the
> same query in succession didn't help much because I think the scan
> node hit the file system cache.
>
> Also, for all filters arrive: we may also have the case where
> arrival delay >= runtime_filter_wait_time_ms since we always sleep
> for SLEEP_PERIOD_MS and that may not be exactly divisible by
> runtime_filter_wait_time_ms. So in reality, the only case we can
> test for is:
> not all filters arrive: arrival delay >= runtime_filter_wait_time_ms
> I'm not sure of a way to reliably reproduce this. (I don't want to
> add a flaky test which trying to fix another).
>
> Do you have any suggestions for adding tests?
Thanks for the explanation. I dont think there is anyway of reliably producing both cases
--
To view, visit http://gerrit.cloudera.org:8080/12401
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I28fd45e75c773bc01d424f5a179ae186ee9b7469
Gerrit-Change-Number: 12401
Gerrit-PatchSet: 3
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Comment-Date: Tue, 12 Feb 2019 01:08:53 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8064: Improve observability of wait times for runtime filters
Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/12401 )
Change subject: IMPALA-8064: Improve observability of wait times for runtime filters
......................................................................
Patch Set 3:
Totally forgot about testing, do you think it makes sense to add tests for both cases:
all filters arrive: arrival delay < runtime_filter_wait_time_ms
not all filters arrive: arrival delay >= runtime_filter_wait_time_ms
--
To view, visit http://gerrit.cloudera.org:8080/12401
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I28fd45e75c773bc01d424f5a179ae186ee9b7469
Gerrit-Change-Number: 12401
Gerrit-PatchSet: 3
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Comment-Date: Tue, 12 Feb 2019 00:02:57 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8064: Improve observability of wait times for runtime filters
Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/12401 )
Change subject: IMPALA-8064: Improve observability of wait times for runtime filters
......................................................................
Patch Set 2:
@Pooja Please make sure you address my comment on L191 in scan-node.cc before you start GVO.
--
To view, visit http://gerrit.cloudera.org:8080/12401
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I28fd45e75c773bc01d424f5a179ae186ee9b7469
Gerrit-Change-Number: 12401
Gerrit-PatchSet: 2
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Comment-Date: Mon, 11 Feb 2019 22:52:27 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8064: Improve observability of wait times for runtime filters
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12401 )
Change subject: IMPALA-8064: Improve observability of wait times for runtime filters
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2073/ : 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/12401
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I28fd45e75c773bc01d424f5a179ae186ee9b7469
Gerrit-Change-Number: 12401
Gerrit-PatchSet: 2
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Comment-Date: Mon, 11 Feb 2019 20:49:37 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8064: Improve observability of wait times for runtime filters
Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/12401 )
Change subject: IMPALA-8064: Improve observability of wait times for runtime filters
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/12401/2/be/src/exec/scan-node.cc
File be/src/exec/scan-node.cc:
http://gerrit.cloudera.org:8080/#/c/12401/2/be/src/exec/scan-node.cc@172
PS2, Line 172: 32
int64 here and for 'end'. int32 will work for wait_time_ms() since delay is a difference between 2 time values, but start and end store the absolute result from MonotonicMillis() which is usually the time since system start and can be a larger number than what int32 can store
http://gerrit.cloudera.org:8080/#/c/12401/2/be/src/exec/scan-node.cc@191
PS2, Line 191: Maximum runtime filter wait time
reading it now, this might sound like its just printing out the max_runtime_filter_wait_time query option instead the actual wait time since it can be equal to the query option in some cases where the filters dont arrive on time. Unfortunately, i dont really have a better suggestion without being verbose.
--
To view, visit http://gerrit.cloudera.org:8080/12401
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I28fd45e75c773bc01d424f5a179ae186ee9b7469
Gerrit-Change-Number: 12401
Gerrit-PatchSet: 2
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Comment-Date: Mon, 11 Feb 2019 21:50:21 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8064: Improve observability of wait times for runtime filters
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12401 )
Change subject: IMPALA-8064: Improve observability of wait times for runtime filters
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/12401
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I28fd45e75c773bc01d424f5a179ae186ee9b7469
Gerrit-Change-Number: 12401
Gerrit-PatchSet: 4
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Comment-Date: Tue, 12 Feb 2019 01:09:16 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8064: Improve observability of wait times for runtime filters
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12401 )
Change subject: IMPALA-8064: Improve observability of wait times for runtime filters
......................................................................
Patch Set 4:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3761/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/12401
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I28fd45e75c773bc01d424f5a179ae186ee9b7469
Gerrit-Change-Number: 12401
Gerrit-PatchSet: 4
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Comment-Date: Tue, 12 Feb 2019 01:09:17 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8064: Improve observability of wait times for runtime filters
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12401 )
Change subject: IMPALA-8064: Improve observability of wait times for runtime filters
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2034/ : 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/12401
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I28fd45e75c773bc01d424f5a179ae186ee9b7469
Gerrit-Change-Number: 12401
Gerrit-PatchSet: 1
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Feb 2019 01:58:22 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8064: Improve observability of wait times for runtime filters
Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/12401 )
Change subject: IMPALA-8064: Improve observability of wait times for runtime filters
......................................................................
Patch Set 1:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/12401/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12401/1//COMMIT_MSG@10
PS1, Line 10: Previously, the wait time was calculated with
: respect to the ScanNode::Init() function while duration was logged
: with respect to the ScanNode::WaitForRuntimeFilters() function.
The filter wait time counts against the elapsed time since the filter's registration in ScanNode::Init() while duration logged in ScanNode::WaitForRuntimeFilters() is the time spent in the function waiting for all filters to arrive. This could be misleading as it doesn't account for the elapsed time spent between ScanNode::Init() and ScanNode::WaitForRuntimeFilters().
http://gerrit.cloudera.org:8080/#/c/12401/1//COMMIT_MSG@18
PS1, Line 18:
to
http://gerrit.cloudera.org:8080/#/c/12401/1/be/src/exec/scan-node.cc
File be/src/exec/scan-node.cc:
http://gerrit.cloudera.org:8080/#/c/12401/1/be/src/exec/scan-node.cc@179
PS1, Line 179: max_wait_time = max(max_wait_time, ctx.filter->time_elapsed());
If I understand it correctly, the point of these changes is to reflect the fact that "FLAGS_runtime_filter_wait_time_ms" is essentially the maximum tolerable elapsed time between the filter's registration and arrival time, which potentially is longer than the elapsed time spent in this function.
While I agree it's useful to show the maximum elapsed time between registration and arrival for all filters, it seems that the original wait time here is also useful for helping us understand the actual wall clock time spent in this function waiting for filters to arrive. The time spent in this function directly adds to the delay on issuing the initial scan ranges and thus how fast we return rows.
So, what do you think about keeping the original wait time while calling the max elapsed time between registration and arrival "max arrival delay".
http://gerrit.cloudera.org:8080/#/c/12401/1/be/src/runtime/runtime-filter.h
File be/src/runtime/runtime-filter.h:
http://gerrit.cloudera.org:8080/#/c/12401/1/be/src/runtime/runtime-filter.h@79
PS1, Line 79: /// Returns the amount of time waited since registration for the filter to
, in ms,
http://gerrit.cloudera.org:8080/#/c/12401/1/be/src/runtime/runtime-filter.h@81
PS1, Line 81: int32_t
Either set this to int64_t or convert time_elapsed() below to return int32_t. I suppose int32_t can tolerate delays up to 24 days.
http://gerrit.cloudera.org:8080/#/c/12401/1/be/src/runtime/runtime-filter.h@86
PS1, Line 86: time
in millisecond
--
To view, visit http://gerrit.cloudera.org:8080/12401
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I28fd45e75c773bc01d424f5a179ae186ee9b7469
Gerrit-Change-Number: 12401
Gerrit-PatchSet: 1
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Feb 2019 07:37:34 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8064: Improve observability of wait times for runtime filters
Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/12401 )
Change subject: IMPALA-8064: Improve observability of wait times for runtime filters
......................................................................
Patch Set 2: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12401/2/be/src/exec/scan-node.cc
File be/src/exec/scan-node.cc:
http://gerrit.cloudera.org:8080/#/c/12401/2/be/src/exec/scan-node.cc@191
PS2, Line 191: Maximum runtime filter wait time
> How about calling it maximum runtime filter arrival delay?
I suppose "runtime filter" here may be omitted (i.e. calling it maximum arrival delay") as this info string should make it plentifully clear it's related to runtime filter. Honestly speaking, I am doubtful that anyone beyond the core group of Impala developers can make sense of this metric.
--
To view, visit http://gerrit.cloudera.org:8080/12401
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I28fd45e75c773bc01d424f5a179ae186ee9b7469
Gerrit-Change-Number: 12401
Gerrit-PatchSet: 2
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Comment-Date: Mon, 11 Feb 2019 22:46:03 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8064: Improve observability of wait times for runtime filters
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/12401 )
Change subject: IMPALA-8064: Improve observability of wait times for runtime filters
......................................................................
IMPALA-8064: Improve observability of wait times for runtime filters
This change is a diagnostic fix to improve the wait times logged
for runtime filters. The filter wait time counts against the
elapsed time since the filter's registration in ScanNode::Init()
while the duration logged in ScanNode::WaitForRuntimeFilters() is
the time spent in the function waiting for all the filters to
arrive. This could be misleading as it doesn't account for the
elapsed time spent between ScanNode::Init() and
ScanNode::WaitForRuntimeFilters(). This change logs the maximum
arrival delay for any runtime filter to arrive.
From my analysis of the logs of the failed tests, I believe the
filters are actually waiting for the specified time but logging
the duration incorrectly. The solution would be to increase the
wait time further. This change would help validate this
hypothesis.
Change-Id: I28fd45e75c773bc01d424f5a179ae186ee9b7469
Reviewed-on: http://gerrit.cloudera.org:8080/12401
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/exec/scan-node.cc
M be/src/runtime/runtime-filter-bank.cc
M be/src/runtime/runtime-filter.h
3 files changed, 17 insertions(+), 12 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/12401
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I28fd45e75c773bc01d424f5a179ae186ee9b7469
Gerrit-Change-Number: 12401
Gerrit-PatchSet: 5
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
[Impala-ASF-CR] IMPALA-8064: Improve observability of wait times for runtime filters
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12401 )
Change subject: IMPALA-8064: Improve observability of wait times for runtime filters
......................................................................
Patch Set 4: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/12401
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I28fd45e75c773bc01d424f5a179ae186ee9b7469
Gerrit-Change-Number: 12401
Gerrit-PatchSet: 4
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Comment-Date: Tue, 12 Feb 2019 05:16:26 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8064: Improve observability of wait times for runtime filters
Posted by "Pooja Nilangekar (Code Review)" <ge...@cloudera.org>.
Pooja Nilangekar has posted comments on this change. ( http://gerrit.cloudera.org:8080/12401 )
Change subject: IMPALA-8064: Improve observability of wait times for runtime filters
......................................................................
Patch Set 3:
> Patch Set 3:
>
> Totally forgot about testing, do you think it makes sense to add tests for both cases:
> all filters arrive: arrival delay < runtime_filter_wait_time_ms
> not all filters arrive: arrival delay >= runtime_filter_wait_time_ms
Actually, I tried to test this change manually. For the "filters don't arrive on time" case, I set a low wait time. But that was not consistent, it somewhat depended on the system load and running the same query in succession didn't help much because I think the scan node hit the file system cache.
Also, for all filters arrive: we may also have the case where arrival delay >= runtime_filter_wait_time_ms since we always sleep for SLEEP_PERIOD_MS and that may not be exactly divisible by runtime_filter_wait_time_ms. So in reality, the only case we can test for is:
not all filters arrive: arrival delay >= runtime_filter_wait_time_ms
I'm not sure of a way to reliably reproduce this. (I don't want to add a flaky test which trying to fix another).
Do you have any suggestions for adding tests?
--
To view, visit http://gerrit.cloudera.org:8080/12401
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I28fd45e75c773bc01d424f5a179ae186ee9b7469
Gerrit-Change-Number: 12401
Gerrit-PatchSet: 3
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Comment-Date: Tue, 12 Feb 2019 00:33:24 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8064: Improve observability of wait times for runtime filters
Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/12401 )
Change subject: IMPALA-8064: Improve observability of wait times for runtime filters
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/12401
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I28fd45e75c773bc01d424f5a179ae186ee9b7469
Gerrit-Change-Number: 12401
Gerrit-PatchSet: 2
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Comment-Date: Mon, 11 Feb 2019 22:50:04 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8064: Improve observability of wait times for runtime filters
Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/12401 )
Change subject: IMPALA-8064: Improve observability of wait times for runtime filters
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/12401/1/be/src/exec/scan-node.cc
File be/src/exec/scan-node.cc:
http://gerrit.cloudera.org:8080/#/c/12401/1/be/src/exec/scan-node.cc@179
PS1, Line 179: max_wait_time = max(max_wait_time, ctx.filter->time_elapsed());
> If I understand it correctly, the point of these changes is to reflect the
Thanks for the context Michael. Based on this it definitely seems like a good idea to add another time metric to the profile, and perhaps re-word the infoString to make it explicitly clear what each means. Naming it to something like "Max runtime filter wait time:" so that it directly relates to the query options as well, making it abundantly clear.
http://gerrit.cloudera.org:8080/#/c/12401/1/be/src/runtime/runtime-filter.h
File be/src/runtime/runtime-filter.h:
http://gerrit.cloudera.org:8080/#/c/12401/1/be/src/runtime/runtime-filter.h@89
PS1, Line 89: time_elapsed
you can probably replace arrival_delay() with this method and rename it to something like wait_time_ms(). The "Returns 0 if filter has not yet arrived" part in arrival_delay() is not used anywhere anyways.
--
To view, visit http://gerrit.cloudera.org:8080/12401
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I28fd45e75c773bc01d424f5a179ae186ee9b7469
Gerrit-Change-Number: 12401
Gerrit-PatchSet: 1
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Feb 2019 18:13:31 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8064: Improve observability of wait times for runtime filters
Posted by "Pooja Nilangekar (Code Review)" <ge...@cloudera.org>.
Pooja Nilangekar has posted comments on this change. ( http://gerrit.cloudera.org:8080/12401 )
Change subject: IMPALA-8064: Improve observability of wait times for runtime filters
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12401/2/be/src/exec/scan-node.cc
File be/src/exec/scan-node.cc:
http://gerrit.cloudera.org:8080/#/c/12401/2/be/src/exec/scan-node.cc@191
PS2, Line 191: Maximum runtime filter wait time
> reading it now, this might sound like its just printing out the max_runtime
How about calling it maximum runtime filter arrival delay?
--
To view, visit http://gerrit.cloudera.org:8080/12401
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I28fd45e75c773bc01d424f5a179ae186ee9b7469
Gerrit-Change-Number: 12401
Gerrit-PatchSet: 2
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Comment-Date: Mon, 11 Feb 2019 21:55:15 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8064: Improve observability of wait times for runtime filters
Posted by "Pooja Nilangekar (Code Review)" <ge...@cloudera.org>.
Pooja Nilangekar has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/12401 )
Change subject: IMPALA-8064: Improve observability of wait times for runtime filters
......................................................................
IMPALA-8064: Improve observability of wait times for runtime filters
This change is a diagnostic fix to improve the wait times logged
for runtime filters. The filter wait time counts against the
elapsed time since the filter's registration in ScanNode::Init()
while the duration logged in ScanNode::WaitForRuntimeFilters() is
the time spent in the function waiting for all the filters to
arrive. This could be misleading as it doesn't account for the
elapsed time spent between ScanNode::Init() and
ScanNode::WaitForRuntimeFilters(). This change logs the maximum
arrival delay for any runtime filter to arrive.
From my analysis of the logs of the failed tests, I believe the
filters are actually waiting for the specified time but logging
the duration incorrectly. The solution would be to increase the
wait time further. This change would help validate this
hypothesis.
Change-Id: I28fd45e75c773bc01d424f5a179ae186ee9b7469
---
M be/src/exec/scan-node.cc
M be/src/runtime/runtime-filter-bank.cc
M be/src/runtime/runtime-filter.h
3 files changed, 24 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/12401/2
--
To view, visit http://gerrit.cloudera.org:8080/12401
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I28fd45e75c773bc01d424f5a179ae186ee9b7469
Gerrit-Change-Number: 12401
Gerrit-PatchSet: 2
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
[Impala-ASF-CR] IMPALA-8064: Improve observability of wait times for runtime filters
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12401 )
Change subject: IMPALA-8064: Improve observability of wait times for runtime filters
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2074/ : 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/12401
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I28fd45e75c773bc01d424f5a179ae186ee9b7469
Gerrit-Change-Number: 12401
Gerrit-PatchSet: 3
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Comment-Date: Tue, 12 Feb 2019 00:24:37 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8064: Improve observability of wait times for runtime filters
Posted by "Pooja Nilangekar (Code Review)" <ge...@cloudera.org>.
Pooja Nilangekar has posted comments on this change. ( http://gerrit.cloudera.org:8080/12401 )
Change subject: IMPALA-8064: Improve observability of wait times for runtime filters
......................................................................
Patch Set 3:
(1 comment)
> Patch Set 2:
>
> @Pooja Please make sure you address my comment on L191 in scan-node.cc before you start GVO.
Done.
http://gerrit.cloudera.org:8080/#/c/12401/2/be/src/exec/scan-node.cc
File be/src/exec/scan-node.cc:
http://gerrit.cloudera.org:8080/#/c/12401/2/be/src/exec/scan-node.cc@191
PS2, Line 191:
> works for me
Done
--
To view, visit http://gerrit.cloudera.org:8080/12401
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I28fd45e75c773bc01d424f5a179ae186ee9b7469
Gerrit-Change-Number: 12401
Gerrit-PatchSet: 3
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Comment-Date: Mon, 11 Feb 2019 23:38:42 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8064: Improve observability of wait times for runtime filters
Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/12401 )
Change subject: IMPALA-8064: Improve observability of wait times for runtime filters
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12401/2/be/src/exec/scan-node.cc
File be/src/exec/scan-node.cc:
http://gerrit.cloudera.org:8080/#/c/12401/2/be/src/exec/scan-node.cc@191
PS2, Line 191: Maximum runtime filter wait time
> I suppose "runtime filter" here may be omitted (i.e. calling it maximum arr
works for me
--
To view, visit http://gerrit.cloudera.org:8080/12401
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I28fd45e75c773bc01d424f5a179ae186ee9b7469
Gerrit-Change-Number: 12401
Gerrit-PatchSet: 2
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Comment-Date: Mon, 11 Feb 2019 22:49:47 +0000
Gerrit-HasComments: Yes