impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Behm (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4987: Fix flaky test test row availability.py
Date Tue, 12 Sep 2017 03:58:08 GMT
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 <twang@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message