You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@impala.apache.org by "Matthew Jacobs (Code Review)" <ge...@cloudera.org> on 2016/09/08 20:24:31 UTC
[Impala-ASF-CR] IMPALA-4097: Crash in kudu-scan-node-test
Matthew Jacobs has uploaded a new change for review.
http://gerrit.cloudera.org:8080/4337
Change subject: IMPALA-4097: Crash in kudu-scan-node-test
......................................................................
IMPALA-4097: Crash in kudu-scan-node-test
The kudu-scan-node-test was calling GetNext() with a NULL
row batch, which isn't valid. This wasn't failing until a
recent code change, and only occasionally due to the timing
of scanner threads producing row batches. One batch is empty
and the other has 1 row. This test worked when the empty row
batch was added to the batch queue first (which usually
happened), but the test code couldn't handle the other
ordering properly. This fixes the test to be more careful
about what is being exercised.
Change-Id: I0e30964232ed137fce7a99bf7e9557293b5905c8
---
M be/src/exec/kudu-scan-node-test.cc
M be/src/exec/kudu-scan-node.cc
2 files changed, 22 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/4337/1
--
To view, visit http://gerrit.cloudera.org:8080/4337
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0e30964232ed137fce7a99bf7e9557293b5905c8
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj...@cloudera.com>
[Impala-ASF-CR] IMPALA-4097: Crash in kudu-scan-node-test
Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has posted comments on this change.
Change subject: IMPALA-4097: Crash in kudu-scan-node-test
......................................................................
Patch Set 1:
I tested this by adding an additional random delay in the start of a scan node to get different scanner thread orderings.
--
To view, visit http://gerrit.cloudera.org:8080/4337
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e30964232ed137fce7a99bf7e9557293b5905c8
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4097: Crash in kudu-scan-node-test
Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has posted comments on this change.
Change subject: IMPALA-4097: Crash in kudu-scan-node-test
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4337/1/be/src/exec/kudu-scan-node-test.cc
File be/src/exec/kudu-scan-node-test.cc:
PS1, Line 320: 1) the row batch containing
: // the single row, 2) an empty row batch (since there are scanners created for both
: // tablets), and 3) a final call which returns eos.
> nit: this might be more readable if you have one point per line
I'll change this but not worth uploading a new diff for this alone. I'll upload the change with the next set of comments.
--
To view, visit http://gerrit.cloudera.org:8080/4337
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e30964232ed137fce7a99bf7e9557293b5905c8
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4097: Crash in kudu-scan-node-test
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-4097: Crash in kudu-scan-node-test
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/4337
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e30964232ed137fce7a99bf7e9557293b5905c8
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4097: Crash in kudu-scan-node-test
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change.
Change subject: IMPALA-4097: Crash in kudu-scan-node-test
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4337/1/be/src/exec/kudu-scan-node-test.cc
File be/src/exec/kudu-scan-node-test.cc:
PS1, Line 320: 1) the row batch containing
: // the single row, 2) an empty row batch (since there are scanners created for both
: // tablets), and 3) a final call which returns eos.
nit: this might be more readable if you have one point per line
--
To view, visit http://gerrit.cloudera.org:8080/4337
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e30964232ed137fce7a99bf7e9557293b5905c8
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4097: Crash in kudu-scan-node-test
Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has posted comments on this change.
Change subject: IMPALA-4097: Crash in kudu-scan-node-test
......................................................................
Patch Set 2: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/4337
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e30964232ed137fce7a99bf7e9557293b5905c8
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4097: Crash in kudu-scan-node-test
Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Hello Tim Armstrong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/4337
to look at the new patch set (#2).
Change subject: IMPALA-4097: Crash in kudu-scan-node-test
......................................................................
IMPALA-4097: Crash in kudu-scan-node-test
The kudu-scan-node-test was calling GetNext() with a NULL
row batch, which isn't valid. This wasn't failing until a
recent code change, and only occasionally due to the timing
of scanner threads producing row batches. One batch is empty
and the other has 1 row. This test worked when the empty row
batch was added to the batch queue first (which usually
happened), but the test code couldn't handle the other
ordering properly. This fixes the test to be more careful
about what is being exercised.
Change-Id: I0e30964232ed137fce7a99bf7e9557293b5905c8
---
M be/src/exec/kudu-scan-node-test.cc
M be/src/exec/kudu-scan-node.cc
2 files changed, 23 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/4337/2
--
To view, visit http://gerrit.cloudera.org:8080/4337
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0e30964232ed137fce7a99bf7e9557293b5905c8
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-4097: Crash in kudu-scan-node-test
Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.
Change subject: IMPALA-4097: Crash in kudu-scan-node-test
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/4337
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e30964232ed137fce7a99bf7e9557293b5905c8
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4097: Crash in kudu-scan-node-test
Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has submitted this change and it was merged.
Change subject: IMPALA-4097: Crash in kudu-scan-node-test
......................................................................
IMPALA-4097: Crash in kudu-scan-node-test
The kudu-scan-node-test was calling GetNext() with a NULL
row batch, which isn't valid. This wasn't failing until a
recent code change, and only occasionally due to the timing
of scanner threads producing row batches. One batch is empty
and the other has 1 row. This test worked when the empty row
batch was added to the batch queue first (which usually
happened), but the test code couldn't handle the other
ordering properly. This fixes the test to be more careful
about what is being exercised.
Change-Id: I0e30964232ed137fce7a99bf7e9557293b5905c8
Reviewed-on: http://gerrit.cloudera.org:8080/4337
Reviewed-by: Dan Hecht <dh...@cloudera.com>
Tested-by: Internal Jenkins
---
M be/src/exec/kudu-scan-node-test.cc
M be/src/exec/kudu-scan-node.cc
2 files changed, 23 insertions(+), 9 deletions(-)
Approvals:
Internal Jenkins: Verified
Dan Hecht: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/4337
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I0e30964232ed137fce7a99bf7e9557293b5905c8
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>