You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Tianyi Wang (Code Review)" <ge...@cloudera.org> on 2017/09/12 01:56:03 UTC
[Impala-ASF-CR] IMPALA-4987: Fix flaky test test row availability.py
Tianyi Wang has uploaded a new change for review.
http://gerrit.cloudera.org:8080/8036
Change subject: IMPALA-4987: Fix flaky test test_row_availability.py
......................................................................
IMPALA-4987: Fix flaky test test_row_availability.py
This patch keeps test_row_availbility from random failure. In this test
the time interval between 'Rows available' event and the previous event
in runtime profile is measured in order to make sure that rows become
available after a specific amount of time. This is not correct since
the previous event is that the coordinator finishes sending query to
backends, which means the execution on backend might have already
started. This patch tracks another event "ready to start" as the
beginning of time interval instead. The coordinator begins to send
query to backends after this event so the time check should always pass.
Change-Id: I96142f1868a26426cbc34aa9a0e0a56979df66c3
---
M tests/query_test/test_rows_availability.py
1 file changed, 30 insertions(+), 21 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/36/8036/1
--
To view, visit http://gerrit.cloudera.org:8080/8036
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I96142f1868a26426cbc34aa9a0e0a56979df66c3
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tianyi Wang <tw...@cloudera.com>
[Impala-ASF-CR] IMPALA-4987: Fix flaky test test row availability.py
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-4987: Fix flaky test test_row_availability.py
......................................................................
Patch Set 3:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1215/
--
To view, visit http://gerrit.cloudera.org:8080/8036
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I96142f1868a26426cbc34aa9a0e0a56979df66c3
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4987: Fix flaky test test row availability.py
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.
Change subject: IMPALA-4987: Fix flaky test test_row_availability.py
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/8036
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I96142f1868a26426cbc34aa9a0e0a56979df66c3
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4987: Fix flaky test test row availability.py
Posted by "Tianyi Wang (Code Review)" <ge...@cloudera.org>.
Tianyi Wang has posted comments on this change.
Change subject: IMPALA-4987: Fix flaky test test_row_availability.py
......................................................................
Patch Set 2:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/8036/2//COMMIT_MSG
Commit Message:
Line 7: IMPALA-4987: Fix flaky test test_row_availability.py
> Sorry to be adamant about grammar nits, but let's get them fixed.
Thanks for the careful review. I should definitely improve on this.
Line 9: This patch keeps test_row_availbility from random failing. In this test
> from randomly failing
Done
Line 14: coordinator finished sending query to the backends, which means the
> sending the query fragments
Done
http://gerrit.cloudera.org:8080/#/c/8036/2/tests/query_test/test_rows_availability.py
File tests/query_test/test_rows_availability.py:
Line 1: # Licensed to the Apache Software Foundation (ASF) under one
> space
Done
--
To view, visit http://gerrit.cloudera.org:8080/8036
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I96142f1868a26426cbc34aa9a0e0a56979df66c3
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4987: Fix flaky test test row availability.py
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.
Change subject: IMPALA-4987: Fix flaky test test_row_availability.py
......................................................................
Patch Set 2:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/8036/2//COMMIT_MSG
Commit Message:
Line 7: IMPALA-4987: Fix flaky test test_row_availability.py
Sorry to be adamant about grammar nits, but let's get them fixed.
Line 9: This patch keeps test_row_availbility from random failing. In this test
from randomly failing
Line 14: coordinator finished sending query to the backends, which means the
sending the query fragments
http://gerrit.cloudera.org:8080/#/c/8036/2/tests/query_test/test_rows_availability.py
File tests/query_test/test_rows_availability.py:
Line 1: # Licensed to the Apache Software Foundation (ASF) under one
space
--
To view, visit http://gerrit.cloudera.org:8080/8036
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I96142f1868a26426cbc34aa9a0e0a56979df66c3
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4987: Fix flaky test test row availability.py
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.
Change subject: IMPALA-4987: Fix flaky test test_row_availability.py
......................................................................
Patch Set 1:
(13 comments)
Thanks for fixing this
http://gerrit.cloudera.org:8080/#/c/8036/1//COMMIT_MSG
Commit Message:
Line 9: This patch keeps test_row_availbility from random failure. In this test
Please take a pass to correct wording/grammar. For example, it should be:
"This patch keeps test_rows_availability from randomly failing."
More such examples below.
Line 10: the time interval between 'Rows available' event and the previous event
the 'Rows available' timeline event
Line 11: in runtime profile is measured in order to make sure that rows become
in the runtime profile
Line 12: available after a specific amount of time. This is not correct since
Instead of 'This' try to rephrase/repeat what 'This' refers to for clarity, e.g. This measurement is not correct ...
Line 13: the previous event is that the coordinator finishes sending query to
finished sending the query to the backends
Line 14: backends, which means the execution on backend might have already
on some backends might have already started
Line 15: started. This patch tracks another event "ready to start" as the
"Read to start"
Line 17: query to backends after this event so the time check should always pass.
the query to backends
http://gerrit.cloudera.org:8080/#/c/8036/1/tests/query_test/test_rows_availability.py
File tests/query_test/test_rows_availability.py:
Line 81: row_avail_time_ms = self.__parse_time_ms(self.__find_time(line))
rows_avail_time_ms (not just one row is available)
Line 92: "'ready to start' event.\nExpected the event to be marked no earlier than "\
'Ready to start'
Line 93: "%sms after the 'ready to start' event.\nQuery: %s"\
'Ready to start'
Line 99: """Find event time point in a line from runtime timeline."""
from the runtime profile timeline
Line 100: # Example line: "- Rows available: 3s311ms (2s300ms)"
Example should include what this function returns
--
To view, visit http://gerrit.cloudera.org:8080/8036
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I96142f1868a26426cbc34aa9a0e0a56979df66c3
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4987: Fix flaky test test row availability.py
Posted by "Tianyi Wang (Code Review)" <ge...@cloudera.org>.
Tianyi Wang has uploaded a new patch set (#2).
Change subject: IMPALA-4987: Fix flaky test test_row_availability.py
......................................................................
IMPALA-4987: Fix flaky test test_row_availability.py
This patch keeps test_row_availbility from random failing. In this test
the time interval between the 'Rows available' timeline event and the
previous event in the runtime profile is measured in order to make sure
that the rows become available after a specific amount of time. This
measurement is not correct since the previous event is that the
coordinator finished sending query to the backends, which means the
execution on some backends might have already started. This patch tracks
another event "Ready to start" as the beginning of time interval
instead. The coordinator begins to send the query to the backends after
this event so the time check should always pass.
Change-Id: I96142f1868a26426cbc34aa9a0e0a56979df66c3
---
M tests/query_test/test_rows_availability.py
1 file changed, 31 insertions(+), 22 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/36/8036/2
--
To view, visit http://gerrit.cloudera.org:8080/8036
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I96142f1868a26426cbc34aa9a0e0a56979df66c3
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
[Impala-ASF-CR] IMPALA-4987: Fix flaky test test row availability.py
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-4987: Fix flaky test test_row_availability.py
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/8036
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I96142f1868a26426cbc34aa9a0e0a56979df66c3
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4987: Fix flaky test test row availability.py
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged.
Change subject: IMPALA-4987: Fix flaky test test_row_availability.py
......................................................................
IMPALA-4987: Fix flaky test test_row_availability.py
This patch keeps test_row_availbility from randomly failing. In this test
the time interval between the 'Rows available' timeline event and the
previous event in the runtime profile is measured in order to make sure
that the rows become available after a specific amount of time. This
measurement is not correct since the previous event is that the
coordinator finished sending the query fragments to the backends, which
means the execution on some backends might have already started. This
patch tracks another event "Ready to start" as the beginning of the time
interval instead. The coordinator begins to send the query fragments to
the backends after this event so the time check should always pass.
Change-Id: I96142f1868a26426cbc34aa9a0e0a56979df66c3
Reviewed-on: http://gerrit.cloudera.org:8080/8036
Reviewed-by: Alex Behm <al...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M tests/query_test/test_rows_availability.py
1 file changed, 30 insertions(+), 21 deletions(-)
Approvals:
Impala Public Jenkins: Verified
Alex Behm: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/8036
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I96142f1868a26426cbc34aa9a0e0a56979df66c3
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
[Impala-ASF-CR] IMPALA-4987: Fix flaky test test row availability.py
Posted by "Tianyi Wang (Code Review)" <ge...@cloudera.org>.
Tianyi Wang has uploaded a new patch set (#3).
Change subject: IMPALA-4987: Fix flaky test test_row_availability.py
......................................................................
IMPALA-4987: Fix flaky test test_row_availability.py
This patch keeps test_row_availbility from randomly failing. In this test
the time interval between the 'Rows available' timeline event and the
previous event in the runtime profile is measured in order to make sure
that the rows become available after a specific amount of time. This
measurement is not correct since the previous event is that the
coordinator finished sending the query fragments to the backends, which
means the execution on some backends might have already started. This
patch tracks another event "Ready to start" as the beginning of the time
interval instead. The coordinator begins to send the query fragments to
the backends after this event so the time check should always pass.
Change-Id: I96142f1868a26426cbc34aa9a0e0a56979df66c3
---
M tests/query_test/test_rows_availability.py
1 file changed, 30 insertions(+), 21 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/36/8036/3
--
To view, visit http://gerrit.cloudera.org:8080/8036
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I96142f1868a26426cbc34aa9a0e0a56979df66c3
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
[Impala-ASF-CR] IMPALA-4987: Fix flaky test test row availability.py
Posted by "Tianyi Wang (Code Review)" <ge...@cloudera.org>.
Tianyi Wang has posted comments on this change.
Change subject: IMPALA-4987: Fix flaky test test_row_availability.py
......................................................................
Patch Set 1:
(13 comments)
http://gerrit.cloudera.org:8080/#/c/8036/1//COMMIT_MSG
Commit Message:
Line 9: This patch keeps test_row_availbility from random failure. In this test
> Please take a pass to correct wording/grammar. For example, it should be:
Done
Line 10: the time interval between 'Rows available' event and the previous event
> the 'Rows available' timeline event
Done
Line 11: in runtime profile is measured in order to make sure that rows become
> in the runtime profile
Done
Line 12: available after a specific amount of time. This is not correct since
> Instead of 'This' try to rephrase/repeat what 'This' refers to for clarity,
Done
Line 13: the previous event is that the coordinator finishes sending query to
> finished sending the query to the backends
Done
Line 14: backends, which means the execution on backend might have already
> on some backends might have already started
Done
Line 15: started. This patch tracks another event "ready to start" as the
> "Read to start"
Done
Line 17: query to backends after this event so the time check should always pass.
> the query to backends
Done
http://gerrit.cloudera.org:8080/#/c/8036/1/tests/query_test/test_rows_availability.py
File tests/query_test/test_rows_availability.py:
Line 81: row_avail_time_ms = self.__parse_time_ms(self.__find_time(line))
> rows_avail_time_ms (not just one row is available)
Done
Line 92: "'ready to start' event.\nExpected the event to be marked no earlier than "\
> 'Ready to start'
Done
Line 93: "%sms after the 'ready to start' event.\nQuery: %s"\
> 'Ready to start'
Done
Line 99: """Find event time point in a line from runtime timeline."""
> from the runtime profile timeline
Done
Line 100: # Example line: "- Rows available: 3s311ms (2s300ms)"
> Example should include what this function returns
Done
--
To view, visit http://gerrit.cloudera.org:8080/8036
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I96142f1868a26426cbc34aa9a0e0a56979df66c3
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-HasComments: Yes