You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org> on 2018/07/11 18:10:07 UTC

[Impala-ASF-CR] IMPALA-7279: Fix flakiness in test rows availability 

Bikramjeet Vig has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10922


Change subject: IMPALA-7279: Fix flakiness in test_rows_availability
......................................................................

IMPALA-7279: Fix flakiness in test_rows_availability

This patch fixes a flaky time string parsing method in
test_rows_availability that fails on strings with microsecond precision.

Change-Id: If7634869823d8cc4059048dd5d3c3a984744f3be
---
M tests/query_test/test_hash_join_timer.py
M tests/query_test/test_rows_availability.py
M tests/util/parse_util.py
3 files changed, 28 insertions(+), 48 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: If7634869823d8cc4059048dd5d3c3a984744f3be
Gerrit-Change-Number: 10922
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>

[Impala-ASF-CR] IMPALA-7279: Fix flakiness in test rows availability

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

Change subject: IMPALA-7279: Fix flakiness in test_rows_availability
......................................................................


Patch Set 4:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7634869823d8cc4059048dd5d3c3a984744f3be
Gerrit-Change-Number: 10922
Gerrit-PatchSet: 4
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Jul 2018 23:25:49 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7279: Fix flakiness in test rows availability

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Hello Michael Brown, David Knupp, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/10922

