You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Joe McDonnell (Code Review)" <ge...@cloudera.org> on 2023/05/16 18:36:33 UTC
[Impala-ASF-CR] IMPALA-10180: Add summary stats for client fetch wait time
Joe McDonnell has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19897
Change subject: IMPALA-10180: Add summary stats for client fetch wait time
......................................................................
IMPALA-10180: Add summary stats for client fetch wait time
This adds ClientFetchWaitTimeStats to the runtime profile
to track the min/max/# of samples for ClientFetchWaitTimer.
Here is some sample output:
- ClientFetchWaitTimeStats: (Avg: 161.554ms ; Min: 101.411ms ; Max: 461.728ms ; Number of samples: 6)
- ClientFetchWaitTimer: 969.326ms
This also fixes the definition of ClientFetchWaitTimer to avoid
including time after end of fetch. When the client is closing
the query, Finalize() gets called. The Finalize() call should
only add extra client wait time if fetch has not completed.
Testing:
- Added test cases in query_test/test_fetch.py with specific
numbers of fetches and verification of the statistics.
Change-Id: I9ca525285e03c7b51b04ac292f7b3531e6178218
---
M be/src/service/client-request-state.cc
M be/src/service/client-request-state.h
M tests/query_test/test_fetch.py
3 files changed, 101 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/97/19897/1
--
To view, visit http://gerrit.cloudera.org:8080/19897
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9ca525285e03c7b51b04ac292f7b3531e6178218
Gerrit-Change-Number: 19897
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
[Impala-ASF-CR] IMPALA-10180: Add summary stats for client fetch wait time
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/19897 )
Change subject: IMPALA-10180: Add summary stats for client fetch wait time
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/19897
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9ca525285e03c7b51b04ac292f7b3531e6178218
Gerrit-Change-Number: 19897
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 May 2023 10:14:12 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10180: Add summary stats for client fetch wait time
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/19897 )
Change subject: IMPALA-10180: Add summary stats for client fetch wait time
......................................................................
IMPALA-10180: Add summary stats for client fetch wait time
This adds ClientFetchWaitTimeStats to the runtime profile
to track the min/max/# of samples for ClientFetchWaitTimer.
Here is some sample output:
- ClientFetchWaitTimeStats: (Avg: 161.554ms ; Min: 101.411ms ; Max: 461.728ms ; Number of samples: 6)
- ClientFetchWaitTimer: 969.326ms
This also fixes the definition of ClientFetchWaitTimer to avoid
including time after end of fetch. When the client is closing
the query, Finalize() gets called. The Finalize() call should
only add extra client wait time if fetch has not completed.
Testing:
- Added test cases in query_test/test_fetch.py with specific
numbers of fetches and verification of the statistics.
- The test cases make use of a new function for parsing
summary stats for timers, and this also gets its own test
case.
Change-Id: I9ca525285e03c7b51b04ac292f7b3531e6178218
Reviewed-on: http://gerrit.cloudera.org:8080/19897
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Csaba Ringhofer <cs...@cloudera.com>
---
M be/src/service/client-request-state.cc
M be/src/service/client-request-state.h
M tests/infra/test_utils.py
M tests/query_test/test_fetch.py
M tests/util/parse_util.py
5 files changed, 180 insertions(+), 3 deletions(-)
Approvals:
Impala Public Jenkins: Verified
Csaba Ringhofer: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/19897
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I9ca525285e03c7b51b04ac292f7b3531e6178218
Gerrit-Change-Number: 19897
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
[Impala-ASF-CR] IMPALA-10180: Add summary stats for client fetch wait time
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/19897 )
Change subject: IMPALA-10180: Add summary stats for client fetch wait time
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19897/1/tests/query_test/test_fetch.py
File tests/query_test/test_fetch.py:
http://gerrit.cloudera.org:8080/#/c/19897/1/tests/query_test/test_fetch.py@134
PS1, Line 134: assert fetch_time_stats and len(fetch_time_stats.groups()) == 1
:
: # Verify the number of samples
: number_of_samples_re = re.search("Number of samples: (.*)", fetch_time_stats.group(1))
: assert number_of_samples_re and len(number_of_samples_re.groups()) == 1
: number_of_samples = int(number_of_samples_re.group(1).strip(')'))
: # There are 5 fetches and the finalization sample for a total of 6.
: assert number_of_samples == 6
:
: # The 2.5 second sleep does count for an incomplete fetch, verify the max is higher.
: max_time = re.search("Max: (.*) ;", fetch_time_stats.group(1))
: assert max_time and len(max_time.groups()) == 1
> optional: extract the time stats parsing logic to a function, e.g. in tests
Added a new function in tests/parse_util.py to parse summary stats for times and switched this to use it.
It duplicates some code from the bytes version, but it has different regexes and type conversions, so I kept it separate.
--
To view, visit http://gerrit.cloudera.org:8080/19897
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9ca525285e03c7b51b04ac292f7b3531e6178218
Gerrit-Change-Number: 19897
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 May 2023 00:21:47 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10180: Add summary stats for client fetch wait time
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/19897 )
Change subject: IMPALA-10180: Add summary stats for client fetch wait time
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19897/1/tests/query_test/test_fetch.py
File tests/query_test/test_fetch.py:
http://gerrit.cloudera.org:8080/#/c/19897/1/tests/query_test/test_fetch.py@134
PS1, Line 134: assert fetch_time_stats and len(fetch_time_stats.groups()) == 1
:
: # Verify the number of samples
: number_of_samples_re = re.search("Number of samples: (.*)", fetch_time_stats.group(1))
: assert number_of_samples_re and len(number_of_samples_re.groups()) == 1
: number_of_samples = int(number_of_samples_re.group(1).strip(')'))
: # There are 5 fetches and the finalization sample for a total of 6.
: assert number_of_samples == 6
:
: # The 2.5 second sleep does count for an incomplete fetch, verify the max is higher.
: max_time = re.search("Max: (.*) ;", fetch_time_stats.group(1))
: assert max_time and len(max_time.groups()) == 1
optional: extract the time stats parsing logic to a function, e.g. in tests/parse_util.py? get_bytes_summary_stats_counter seems to do something similar
--
To view, visit http://gerrit.cloudera.org:8080/19897
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9ca525285e03c7b51b04ac292f7b3531e6178218
Gerrit-Change-Number: 19897
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Comment-Date: Wed, 17 May 2023 06:53:17 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10180: Add summary stats for client fetch wait time
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19897 )
Change subject: IMPALA-10180: Add summary stats for client fetch wait time
......................................................................
Patch Set 2:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9326/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/19897
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9ca525285e03c7b51b04ac292f7b3531e6178218
Gerrit-Change-Number: 19897
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 May 2023 00:54:58 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10180: Add summary stats for client fetch wait time
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19897 )
Change subject: IMPALA-10180: Add summary stats for client fetch wait time
......................................................................
Patch Set 2: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/19897
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9ca525285e03c7b51b04ac292f7b3531e6178218
Gerrit-Change-Number: 19897
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 May 2023 06:17:40 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10180: Add summary stats for client fetch wait time
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19897 )
Change subject: IMPALA-10180: Add summary stats for client fetch wait time
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13059/ : 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/19897
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9ca525285e03c7b51b04ac292f7b3531e6178218
Gerrit-Change-Number: 19897
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Comment-Date: Tue, 16 May 2023 18:58:58 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10180: Add summary stats for client fetch wait time
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Kurt Deschler, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19897
to look at the new patch set (#2).
Change subject: IMPALA-10180: Add summary stats for client fetch wait time
......................................................................
IMPALA-10180: Add summary stats for client fetch wait time
This adds ClientFetchWaitTimeStats to the runtime profile
to track the min/max/# of samples for ClientFetchWaitTimer.
Here is some sample output:
- ClientFetchWaitTimeStats: (Avg: 161.554ms ; Min: 101.411ms ; Max: 461.728ms ; Number of samples: 6)
- ClientFetchWaitTimer: 969.326ms
This also fixes the definition of ClientFetchWaitTimer to avoid
including time after end of fetch. When the client is closing
the query, Finalize() gets called. The Finalize() call should
only add extra client wait time if fetch has not completed.
Testing:
- Added test cases in query_test/test_fetch.py with specific
numbers of fetches and verification of the statistics.
- The test cases make use of a new function for parsing
summary stats for timers, and this also gets its own test
case.
Change-Id: I9ca525285e03c7b51b04ac292f7b3531e6178218
---
M be/src/service/client-request-state.cc
M be/src/service/client-request-state.h
M tests/infra/test_utils.py
M tests/query_test/test_fetch.py
M tests/util/parse_util.py
5 files changed, 180 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/97/19897/2
--
To view, visit http://gerrit.cloudera.org:8080/19897
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9ca525285e03c7b51b04ac292f7b3531e6178218
Gerrit-Change-Number: 19897
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
[Impala-ASF-CR] IMPALA-10180: Add summary stats for client fetch wait time
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19897 )
Change subject: IMPALA-10180: Add summary stats for client fetch wait time
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13071/ : 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/19897
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9ca525285e03c7b51b04ac292f7b3531e6178218
Gerrit-Change-Number: 19897
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 May 2023 00:42:46 +0000
Gerrit-HasComments: No