You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Tim Armstrong (Code Review)" <ge...@cloudera.org> on 2018/09/19 17:48:11 UTC

[Impala-ASF-CR] IMPALA-7306: regression test for non-removed transient updates

Tim Armstrong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11470


Change subject: IMPALA-7306: regression test for non-removed transient updates
......................................................................

IMPALA-7306: regression test for non-removed transient updates

Adds a test for IMPALA-7305 that reproduces the bug by delaying
heartbeats and updates.

Increased some timeouts in the test because they were hit
once after looping for ~12 hours.

Testing:
Manually reintroduced the bug by commenting out the code that
fixed it and confirmed that the test failed.

Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc
---
M tests/statestore/test_statestore.py
1 file changed, 58 insertions(+), 7 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc
Gerrit-Change-Number: 11470
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-7306: regression test for non-removed transient updates

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

Change subject: IMPALA-7306: regression test for non-removed transient updates
......................................................................


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/11470/1/tests/statestore/test_statestore.py
File tests/statestore/test_statestore.py:

http://gerrit.cloudera.org:8080/#/c/11470/1/tests/statestore/test_statestore.py@73
PS1, Line 73: 0
flake8: E501 line too long (92 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/11470/1/tests/statestore/test_statestore.py@301
PS1, Line 301: W
flake8: E122 continuation line missing indentation or outdented


http://gerrit.cloudera.org:8080/#/c/11470/1/tests/statestore/test_statestore.py@324
PS1, Line 324: t
flake8: E122 continuation line missing indentation or outdented


http://gerrit.cloudera.org:8080/#/c/11470/1/tests/statestore/test_statestore.py@780
PS1, Line 780: d
flake8: E306 expected 1 blank line before a nested definition, found 0



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc
Gerrit-Change-Number: 11470
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 19 Sep 2018 18:51:19 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7306: regression test for non-removed transient updates

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

Change subject: IMPALA-7306: regression test for non-removed transient updates
......................................................................


Patch Set 3:

Looking into this, trying to wrap my head around the parent bug (forgot the context).


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc
Gerrit-Change-Number: 11470
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Sep 2018 17:01:02 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7306: regression test for non-removed transient updates

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

Change subject: IMPALA-7306: regression test for non-removed transient updates
......................................................................


Patch Set 3:

(4 comments)

Patch LGTM, some nits and clarifications.

http://gerrit.cloudera.org:8080/#/c/11470/3/tests/statestore/test_statestore.py
File tests/statestore/test_statestore.py:

http://gerrit.cloudera.org:8080/#/c/11470/3/tests/statestore/test_statestore.py@773
PS3, Line 773: test_transient_entry_removal
nit: test_transient_entry_removal_delayed_hbs() or something? This sounds very similar to test_topic_persistence() above.


http://gerrit.cloudera.org:8080/#/c/11470/3/tests/statestore/test_statestore.py@778
PS3, Line 778:     topic_name = "test_transient_entry_removal"
Should we make sure that non-transient entries are not removed under these delayed conditions? Feel free to ignore if you think test_topic_persistence has enough coverage for it.


http://gerrit.cloudera.org:8080/#/c/11470/3/tests/statestore/test_statestore.py@787
PS3, Line 787: with_delay
nit: after_hb_failure? (or some such thing)


http://gerrit.cloudera.org:8080/#/c/11470/3/tests/statestore/test_statestore.py@804
PS3, Line 804: + 20
why additional 20s? Doesn't the above delayed HB cause it to be unsubscribed before WAIT_FOR_FAILURE_TIMEOUT?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc
Gerrit-Change-Number: 11470
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Sep 2018 19:12:32 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7306: regression test for non-removed transient updates

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

Change subject: IMPALA-7306: regression test for non-removed transient updates
......................................................................


Patch Set 7:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc
Gerrit-Change-Number: 11470
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 25 Sep 2018 01:00:00 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7306: regression test for non-removed transient updates

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-7306: regression test for non-removed transient updates
......................................................................

IMPALA-7306: regression test for non-removed transient updates

Adds a test for IMPALA-7305 that reproduces the bug by delaying
heartbeats and updates.

Increased some timeouts in the test because they were hit
once after looping for ~12 hours.

Testing:
Manually reintroduced the bug by commenting out the code that
fixed it and confirmed that the test failed.

Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc
---
M tests/statestore/test_statestore.py
1 file changed, 79 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/70/11470/6
-- 
To view, visit http://gerrit.cloudera.org:8080/11470
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc
Gerrit-Change-Number: 11470
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-7306: regression test for non-removed transient updates

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

Change subject: IMPALA-7306: regression test for non-removed transient updates
......................................................................


Patch Set 2:

Fixed the bot comments


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc
Gerrit-Change-Number: 11470
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 19 Sep 2018 18:56:11 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7306: regression test for non-removed transient updates

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

Change subject: IMPALA-7306: regression test for non-removed transient updates
......................................................................


Patch Set 6:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/773/ : 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/11470
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc
Gerrit-Change-Number: 11470
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 25 Sep 2018 01:57:11 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7306: regression test for non-removed transient updates

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-7306: regression test for non-removed transient updates
......................................................................

IMPALA-7306: regression test for non-removed transient updates

Adds a test for IMPALA-7305 that reproduces the bug by delaying
heartbeats and updates.

Increased some timeouts in the test because they were hit
once after looping for ~12 hours.

Testing:
Manually reintroduced the bug by commenting out the code that
fixed it and confirmed that the test failed.

Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc
---
M tests/statestore/test_statestore.py
1 file changed, 61 insertions(+), 8 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc
Gerrit-Change-Number: 11470
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-7306: regression test for non-removed transient updates

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

Change subject: IMPALA-7306: regression test for non-removed transient updates
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/710/ : 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/11470
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc
Gerrit-Change-Number: 11470
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 19 Sep 2018 18:21:23 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7306: regression test for non-removed transient updates

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

Change subject: IMPALA-7306: regression test for non-removed transient updates
......................................................................


Patch Set 5:

The 0 timeout was based on a bad assumption - I thought the update count was incremented before rather than after the update.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc
Gerrit-Change-Number: 11470
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 25 Sep 2018 00:59:33 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7306: regression test for non-removed transient updates

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

Change subject: IMPALA-7306: regression test for non-removed transient updates
......................................................................


Patch Set 6: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc
Gerrit-Change-Number: 11470
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 25 Sep 2018 00:59:39 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7306: regression test for non-removed transient updates

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

Change subject: IMPALA-7306: regression test for non-removed transient updates
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/11470/2/tests/statestore/test_statestore.py
File tests/statestore/test_statestore.py:

http://gerrit.cloudera.org:8080/#/c/11470/2/tests/statestore/test_statestore.py@302
PS2, Line 302: W
flake8: E122 continuation line missing indentation or outdented


http://gerrit.cloudera.org:8080/#/c/11470/2/tests/statestore/test_statestore.py@325
PS2, Line 325: t
flake8: E122 continuation line missing indentation or outdented



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc
Gerrit-Change-Number: 11470
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 19 Sep 2018 21:02:12 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7306: regression test for non-removed transient updates

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

Change subject: IMPALA-7306: regression test for non-removed transient updates
......................................................................


Patch Set 7: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc
Gerrit-Change-Number: 11470
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 25 Sep 2018 00:59:59 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7306: regression test for non-removed transient updates

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

Change subject: IMPALA-7306: regression test for non-removed transient updates
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/712/ : 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/11470
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc
Gerrit-Change-Number: 11470
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 19 Sep 2018 19:51:05 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7306: regression test for non-removed transient updates

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

Change subject: IMPALA-7306: regression test for non-removed transient updates
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/718/ : 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/11470
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc
Gerrit-Change-Number: 11470
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 19 Sep 2018 22:20:01 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7306: regression test for non-removed transient updates

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

Change subject: IMPALA-7306: regression test for non-removed transient updates
......................................................................


Patch Set 7: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc
Gerrit-Change-Number: 11470
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 25 Sep 2018 04:52:11 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7306: regression test for non-removed transient updates

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-7306: regression test for non-removed transient updates
......................................................................

IMPALA-7306: regression test for non-removed transient updates

Adds a test for IMPALA-7305 that reproduces the bug by delaying
heartbeats and updates.

Increased some timeouts in the test because they were hit
once after looping for ~12 hours.

Testing:
Manually reintroduced the bug by commenting out the code that
fixed it and confirmed that the test failed.

Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc
---
M tests/statestore/test_statestore.py
1 file changed, 79 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/70/11470/4
-- 
To view, visit http://gerrit.cloudera.org:8080/11470
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc
Gerrit-Change-Number: 11470
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-7306: regression test for non-removed transient updates

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

Change subject: IMPALA-7306: regression test for non-removed transient updates
......................................................................


Patch Set 4: Code-Review+2

LGTM.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc
Gerrit-Change-Number: 11470
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 24 Sep 2018 20:10:35 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7306: regression test for non-removed transient updates

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

Change subject: IMPALA-7306: regression test for non-removed transient updates
......................................................................


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/11470/3/tests/statestore/test_statestore.py
File tests/statestore/test_statestore.py:

http://gerrit.cloudera.org:8080/#/c/11470/3/tests/statestore/test_statestore.py@773
PS3, Line 773: test_transient_entry_removal
> nit: test_transient_entry_removal_delayed_hbs() or something? This sounds v
I just added _race as a suffix to be more concise - I think that captures that it testing for a particular race.


http://gerrit.cloudera.org:8080/#/c/11470/3/tests/statestore/test_statestore.py@778
PS3, Line 778:     topic_name = "test_transient_entry_removal"
> Should we make sure that non-transient entries are not removed under these 
Done


http://gerrit.cloudera.org:8080/#/c/11470/3/tests/statestore/test_statestore.py@787
PS3, Line 787: with_delay
> nit: after_hb_failure? (or some such thing)
Done


http://gerrit.cloudera.org:8080/#/c/11470/3/tests/statestore/test_statestore.py@804
PS3, Line 804: + 20
> why additional 20s? Doesn't the above delayed HB cause it to be unsubscribe
Actually we can set it to zero, since the first update occurs after the failure. Updated and commented to explain. Had to fix wait_for_failure to handle a 0 timeout correctly too.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc
Gerrit-Change-Number: 11470
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 24 Sep 2018 17:57:53 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7306: regression test for non-removed transient updates

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

Change subject: IMPALA-7306: regression test for non-removed transient updates
......................................................................


Patch Set 5: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc
Gerrit-Change-Number: 11470
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 24 Sep 2018 20:36:52 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7306: regression test for non-removed transient updates

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

Change subject: IMPALA-7306: regression test for non-removed transient updates
......................................................................


Patch Set 5: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3208/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc
Gerrit-Change-Number: 11470
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 25 Sep 2018 00:26:47 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7306: regression test for non-removed transient updates

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

Change subject: IMPALA-7306: regression test for non-removed transient updates
......................................................................

IMPALA-7306: regression test for non-removed transient updates

Adds a test for IMPALA-7305 that reproduces the bug by delaying
heartbeats and updates.

Increased some timeouts in the test because they were hit
once after looping for ~12 hours.

Testing:
Manually reintroduced the bug by commenting out the code that
fixed it and confirmed that the test failed.

Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc
Reviewed-on: http://gerrit.cloudera.org:8080/11470
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M tests/statestore/test_statestore.py
1 file changed, 79 insertions(+), 10 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc
Gerrit-Change-Number: 11470
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-7306: regression test for non-removed transient updates

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

Change subject: IMPALA-7306: regression test for non-removed transient updates
......................................................................


Patch Set 5:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc
Gerrit-Change-Number: 11470
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 24 Sep 2018 20:36:54 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7306: regression test for non-removed transient updates

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-7306: regression test for non-removed transient updates
......................................................................

IMPALA-7306: regression test for non-removed transient updates

Adds a test for IMPALA-7305 that reproduces the bug by delaying
heartbeats and updates.

Increased some timeouts in the test because they were hit
once after looping for ~12 hours.

Testing:
Manually reintroduced the bug by commenting out the code that
fixed it and confirmed that the test failed.

Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc
---
M tests/statestore/test_statestore.py
1 file changed, 61 insertions(+), 8 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc
Gerrit-Change-Number: 11470
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-7306: regression test for non-removed transient updates

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

Change subject: IMPALA-7306: regression test for non-removed transient updates
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/760/ : 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/11470
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc
Gerrit-Change-Number: 11470
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 24 Sep 2018 18:31:38 +0000
Gerrit-HasComments: No