to look at the new patch set (#3).

Change subject: IMPALA-7279: Fix flakiness in test_rows_availability
......................................................................

IMPALA-7279: Fix flakiness in test_rows_availability

This patch fixes a flaky time string parsing method in
test_rows_availability that fails on strings with microsecond precision.

Change-Id: If7634869823d8cc4059048dd5d3c3a984744f3be
---
M tests/query_test/test_hash_join_timer.py
M tests/query_test/test_rows_availability.py
M tests/util/parse_util.py
3 files changed, 22 insertions(+), 48 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/22/10922/3
-- 
To view, visit http://gerrit.cloudera.org:8080/10922
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If7634869823d8cc4059048dd5d3c3a984744f3be
Gerrit-Change-Number: 10922
Gerrit-PatchSet: 3
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>

[Impala-ASF-CR] IMPALA-7279: Fix flakiness in test rows availability

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

Change subject: IMPALA-7279: Fix flakiness in test_rows_availability
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10922/1/tests/util/parse_util.py
File tests/util/parse_util.py:

http://gerrit.cloudera.org:8080/#/c/10922/1/tests/util/parse_util.py@83
PS1, Line 83:     if (match[2] == 'h'):
Also, rather than the if/elif construction, you could maybe do this:

  pattern = r'(?P<value>[0-9]+\.?[0-9]*?)(?P<units>\D+)'
  matches = list(re.finditer(pattern, duration))   # You need list() here if you plan to assert
  assert matches, 'Failed to parse duration string %s' % duration
  
  times = {'h': 0, 'm': 0, 's': 0, 'ms': 0}
  for match in matches:
    parsed = match.groupdict()
    times[parsed['units']] = float(parsed['value'])

  return times['h'] * 60 * 60 * 1000 + times['m'] * 60 * 1000 + times['s'] * 1000 + times['ms']



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7634869823d8cc4059048dd5d3c3a984744f3be
Gerrit-Change-Number: 10922
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Jul 2018 19:40:14 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7279: Fix flakiness in test rows availability

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

Change subject: IMPALA-7279: Fix flakiness in test_rows_availability
......................................................................


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7634869823d8cc4059048dd5d3c3a984744f3be
Gerrit-Change-Number: 10922
Gerrit-PatchSet: 4
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Jul 2018 23:25:48 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7279: Fix flakiness in test rows availability

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

Change subject: IMPALA-7279: Fix flakiness in test_rows_availability
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10922/2/tests/util/parse_util.py
File tests/util/parse_util.py:

http://gerrit.cloudera.org:8080/#/c/10922/2/tests/util/parse_util.py@85
PS2, Line 85:   return (times['h'] * 60 * 60 + times['m'] * 60 + times['s']) * 1000 + times['ms']
> Nit: long line.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7634869823d8cc4059048dd5d3c3a984744f3be
Gerrit-Change-Number: 10922
Gerrit-PatchSet: 3
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Jul 2018 23:07:33 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7279: Fix flakiness in test rows availability

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Hello Michael Brown, David Knupp, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/10922

to look at the new patch set (#2).

Change subject: IMPALA-7279: Fix flakiness in test_rows_availability
......................................................................

IMPALA-7279: Fix flakiness in test_rows_availability

This patch fixes a flaky time string parsing method in
test_rows_availability that fails on strings with microsecond precision.

Change-Id: If7634869823d8cc4059048dd5d3c3a984744f3be
---
M tests/query_test/test_hash_join_timer.py
M tests/query_test/test_rows_availability.py
M tests/util/parse_util.py
3 files changed, 23 insertions(+), 48 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/22/10922/2
-- 
To view, visit http://gerrit.cloudera.org:8080/10922
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If7634869823d8cc4059048dd5d3c3a984744f3be
Gerrit-Change-Number: 10922
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>

[Impala-ASF-CR] IMPALA-7279: Fix flakiness in test rows availability

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

Change subject: IMPALA-7279: Fix flakiness in test_rows_availability
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10922/2/tests/util/parse_util.py
File tests/util/parse_util.py:

http://gerrit.cloudera.org:8080/#/c/10922/2/tests/util/parse_util.py@85
PS2, Line 85:   return times['h'] * 60 * 60 * 1000 + times['m'] * 60 * 1000 + times['s'] * 1000 + times[
Nit: long line.

Maybe you can factor out the multiple "* 1000"?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7634869823d8cc4059048dd5d3c3a984744f3be
Gerrit-Change-Number: 10922
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Jul 2018 22:43:04 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7279: Fix flakiness in test rows availability

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

Change subject: IMPALA-7279: Fix flakiness in test_rows_availability
......................................................................


Patch Set 1:

(2 comments)

Thanks David!

http://gerrit.cloudera.org:8080/#/c/10922/1/tests/util/parse_util.py
File tests/util/parse_util.py:

http://gerrit.cloudera.org:8080/#/c/10922/1/tests/util/parse_util.py@76
PS1, Line 76: r'(?P<value>[0-9]+(\.[0-9]+)?)(?P<units>\D+)'
> There's something weird about your pattern. If I print out the result of ap
Done


http://gerrit.cloudera.org:8080/#/c/10922/1/tests/util/parse_util.py@83
PS1, Line 83:     if (match[2] == 'h'):
> Also, rather than the if/elif construction, you could maybe do this:
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7634869823d8cc4059048dd5d3c3a984744f3be
Gerrit-Change-Number: 10922
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Jul 2018 21:03:58 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7279: Fix flakiness in test rows availability

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

Change subject: IMPALA-7279: Fix flakiness in test_rows_availability
......................................................................


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7634869823d8cc4059048dd5d3c3a984744f3be
Gerrit-Change-Number: 10922
Gerrit-PatchSet: 3
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Jul 2018 23:16:33 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7279: Fix flakiness in test rows availability

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

Change subject: IMPALA-7279: Fix flakiness in test_rows_availability
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10922/1/tests/util/parse_util.py
File tests/util/parse_util.py:

http://gerrit.cloudera.org:8080/#/c/10922/1/tests/util/parse_util.py@76
PS1, Line 76: r'(?P<value>[0-9]+(\.[0-9]+)?)(?P<units>\D+)'
There's something weird about your pattern. If I print out the result of applying re.findall() to your sample duration string, each match winds up being a tuple with 3 elements:

  [('1', '', 'h'), ('2', '', 'h'), ('3', '', 'm'), ('4', '', 's'), ('5.6', '.6', 'ms'), ('4.5', '.5', 'us'), ('7.8', '.8', 'ns')]

...when you *probably* want each tuple to have 2 elements -- just the value and the units, like this:

  [('1', 'h'), ('2', 'h'), ('3', 'm'), ('4', 's'), ('5.6', 'ms'), ('4.5', 'us'), ('7.8', 'ns')]

I think this pattern will give you that (but I'm not a regex expert):

  r'(?P<value>[0-9]+\.?[0-9]*?)(?P<units>\D+)'

Furthermore, you're using the semantics for named groups (i.e., ?P<value> and ?P<units>), but then resorting to referencing by index numbers. You could try something like this:

  pattern = r'(?P<value>[0-9]+\.?[0-9]*?)(?P<units>\D+)'
  matches = re.finditer(pattern, duration)
  
  for match in matches:
    parsed = match.groupdict()
    if parsed['units'] == 'h':
      hours = float(parsed['value'])
    etc...

...which is a little more readable than [0] and [2].



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7634869823d8cc4059048dd5d3c3a984744f3be
Gerrit-Change-Number: 10922
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Jul 2018 19:18:24 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7279: Fix flakiness in test rows availability

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

Change subject: IMPALA-7279: Fix flakiness in test_rows_availability
......................................................................

IMPALA-7279: Fix flakiness in test_rows_availability

This patch fixes a flaky time string parsing method in
test_rows_availability that fails on strings with microsecond precision.

Change-Id: If7634869823d8cc4059048dd5d3c3a984744f3be
Reviewed-on: http://gerrit.cloudera.org:8080/10922
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M tests/query_test/test_hash_join_timer.py
M tests/query_test/test_rows_availability.py
M tests/util/parse_util.py
3 files changed, 22 insertions(+), 48 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: If7634869823d8cc4059048dd5d3c3a984744f3be
Gerrit-Change-Number: 10922
Gerrit-PatchSet: 5
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>

[Impala-ASF-CR] IMPALA-7279: Fix flakiness in test rows availability

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

Change subject: IMPALA-7279: Fix flakiness in test_rows_availability
......................................................................


Patch Set 4: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7634869823d8cc4059048dd5d3c3a984744f3be
Gerrit-Change-Number: 10922
Gerrit-PatchSet: 4
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Jul 2018 02:41:59 +0000
Gerrit-HasComments: No