You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Henry Robinson (Code Review)" <ge...@cloudera.org> on 2016/12/02 23:41:15 UTC
[Impala-ASF-CR] IMPALA-4580: Fix crash with FETCH FIRST when #rows < result cache size
Henry Robinson has uploaded a new change for review.
http://gerrit.cloudera.org:8080/5335
Change subject: IMPALA-4580: Fix crash with FETCH_FIRST when #rows < result cache size
......................................................................
IMPALA-4580: Fix crash with FETCH_FIRST when #rows < result cache size
The following sequence can lead to a crash:
1. Client sets result cache size to N
2. Client issues query with #results < N
3. Client fetches all results, triggering eos and tearing down
Coordinator::root_sink_.
4. Client restarts query with FETCH_FIRST.
5. Client reads all results again. After cache is exhausted,
Coordinator::GetNext() is called to detect eos condition again.
6. GetNext() hits DCHECK(root_sink_ != nullptr).
This patch makes GetNext() a no-op if called after it sets *eos,
avoiding the crash..
Testing:
Regression test that triggered the bug before this fix.
Change-Id: I454cd8a6cf438bdd0c49fd27c2725d8f6c43bb1d
---
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
M tests/hs2/test_fetch_first.py
3 files changed, 38 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/35/5335/1
--
To view, visit http://gerrit.cloudera.org:8080/5335
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I454cd8a6cf438bdd0c49fd27c2725d8f6c43bb1d
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
[Impala-ASF-CR] IMPALA-4580: Fix crash with FETCH FIRST when #rows < result cache size
Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has submitted this change and it was merged.
Change subject: IMPALA-4580: Fix crash with FETCH_FIRST when #rows < result cache size
......................................................................
IMPALA-4580: Fix crash with FETCH_FIRST when #rows < result cache size
The following sequence can lead to a crash:
1. Client sets result cache size to N
2. Client issues query with #results < N
3. Client fetches all results, triggering eos and tearing down
Coordinator::root_sink_.
4. Client restarts query with FETCH_FIRST.
5. Client reads all results again. After cache is exhausted,
Coordinator::GetNext() is called to detect eos condition again.
6. GetNext() hits DCHECK(root_sink_ != nullptr).
This patch makes GetNext() a no-op if called after it sets *eos,
avoiding the crash..
Testing:
Regression test that triggered the bug before this fix.
Change-Id: I454cd8a6cf438bdd0c49fd27c2725d8f6c43bb1d
Reviewed-on: http://gerrit.cloudera.org:8080/5335
Reviewed-by: Henry Robinson <he...@cloudera.com>
Reviewed-by: Dan Hecht <dh...@cloudera.com>
Tested-by: Internal Jenkins
---
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
M tests/hs2/test_fetch_first.py
3 files changed, 39 insertions(+), 2 deletions(-)
Approvals:
Henry Robinson: Looks good to me, approved
Internal Jenkins: Verified
Dan Hecht: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/5335
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I454cd8a6cf438bdd0c49fd27c2725d8f6c43bb1d
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
[Impala-ASF-CR] IMPALA-4580: Fix crash with FETCH FIRST when #rows < result cache size
Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Hello Dan Hecht,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/5335
to look at the new patch set (#2).
Change subject: IMPALA-4580: Fix crash with FETCH_FIRST when #rows < result cache size
......................................................................
IMPALA-4580: Fix crash with FETCH_FIRST when #rows < result cache size
The following sequence can lead to a crash:
1. Client sets result cache size to N
2. Client issues query with #results < N
3. Client fetches all results, triggering eos and tearing down
Coordinator::root_sink_.
4. Client restarts query with FETCH_FIRST.
5. Client reads all results again. After cache is exhausted,
Coordinator::GetNext() is called to detect eos condition again.
6. GetNext() hits DCHECK(root_sink_ != nullptr).
This patch makes GetNext() a no-op if called after it sets *eos,
avoiding the crash..
Testing:
Regression test that triggered the bug before this fix.
Change-Id: I454cd8a6cf438bdd0c49fd27c2725d8f6c43bb1d
---
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
M tests/hs2/test_fetch_first.py
3 files changed, 39 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/35/5335/2
--
To view, visit http://gerrit.cloudera.org:8080/5335
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I454cd8a6cf438bdd0c49fd27c2725d8f6c43bb1d
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
[Impala-ASF-CR] IMPALA-4580: Fix crash with FETCH FIRST when #rows < result cache size
Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.
Change subject: IMPALA-4580: Fix crash with FETCH_FIRST when #rows < result cache size
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/5335
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I454cd8a6cf438bdd0c49fd27c2725d8f6c43bb1d
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4580: Fix crash with FETCH FIRST when #rows < result cache size
Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has posted comments on this change.
Change subject: IMPALA-4580: Fix crash with FETCH_FIRST when #rows < result cache size
......................................................................
Patch Set 2: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/5335
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I454cd8a6cf438bdd0c49fd27c2725d8f6c43bb1d
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4580: Fix crash with FETCH FIRST when #rows < result cache size
Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has posted comments on this change.
Change subject: IMPALA-4580: Fix crash with FETCH_FIRST when #rows < result cache size
......................................................................
Patch Set 2: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5335/1/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:
Line 1108: // May be called after the first time we set *eos. Re-set *eos and return here;
> This comment seemed misleading to me becuase it makes it sound like it's he
Done
--
To view, visit http://gerrit.cloudera.org:8080/5335
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I454cd8a6cf438bdd0c49fd27c2725d8f6c43bb1d
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4580: Fix crash with FETCH FIRST when #rows < result cache size
Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.
Change subject: IMPALA-4580: Fix crash with FETCH_FIRST when #rows < result cache size
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5335/1/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:
Line 1108: // May be called after the first time we set *eos. Re-set *eos and return quickly.
This comment seemed misleading to me becuase it makes it sound like it's here just as an optimization. How about either deleting this comment or rewording about the teardown?
--
To view, visit http://gerrit.cloudera.org:8080/5335
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I454cd8a6cf438bdd0c49fd27c2725d8f6c43bb1d
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-HasComments: Yes