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/06/26 15:48:28 UTC
[Impala-ASF-CR] IMPALA-5554: sorter DCHECK on null column
Tim Armstrong has uploaded a new change for review.
http://gerrit.cloudera.org:8080/7295
Change subject: IMPALA-5554: sorter DCHECK on null column
......................................................................
IMPALA-5554: sorter DCHECK on null column
Testing:
Added a test that reproduces the crash.
Change-Id: I7a8dee982501008efff5b5abc192cfb5e6544a90
---
M be/src/runtime/sorter.cc
M testdata/workloads/functional-query/queries/QueryTest/single-node-large-sorts.test
2 files changed, 39 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/7295/1
--
To view, visit http://gerrit.cloudera.org:8080/7295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7a8dee982501008efff5b5abc192cfb5e6544a90
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-5554: sorter DCHECK on null column
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-5554: sorter DCHECK on null column
......................................................................
Patch Set 4: Code-Review+2
Carry +2
--
To view, visit http://gerrit.cloudera.org:8080/7295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I7a8dee982501008efff5b5abc192cfb5e6544a90
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5554: sorter DCHECK on null column
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Hello Thomas Tauber-Marshall, Alex Behm,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7295
to look at the new patch set (#3).
Change subject: IMPALA-5554: sorter DCHECK on null column
......................................................................
IMPALA-5554: sorter DCHECK on null column
The bug was in the DCHECK. The DCHECK is intended to make sure that a
tuple's string data didn't get split across blocks. The logic assumed
that if the second-or-later string column was in the next-block, that
the strings were split between blocks. However, that assumption is
invalid if there are NULL strings, which do not belong in any block.
The fix for the DCHECK (which is still useful) is to count the number
of non-NULL strings and make sure that no non-NULL strings were split
between blocks.
Testing:
Added a test that reproduces the crash.
Change-Id: I7a8dee982501008efff5b5abc192cfb5e6544a90
---
M be/src/runtime/sorter.cc
M testdata/workloads/functional-query/queries/QueryTest/single-node-large-sorts.test
2 files changed, 39 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/7295/3
--
To view, visit http://gerrit.cloudera.org:8080/7295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7a8dee982501008efff5b5abc192cfb5e6544a90
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-5554: sorter DCHECK on null column
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-5554: sorter DCHECK on null column
......................................................................
Patch Set 4:
Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/788/
--
To view, visit http://gerrit.cloudera.org:8080/7295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I7a8dee982501008efff5b5abc192cfb5e6544a90
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5554: sorter DCHECK on null column
Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change.
Change subject: IMPALA-5554: sorter DCHECK on null column
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7295/1//COMMIT_MSG
Commit Message:
Line 8:
Brief description of the problem.
--
To view, visit http://gerrit.cloudera.org:8080/7295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I7a8dee982501008efff5b5abc192cfb5e6544a90
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5554: sorter DCHECK on null column
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-5554: sorter DCHECK on null column
......................................................................
Patch Set 2: Code-Review+1
(1 comment)
carry +1
http://gerrit.cloudera.org:8080/#/c/7295/1//COMMIT_MSG
Commit Message:
Line 8:
> Brief description of the problem.
Done
--
To view, visit http://gerrit.cloudera.org:8080/7295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I7a8dee982501008efff5b5abc192cfb5e6544a90
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5554: sorter DCHECK on null column
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged.
Change subject: IMPALA-5554: sorter DCHECK on null column
......................................................................
IMPALA-5554: sorter DCHECK on null column
The bug was in the DCHECK. The DCHECK is intended to make sure that a
tuple's string data didn't get split across blocks. The logic assumed
that if the second-or-later string column was in the next-block, that
the strings were split between blocks. However, that assumption is
invalid if there are NULL strings, which do not belong in any block.
The fix for the DCHECK (which is still useful) is to count the number
of non-NULL strings and make sure that no non-NULL strings were split
between blocks.
Testing:
Added a test that reproduces the crash.
Change-Id: I7a8dee982501008efff5b5abc192cfb5e6544a90
Reviewed-on: http://gerrit.cloudera.org:8080/7295
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M be/src/runtime/sorter.cc
M testdata/workloads/functional-query/queries/QueryTest/single-node-large-sorts.test
2 files changed, 39 insertions(+), 3 deletions(-)
Approvals:
Impala Public Jenkins: Verified
Tim Armstrong: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/7295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I7a8dee982501008efff5b5abc192cfb5e6544a90
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-5554: sorter DCHECK on null column
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.
Change subject: IMPALA-5554: sorter DCHECK on null column
......................................................................
Patch Set 2: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7295/2/be/src/runtime/sorter.cc
File be/src/runtime/sorter.cc:
Line 1031: // This must be the first column with var len data for the tuple. Var len data
first column -> first slot
--
To view, visit http://gerrit.cloudera.org:8080/7295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I7a8dee982501008efff5b5abc192cfb5e6544a90
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5554: sorter DCHECK on null column
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-5554: sorter DCHECK on null column
......................................................................
Patch Set 4:
Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/792/
--
To view, visit http://gerrit.cloudera.org:8080/7295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I7a8dee982501008efff5b5abc192cfb5e6544a90
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5554: sorter DCHECK on null column
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-5554: sorter DCHECK on null column
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7295/2/be/src/runtime/sorter.cc
File be/src/runtime/sorter.cc:
Line 1031: // This must be the first column with var len data for the tuple. Var len data
> first column -> first slot
Done
--
To view, visit http://gerrit.cloudera.org:8080/7295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I7a8dee982501008efff5b5abc192cfb5e6544a90
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5554: sorter DCHECK on null column
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-5554: sorter DCHECK on null column
......................................................................
Patch Set 4: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/7295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I7a8dee982501008efff5b5abc192cfb5e6544a90
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5554: sorter DCHECK on null column
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Hello Thomas Tauber-Marshall,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7295
to look at the new patch set (#2).
Change subject: IMPALA-5554: sorter DCHECK on null column
......................................................................
IMPALA-5554: sorter DCHECK on null column
The bug was in the DCHECK. The DCHECK is intended to make sure that a
tuple's string data didn't get split across blocks. The logic assumed
that if the second-or-later string column was in the next-block, that
the strings were split between blocks. However, that assumption is
invalid if there are NULL strings, which do not belong in any block.
The fix for the DCHECK (which is still useful) is to count the number
of non-NULL strings and make sure that no non-NULL strings were split
between blocks.
Testing:
Added a test that reproduces the crash.
Change-Id: I7a8dee982501008efff5b5abc192cfb5e6544a90
---
M be/src/runtime/sorter.cc
M testdata/workloads/functional-query/queries/QueryTest/single-node-large-sorts.test
2 files changed, 39 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/7295/2
--
To view, visit http://gerrit.cloudera.org:8080/7295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7a8dee982501008efff5b5abc192cfb5e6544a90
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-5554: sorter DCHECK on null column
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-5554: sorter DCHECK on null column
......................................................................
Patch Set 4: Verified-1
Build failed: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/788/
--
To view, visit http://gerrit.cloudera.org:8080/7295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I7a8dee982501008efff5b5abc192cfb5e6544a90
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No