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 2017/08/28 14:52:36 UTC

[Impala-ASF-CR] Update stress test with admission control rejected messages

Tim Armstrong has uploaded a new change for review.

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

Change subject: Update stress test with admission control rejected messages
......................................................................

Update stress test with admission control rejected messages

IMPALA-5644 added a couple of new messages that indicate that the
mem_limit was too low to execute the query. The stress test script needs
updating to reflect this.

Testing:
Was able to do a full binary search with the modified stress test
script.

Change-Id: Ib2f6fa7c0f4e5875fcb92af8f712009ffa02c964
---
M tests/stress/concurrent_select.py
1 file changed, 2 insertions(+), 0 deletions(-)


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

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

[Impala-ASF-CR] Update stress test with admission control rejected messages

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.

Change subject: Update stress test with admission control rejected messages
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7854/2/tests/stress/concurrent_select.py
File tests/stress/concurrent_select.py:

PS2, Line 950: caught_msg == "cancelled":
> I searched through all of the available stress test results on Jenkins and 
So it seems like it's likely fixed in 2.10. The question is whether we just remove the check or whether we try to disable it based on the version, given that we may still want to run this on older versions of Impala.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib2f6fa7c0f4e5875fcb92af8f712009ffa02c964
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Matthew Mulder <mm...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] Update stress test with admission control rejected messages

Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.

Change subject: Update stress test with admission control rejected messages
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7854/2/tests/stress/concurrent_select.py
File tests/stress/concurrent_select.py:

PS2, Line 950: caught_msg == "cancelled":
> Because of https://issues.apache.org/jira/browse/IMPALA-2234 - I'm not sure
Yeah, I hope we fixed that. Should we remove this check, or do we want the stress test to be able to run against arbitrary old versions of Impala?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib2f6fa7c0f4e5875fcb92af8f712009ffa02c964
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Matthew Mulder <mm...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] Update stress test with admission control rejected messages

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.

Change subject: Update stress test with admission control rejected messages
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7854/2/tests/stress/concurrent_select.py
File tests/stress/concurrent_select.py:

PS2, Line 950: caught_msg == "cancelled":
> why do we have that check for determining report.mem_limit_exceeded?
Because of https://issues.apache.org/jira/browse/IMPALA-2234 - I'm not sure if that's still valid though.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib2f6fa7c0f4e5875fcb92af8f712009ffa02c964
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Matthew Mulder <mm...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] Update stress test with admission control rejected messages

Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.

Change subject: Update stress test with admission control rejected messages
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7854/2/tests/stress/concurrent_select.py
File tests/stress/concurrent_select.py:

PS2, Line 950: caught_msg == "cancelled":
why do we have that check for determining report.mem_limit_exceeded?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib2f6fa7c0f4e5875fcb92af8f712009ffa02c964
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Matthew Mulder <mm...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] Update stress test with admission control rejected messages

Posted by "Matthew Mulder (Code Review)" <ge...@cloudera.org>.
Matthew Mulder has posted comments on this change.

Change subject: Update stress test with admission control rejected messages
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7854/2/tests/stress/concurrent_select.py
File tests/stress/concurrent_select.py:

PS2, Line 950: caught_msg == "cancelled":
> I think also there were other bugs that would cause CANCELLED to overwrite 
I searched through all of the available stress test results on Jenkins and didn't see this "cancelled" message.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib2f6fa7c0f4e5875fcb92af8f712009ffa02c964
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Matthew Mulder <mm...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] Update stress test with admission control rejected messages

Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.

Change subject: Update stress test with admission control rejected messages
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7854/2/tests/stress/concurrent_select.py
File tests/stress/concurrent_select.py:

PS2, Line 950: caught_msg == "cancelled":
> That would be my guess.  I think we should run the stress test without this
I think also there were other bugs that would cause CANCELLED to overwrite the real status, on the coordinator side. But I don't remember off hand the specific JIRA.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib2f6fa7c0f4e5875fcb92af8f712009ffa02c964
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Matthew Mulder <mm...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] Update stress test with admission control rejected messages

Posted by "Michael Brown (Code Review)" <ge...@cloudera.org>.
Michael Brown has posted comments on this change.

Change subject: Update stress test with admission control rejected messages
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7854/2/tests/stress/concurrent_select.py
File tests/stress/concurrent_select.py:

PS2, Line 950: caught_msg == "cancelled":
> Yeah, I hope we fixed that. Should we remove this check, or do we want the 
I expect we won't run the stress test on anything older than 2.7. Any idea how long ago this may have been fixed?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib2f6fa7c0f4e5875fcb92af8f712009ffa02c964
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Matthew Mulder <mm...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] Update stress test with admission control rejected messages

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.

