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/01/12 02:08:05 UTC
[Impala-ASF-CR] IMPALA-4748: crash in TmpFileMgr when hitting process mem limit
Tim Armstrong has uploaded a new change for review.
http://gerrit.cloudera.org:8080/5683
Change subject: IMPALA-4748: crash in TmpFileMgr when hitting process mem limit
......................................................................
IMPALA-4748: crash in TmpFileMgr when hitting process mem limit
The bug is that FileGroup didn't correctly handle its 'io_ctx_'
being asynchronously cancelled: it left the WriteHandle in an
invalid state. This could happen when the process memory limit
was exceeded.
I fixed this in two ways (either of which would be sufficient
to avoid this exact crash):
* Fix the error handling in TmpFileMgr so that things are left
in a valid state on the error path.
* Stop DiskIoMgr from asynchronously cancelling I/O contexts
with no associated MemTracker. The mem_limit check and error
propagation is necessary when DiskIoMgr will allocate memory
on behalf of the client, but is not necessary when it is not
allocating memory for the client - it just added a redundant
error propagation mechanism.
Testing:
This scenario should no longer be possible for BufferedBlockMgr
since DiskIoMgr won't cancel its I/O context, since it has no
associated MemTracker. However, to test that errors on this path
are correctly handled, I added a simple unit test to TmpFileMgr
that forces cancellation of the I/O context.
Change-Id: Ib0a624212bc17f7824e6d14ad143c0d5894206f8
---
M be/src/runtime/disk-io-mgr.cc
M be/src/runtime/tmp-file-mgr-test.cc
M be/src/runtime/tmp-file-mgr.cc
M be/src/runtime/tmp-file-mgr.h
4 files changed, 48 insertions(+), 11 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/5683/2
--
To view, visit http://gerrit.cloudera.org:8080/5683
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib0a624212bc17f7824e6d14ad143c0d5894206f8
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-4748: crash in TmpFileMgr when hitting process mem limit
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/5683
to look at the new patch set (#4).
Change subject: IMPALA-4748: crash in TmpFileMgr when hitting process mem limit
......................................................................
IMPALA-4748: crash in TmpFileMgr when hitting process mem limit
The bug is that FileGroup didn't correctly handle its 'io_ctx_'
being asynchronously cancelled: it left the WriteHandle in an
invalid state. This could happen when the process memory limit
was exceeded.
I fixed this in two ways (either of which would be sufficient
to avoid this exact crash):
* Fix the error handling in TmpFileMgr so that things are left
in a valid state on the error path.
* Stop DiskIoMgr from asynchronously cancelling I/O contexts
with no associated MemTracker. The mem_limit check and error
propagation is necessary when DiskIoMgr will allocate memory
on behalf of the client, but is not necessary when it is not
allocating memory for the client - it just added a redundant
error propagation mechanism.
Testing:
This scenario should no longer be possible for BufferedBlockMgr
since DiskIoMgr won't cancel its I/O context, since it has no
associated MemTracker. However, to test that errors on this path
are correctly handled, I added a simple unit test to TmpFileMgr
that forces cancellation of the I/O context.
Change-Id: Ib0a624212bc17f7824e6d14ad143c0d5894206f8
---
M be/src/runtime/disk-io-mgr.cc
M be/src/runtime/tmp-file-mgr-test.cc
M be/src/runtime/tmp-file-mgr.cc
M be/src/runtime/tmp-file-mgr.h
4 files changed, 61 insertions(+), 16 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/5683/4
--
To view, visit http://gerrit.cloudera.org:8080/5683
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib0a624212bc17f7824e6d14ad143c0d5894206f8
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-4748: crash in TmpFileMgr when hitting process mem limit
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-4748: crash in TmpFileMgr when hitting process mem limit
......................................................................
Patch Set 6: Code-Review+2
There was a mistake in merging when I rebased. Fixed it.
--
To view, visit http://gerrit.cloudera.org:8080/5683
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0a624212bc17f7824e6d14ad143c0d5894206f8
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@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-4748: crash in TmpFileMgr when hitting process mem limit
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-4748: crash in TmpFileMgr when hitting process mem limit
......................................................................
Patch Set 5: Verified-1
Build failed: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/195/
--
To view, visit http://gerrit.cloudera.org:8080/5683
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0a624212bc17f7824e6d14ad143c0d5894206f8
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@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-4748: crash in TmpFileMgr when hitting process mem limit
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-4748: crash in TmpFileMgr when hitting process mem limit
......................................................................
Patch Set 5:
Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/195/
--
To view, visit http://gerrit.cloudera.org:8080/5683
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0a624212bc17f7824e6d14ad143c0d5894206f8
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@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-4748: crash in TmpFileMgr when hitting process mem limit
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has uploaded a new patch set (#3).
Change subject: IMPALA-4748: crash in TmpFileMgr when hitting process mem limit
......................................................................
IMPALA-4748: crash in TmpFileMgr when hitting process mem limit
The bug is that FileGroup didn't correctly handle its 'io_ctx_'
being asynchronously cancelled: it left the WriteHandle in an
invalid state. This could happen when the process memory limit
was exceeded.
I fixed this in two ways (either of which would be sufficient
to avoid this exact crash):
* Fix the error handling in TmpFileMgr so that things are left
in a valid state on the error path.
* Stop DiskIoMgr from asynchronously cancelling I/O contexts
with no associated MemTracker. The mem_limit check and error
propagation is necessary when DiskIoMgr will allocate memory
on behalf of the client, but is not necessary when it is not
allocating memory for the client - it just added a redundant
error propagation mechanism.
Testing:
This scenario should no longer be possible for BufferedBlockMgr
since DiskIoMgr won't cancel its I/O context, since it has no
associated MemTracker. However, to test that errors on this path
are correctly handled, I added a simple unit test to TmpFileMgr
that forces cancellation of the I/O context.
Change-Id: Ib0a624212bc17f7824e6d14ad143c0d5894206f8
---
M be/src/runtime/disk-io-mgr.cc
M be/src/runtime/tmp-file-mgr-test.cc
M be/src/runtime/tmp-file-mgr.cc
M be/src/runtime/tmp-file-mgr.h
4 files changed, 57 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/5683/3
--
To view, visit http://gerrit.cloudera.org:8080/5683
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib0a624212bc17f7824e6d14ad143c0d5894206f8
Gerrit-PatchSet: 3
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-4748: crash in TmpFileMgr when hitting process mem limit
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged.
Change subject: IMPALA-4748: crash in TmpFileMgr when hitting process mem limit
......................................................................
IMPALA-4748: crash in TmpFileMgr when hitting process mem limit
The bug is that FileGroup didn't correctly handle its 'io_ctx_'
being asynchronously cancelled: it left the WriteHandle in an
invalid state. This could happen when the process memory limit
was exceeded.
I fixed this in two ways (either of which would be sufficient
to avoid this exact crash):
* Fix the error handling in TmpFileMgr so that things are left
in a valid state on the error path.
* Stop DiskIoMgr from asynchronously cancelling I/O contexts
with no associated MemTracker. The mem_limit check and error
propagation is necessary when DiskIoMgr will allocate memory
on behalf of the client, but is not necessary when it is not
allocating memory for the client - it just added a redundant
error propagation mechanism.
Testing:
This scenario should no longer be possible for BufferedBlockMgr
since DiskIoMgr won't cancel its I/O context, since it has no
associated MemTracker. However, to test that errors on this path
are correctly handled, I added a simple unit test to TmpFileMgr
that forces cancellation of the I/O context.
Change-Id: Ib0a624212bc17f7824e6d14ad143c0d5894206f8
Reviewed-on: http://gerrit.cloudera.org:8080/5683
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M be/src/runtime/disk-io-mgr.cc
M be/src/runtime/tmp-file-mgr-test.cc
M be/src/runtime/tmp-file-mgr.cc
M be/src/runtime/tmp-file-mgr.h
4 files changed, 61 insertions(+), 16 deletions(-)
Approvals:
Impala Public Jenkins: Verified
Tim Armstrong: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/5683
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib0a624212bc17f7824e6d14ad143c0d5894206f8
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@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-4748: crash in TmpFileMgr when hitting process mem limit
Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.
Change subject: IMPALA-4748: crash in TmpFileMgr when hitting process mem limit
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/5683/3/be/src/runtime/tmp-file-mgr.cc
File be/src/runtime/tmp-file-mgr.cc:
Line 531: is_cancelled_ = true;
i don't understand why this line is needed (or the right thing), even with the comment.
Line 545: write_in_flight_ = false;
and then why does this case not need to set is_cancelled if the other one needs it?
--
To view, visit http://gerrit.cloudera.org:8080/5683
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0a624212bc17f7824e6d14ad143c0d5894206f8
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4748: crash in TmpFileMgr when hitting process mem limit
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Hello Thomas Tauber-Marshall, Dan Hecht,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/5683
to look at the new patch set (#5).
Change subject: IMPALA-4748: crash in TmpFileMgr when hitting process mem limit
......................................................................
IMPALA-4748: crash in TmpFileMgr when hitting process mem limit
The bug is that FileGroup didn't correctly handle its 'io_ctx_'
being asynchronously cancelled: it left the WriteHandle in an
invalid state. This could happen when the process memory limit
was exceeded.
I fixed this in two ways (either of which would be sufficient
to avoid this exact crash):
* Fix the error handling in TmpFileMgr so that things are left
in a valid state on the error path.
* Stop DiskIoMgr from asynchronously cancelling I/O contexts
with no associated MemTracker. The mem_limit check and error
propagation is necessary when DiskIoMgr will allocate memory
on behalf of the client, but is not necessary when it is not
allocating memory for the client - it just added a redundant
error propagation mechanism.
Testing:
This scenario should no longer be possible for BufferedBlockMgr
since DiskIoMgr won't cancel its I/O context, since it has no
associated MemTracker. However, to test that errors on this path
are correctly handled, I added a simple unit test to TmpFileMgr
that forces cancellation of the I/O context.
Change-Id: Ib0a624212bc17f7824e6d14ad143c0d5894206f8
---
M be/src/runtime/disk-io-mgr.cc
M be/src/runtime/tmp-file-mgr-test.cc
M be/src/runtime/tmp-file-mgr.cc
M be/src/runtime/tmp-file-mgr.h
4 files changed, 66 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/5683/5
--
To view, visit http://gerrit.cloudera.org:8080/5683
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib0a624212bc17f7824e6d14ad143c0d5894206f8
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-4748: crash in TmpFileMgr when hitting process mem limit
Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change.
Change subject: IMPALA-4748: crash in TmpFileMgr when hitting process mem limit
......................................................................
Patch Set 3: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/5683
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0a624212bc17f7824e6d14ad143c0d5894206f8
Gerrit-PatchSet: 3
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: No
[Impala-ASF-CR] IMPALA-4748: crash in TmpFileMgr when hitting process mem limit
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-4748: crash in TmpFileMgr when hitting process mem limit
......................................................................
Patch Set 6: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/5683
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0a624212bc17f7824e6d14ad143c0d5894206f8
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@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-4748: crash in TmpFileMgr when hitting process mem limit
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, Thomas Tauber-Marshall, Dan Hecht,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/5683
to look at the new patch set (#6).
Change subject: IMPALA-4748: crash in TmpFileMgr when hitting process mem limit
......................................................................
IMPALA-4748: crash in TmpFileMgr when hitting process mem limit
The bug is that FileGroup didn't correctly handle its 'io_ctx_'
being asynchronously cancelled: it left the WriteHandle in an
invalid state. This could happen when the process memory limit
was exceeded.
I fixed this in two ways (either of which would be sufficient
to avoid this exact crash):
* Fix the error handling in TmpFileMgr so that things are left
in a valid state on the error path.
* Stop DiskIoMgr from asynchronously cancelling I/O contexts
with no associated MemTracker. The mem_limit check and error
propagation is necessary when DiskIoMgr will allocate memory
on behalf of the client, but is not necessary when it is not
allocating memory for the client - it just added a redundant
error propagation mechanism.
Testing:
This scenario should no longer be possible for BufferedBlockMgr
since DiskIoMgr won't cancel its I/O context, since it has no
associated MemTracker. However, to test that errors on this path
are correctly handled, I added a simple unit test to TmpFileMgr
that forces cancellation of the I/O context.
Change-Id: Ib0a624212bc17f7824e6d14ad143c0d5894206f8
---
M be/src/runtime/disk-io-mgr.cc
M be/src/runtime/tmp-file-mgr-test.cc
M be/src/runtime/tmp-file-mgr.cc
M be/src/runtime/tmp-file-mgr.h
4 files changed, 61 insertions(+), 16 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/5683/6
--
To view, visit http://gerrit.cloudera.org:8080/5683
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib0a624212bc17f7824e6d14ad143c0d5894206f8
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@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-4748: crash in TmpFileMgr when hitting process mem limit
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-4748: crash in TmpFileMgr when hitting process mem limit
......................................................................
Patch Set 2:
(1 comment)
Hopefully that is a little clearer.
http://gerrit.cloudera.org:8080/#/c/5683/2/be/src/runtime/tmp-file-mgr.h
File be/src/runtime/tmp-file-mgr.h:
Line 282: Status Write(DiskIoMgr* io_mgr, DiskIoRequestContext* io_ctx, File* file,
> Maybe briefly note pre/post conditions on write_in_flight_ here and for Ret
Done
--
To view, visit http://gerrit.cloudera.org:8080/5683
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0a624212bc17f7824e6d14ad143c0d5894206f8
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-4748: crash in TmpFileMgr when hitting process mem limit
Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change.
Change subject: IMPALA-4748: crash in TmpFileMgr when hitting process mem limit
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5683/2/be/src/runtime/tmp-file-mgr.h
File be/src/runtime/tmp-file-mgr.h:
Line 282: Status Write(DiskIoMgr* io_mgr, DiskIoRequestContext* io_ctx, File* file,
Maybe briefly note pre/post conditions on write_in_flight_ here and for RetryWrite below.
--
To view, visit http://gerrit.cloudera.org:8080/5683
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0a624212bc17f7824e6d14ad143c0d5894206f8
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-HasComments: Yes
[Impala-ASF-CR] IMPALA-4748: crash in TmpFileMgr when hitting process mem limit
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-4748: crash in TmpFileMgr when hitting process mem limit
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/5683/3/be/src/runtime/tmp-file-mgr.cc
File be/src/runtime/tmp-file-mgr.cc:
Line 531: is_cancelled_ = true;
> i don't understand why this line is needed (or the right thing), even with
Clarified a bit more. It avoids the DCHECK in ~WriteHandle(). I want to leave the DCHECK there since it can detect if the WriteHandle wasn't cleaned up properly by DestroyWriteHandle() or CancelWriteAndDestroyData().
The key thing is that if we fail here, the WriteHandle reference is not returned to the caller of Write(), so we won't go through the normal destruction path of DestroyWriteHandle() or CancelWriteAndDestroyData().
Line 545: write_in_flight_ = false;
> and then why does this case not need to set is_cancelled if the other one n
If we fail here, the WriteHandle was returned to the FileGroup client by Write(). So it will be destroyed in the normal way by calling DestroyWriteHandle() or CancelWriteAndDestroyData().
--
To view, visit http://gerrit.cloudera.org:8080/5683
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0a624212bc17f7824e6d14ad143c0d5894206f8
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4748: crash in TmpFileMgr when hitting process mem limit
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-4748: crash in TmpFileMgr when hitting process mem limit
......................................................................
Patch Set 6:
Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/196/
--
To view, visit http://gerrit.cloudera.org:8080/5683
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0a624212bc17f7824e6d14ad143c0d5894206f8
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@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-4748: crash in TmpFileMgr when hitting process mem limit
Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.
Change subject: IMPALA-4748: crash in TmpFileMgr when hitting process mem limit
......................................................................
Patch Set 4: Code-Review+2
Okay. Was wondering if it was for that dcheck; this makes it clear.
--
To view, visit http://gerrit.cloudera.org:8080/5683
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0a624212bc17f7824e6d14ad143c0d5894206f8
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No