Change subject: Update stress test with admission control rejected messages
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7854/2/tests/stress/concurrent_select.py
File tests/stress/concurrent_select.py:

PS2, Line 950: caught_msg == "cancelled":
> I expect we won't run the stress test on anything older than 2.7. Any idea 
I'm not sure what the root cause of the problem was. It's possible that it it a BufferedBlockMgr lifecycle issue that is fixed in 2.10. I'm not sure if there's an alternative way that a cancelled status could bubble up in the wrong place.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib2f6fa7c0f4e5875fcb92af8f712009ffa02c964
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Matthew Mulder <mm...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] Update stress test with admission control rejected messages

Posted by "Michael Brown (Code Review)" <ge...@cloudera.org>.
Michael Brown has posted comments on this change.

Change subject: Update stress test with admission control rejected messages
......................................................................


Patch Set 1: Code-Review+2

Looks like this satisfies https://gerrit.cloudera.org/#/c/7834/ as well.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib2f6fa7c0f4e5875fcb92af8f712009ffa02c964
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Matthew Mulder <mm...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] Update stress test with admission control rejected messages

Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.

Change subject: Update stress test with admission control rejected messages
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7854/2/tests/stress/concurrent_select.py
File tests/stress/concurrent_select.py:

PS2, Line 950: caught_msg == "cancelled":
> So it seems like it's likely fixed in 2.10. The question is whether we just
Either way is okay with me. I guess if we don't take the concurrent_select.py corresponding to the version of impala being tested, we have to check the version.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib2f6fa7c0f4e5875fcb92af8f712009ffa02c964
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Matthew Mulder <mm...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] Update stress test with admission control rejected messages

Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.

Change subject: Update stress test with admission control rejected messages
......................................................................


Patch Set 2:

> (1 comment)

What I'm worried about is if we didn't fix that, then we should -- it's a real bug to not report the mem limit exceeded, of course. And so we should regression test it.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib2f6fa7c0f4e5875fcb92af8f712009ffa02c964
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Matthew Mulder <mm...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] Update stress test with admission control rejected messages

Posted by "Matthew Mulder (Code Review)" <ge...@cloudera.org>.
Matthew Mulder has posted comments on this change.

Change subject: Update stress test with admission control rejected messages
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7854/2/tests/stress/concurrent_select.py
File tests/stress/concurrent_select.py:

PS2, Line 950: caught_msg == "cancelled":
> Either way is okay with me. I guess if we don't take the concurrent_select.
As infrequently as the stress test is run on older versions of Impala, I doubt it's worth putting version logic in for this. The least I would do is remove this check, and the most I would do is move this check to its own if block that simply logs a helpful message that this is not supposed to happen after 2.9.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib2f6fa7c0f4e5875fcb92af8f712009ffa02c964
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Matthew Mulder <mm...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] Update stress test with admission control rejected messages

Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.

Change subject: Update stress test with admission control rejected messages
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7854/2/tests/stress/concurrent_select.py
File tests/stress/concurrent_select.py:

PS2, Line 950: caught_msg == "cancelled":
> I'm not sure what the root cause of the problem was. It's possible that it 
That would be my guess.  I think we should run the stress test without this and see if it pops up. If not, let's close IMPALA-2234 and make the change. If it does, I think we should debug it and fix it.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib2f6fa7c0f4e5875fcb92af8f712009ffa02c964
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Matthew Mulder <mm...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] Update stress test with admission control rejected messages

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has submitted this change and it was merged.

Change subject: Update stress test with admission control rejected messages
......................................................................


Update stress test with admission control rejected messages

IMPALA-5644 added a couple of new messages that indicate that the
mem_limit was too low to execute the query. The stress test script needs
updating to reflect this.

Testing:
Was able to do a full binary search with the modified stress test
script.

Change-Id: Ib2f6fa7c0f4e5875fcb92af8f712009ffa02c964
Reviewed-on: http://gerrit.cloudera.org:8080/7854
Reviewed-by: Michael Brown <mi...@cloudera.com>
Tested-by: Tim Armstrong <ta...@cloudera.com>
---
M tests/stress/concurrent_select.py
1 file changed, 2 insertions(+), 0 deletions(-)

Approvals:
  Michael Brown: Looks good to me, approved
  Tim Armstrong: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ib2f6fa7c0f4e5875fcb92af8f712009ffa02c964
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Matthew Mulder <mm...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] Update stress test with admission control rejected messages

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.

Change subject: Update stress test with admission control rejected messages
......................................................................


Patch Set 1: Verified+1

Precommit test doesn't exercise this. Manually verified by running stress test.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib2f6fa7c0f4e5875fcb92af8f712009ffa02c964
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Matthew Mulder <mm...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No