[Impala-ASF-CR] IMPALA-3202: refactor scratch file management into TmpFileMgr
Internal Jenkins has submitted this change and it was merged. Change subject: IMPALA-3202: refactor scratch file management into TmpFileMgr .. IMPALA-3202: refactor scratch file management into TmpFileMgr This is a pure refactoring patch that moves all of the logic for allocating scratch file ranges into TmpFileMgr in anticipation of this logic being used by the new BufferPool. There should be no behavioural changes. Also remove a bunch of TODOs that we're not going to fix. Change-Id: I0c56c195f3f28d520034f8c384494e566635fc62 Reviewed-on: http://gerrit.cloudera.org:8080/4898 Reviewed-by: Tim Armstrong Tested-by: Internal Jenkins --- M be/src/runtime/buffered-block-mgr-test.cc M be/src/runtime/buffered-block-mgr.cc M be/src/runtime/buffered-block-mgr.h 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 6 files changed, 441 insertions(+), 432 deletions(-) Approvals: Internal Jenkins: Verified Tim Armstrong: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/4898 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I0c56c195f3f28d520034f8c384494e566635fc62 Gerrit-PatchSet: 7 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-3202: refactor scratch file management into TmpFileMgr
Internal Jenkins has posted comments on this change. Change subject: IMPALA-3202: refactor scratch file management into TmpFileMgr .. Patch Set 6: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/4898 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0c56c195f3f28d520034f8c384494e566635fc62 Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3202: refactor scratch file management into TmpFileMgr
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3202: refactor scratch file management into TmpFileMgr .. Patch Set 6: Code-Review+2 Rebase -- To view, visit http://gerrit.cloudera.org:8080/4898 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0c56c195f3f28d520034f8c384494e566635fc62 Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3202: refactor scratch file management into TmpFileMgr
Dan Hecht has posted comments on this change. Change subject: IMPALA-3202: refactor scratch file management into TmpFileMgr .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4898 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0c56c195f3f28d520034f8c384494e566635fc62 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3202: refactor scratch file management into TmpFileMgr
Hello Lars Volker, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4898 to look at the new patch set (#5). Change subject: IMPALA-3202: refactor scratch file management into TmpFileMgr .. IMPALA-3202: refactor scratch file management into TmpFileMgr This is a pure refactoring patch that moves all of the logic for allocating scratch file ranges into TmpFileMgr in anticipation of this logic being used by the new BufferPool. There should be no behavioural changes. Also remove a bunch of TODOs that we're not going to fix. Change-Id: I0c56c195f3f28d520034f8c384494e566635fc62 --- M be/src/runtime/buffered-block-mgr-test.cc M be/src/runtime/buffered-block-mgr.cc M be/src/runtime/buffered-block-mgr.h 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 6 files changed, 442 insertions(+), 433 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/98/4898/5 -- To view, visit http://gerrit.cloudera.org:8080/4898 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0c56c195f3f28d520034f8c384494e566635fc62 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-3202: refactor scratch file management into TmpFileMgr
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-3202: refactor scratch file management into TmpFileMgr
..
Patch Set 4:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/4898/4/be/src/runtime/tmp-file-mgr.cc
File be/src/runtime/tmp-file-mgr.cc:
Line 292: " or see log for previous errors that prevented use of
provided directories");
> does the log contain the errors, or do we need to log the error status at l
The errors were logged via ReportIoError(). It makes more sense to accumulate
the errors though and merge them together like we do below.
Line 336: } else if (status.code() == TErrorCode::SCRATCH_LIMIT_EXCEEDED) {
> seems like dead code now that the check is handled at the file group level,
Yes, good we can simplify this.
http://gerrit.cloudera.org:8080/#/c/4898/4/be/src/runtime/tmp-file-mgr.h
File be/src/runtime/tmp-file-mgr.h:
PS4, Line 72: allocation would exceed the allocation limit of its associated
FileGrou
> this check doesn't seem to be done by this function anymore.
Done. Thought I removed this but memory must be faulty.
--
To view, visit http://gerrit.cloudera.org:8080/4898
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c56c195f3f28d520034f8c384494e566635fc62
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Dan Hecht
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Tim Armstrong
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3202: refactor scratch file management into TmpFileMgr
Dan Hecht has posted comments on this change.
Change subject: IMPALA-3202: refactor scratch file management into TmpFileMgr
..
Patch Set 4:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/4898/4/be/src/runtime/tmp-file-mgr.cc
File be/src/runtime/tmp-file-mgr.cc:
Line 292: " or see log for previous errors that prevented use of
provided directories");
does the log contain the errors, or do we need to log the error status at line
286?
Line 336: } else if (status.code() == TErrorCode::SCRATCH_LIMIT_EXCEEDED) {
seems like dead code now that the check is handled at the file group level, no?
http://gerrit.cloudera.org:8080/#/c/4898/4/be/src/runtime/tmp-file-mgr.h
File be/src/runtime/tmp-file-mgr.h:
PS4, Line 72: allocation would exceed the allocation limit of its associated
FileGrou
this check doesn't seem to be done by this function anymore.
--
To view, visit http://gerrit.cloudera.org:8080/4898
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c56c195f3f28d520034f8c384494e566635fc62
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Dan Hecht
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Tim Armstrong
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3202: refactor scratch file management into TmpFileMgr
Hello Lars Volker, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4898 to look at the new patch set (#4). Change subject: IMPALA-3202: refactor scratch file management into TmpFileMgr .. IMPALA-3202: refactor scratch file management into TmpFileMgr This is a pure refactoring patch that moves all of the logic for allocating scratch file ranges into TmpFileMgr in anticipation of this logic being used by the new BufferPool. There should be no behavioural changes. Also remove a bunch of TODOs that we're not going to fix. Change-Id: I0c56c195f3f28d520034f8c384494e566635fc62 --- M be/src/runtime/buffered-block-mgr-test.cc M be/src/runtime/buffered-block-mgr.cc M be/src/runtime/buffered-block-mgr.h 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 6 files changed, 440 insertions(+), 433 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/98/4898/4 -- To view, visit http://gerrit.cloudera.org:8080/4898 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0c56c195f3f28d520034f8c384494e566635fc62 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-3202: refactor scratch file management into TmpFileMgr
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3202: refactor scratch file management into TmpFileMgr .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/4898/3/be/src/runtime/buffered-block-mgr.h File be/src/runtime/buffered-block-mgr.h: Line 508: /// is > nit: line wrapping Done -- To view, visit http://gerrit.cloudera.org:8080/4898 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0c56c195f3f28d520034f8c384494e566635fc62 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3202: refactor scratch file management into TmpFileMgr
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3202: refactor scratch file management into TmpFileMgr .. Patch Set 4: Code-Review+1 Carry +1 -- To view, visit http://gerrit.cloudera.org:8080/4898 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0c56c195f3f28d520034f8c384494e566635fc62 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3202: refactor scratch file management into TmpFileMgr
Lars Volker has posted comments on this change.
Change subject: IMPALA-3202: refactor scratch file management into TmpFileMgr
..
Patch Set 3: Code-Review+1
(4 comments)
http://gerrit.cloudera.org:8080/#/c/4898/3/be/src/runtime/buffered-block-mgr.h
File be/src/runtime/buffered-block-mgr.h:
Line 508: /// is
nit: line wrapping
http://gerrit.cloudera.org:8080/#/c/4898/1/be/src/runtime/tmp-file-mgr.cc
File be/src/runtime/tmp-file-mgr.cc:
PS1, Line 296: x_ = ran
> Good point, users have been confused by this before when the mixed terminol
I agree that replacing Tmp with Scratch might not be worth it. Having those two
works for me.
Line 336: } else if (status.code() == TErrorCode::SCRATCH_LIMIT_EXCEEDED) {
> I prefer it this way since it's more obvious that it's an "successful" retu
Good point.
http://gerrit.cloudera.org:8080/#/c/4898/1/be/src/runtime/tmp-file-mgr.h
File be/src/runtime/tmp-file-mgr.h:
PS1, Line 136: unique_i
> I don't think it will ever be anything asides from the query id, but maybe
Thanks. When I first saw query_id I assumed there might be some side effects
that require query id semantics, so unique_id seems clearer to me.
--
To view, visit http://gerrit.cloudera.org:8080/4898
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c56c195f3f28d520034f8c384494e566635fc62
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Dan Hecht
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Tim Armstrong
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3202: refactor scratch file management into TmpFileMgr
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3202: refactor scratch file management into TmpFileMgr .. Patch Set 3: PS3 is a rebase -- To view, visit http://gerrit.cloudera.org:8080/4898 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0c56c195f3f28d520034f8c384494e566635fc62 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3202: refactor scratch file management into TmpFileMgr
Tim Armstrong has uploaded a new patch set (#3). Change subject: IMPALA-3202: refactor scratch file management into TmpFileMgr .. IMPALA-3202: refactor scratch file management into TmpFileMgr This is a pure refactoring patch that moves all of the logic for allocating scratch file ranges into TmpFileMgr in anticipation of this logic being used by the new BufferPool. There should be no behavioural changes. Also remove a bunch of TODOs that we're not going to fix. Change-Id: I0c56c195f3f28d520034f8c384494e566635fc62 --- M be/src/runtime/buffered-block-mgr-test.cc M be/src/runtime/buffered-block-mgr.cc M be/src/runtime/buffered-block-mgr.h 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 6 files changed, 439 insertions(+), 432 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/98/4898/3 -- To view, visit http://gerrit.cloudera.org:8080/4898 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0c56c195f3f28d520034f8c384494e566635fc62 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-3202: refactor scratch file management into TmpFileMgr
Tim Armstrong has uploaded a new patch set (#2). Change subject: IMPALA-3202: refactor scratch file management into TmpFileMgr .. IMPALA-3202: refactor scratch file management into TmpFileMgr This is a pure refactoring patch that moves all of the logic for allocating scratch file ranges into TmpFileMgr in anticipation of this logic being used by the new BufferPool. There should be no behavioural changes. Also remove a bunch of TODOs that we're not going to fix. Change-Id: I0c56c195f3f28d520034f8c384494e566635fc62 --- M be/src/runtime/buffered-block-mgr-test.cc M be/src/runtime/buffered-block-mgr.cc M be/src/runtime/buffered-block-mgr.h 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 6 files changed, 441 insertions(+), 434 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/98/4898/2 -- To view, visit http://gerrit.cloudera.org:8080/4898 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0c56c195f3f28d520034f8c384494e566635fc62 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-3202: refactor scratch file management into TmpFileMgr
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3202: refactor scratch file management into TmpFileMgr .. Patch Set 1: (10 comments) http://gerrit.cloudera.org:8080/#/c/4898/1/be/src/runtime/buffered-block-mgr-test.cc File be/src/runtime/buffered-block-mgr-test.cc: PS1, Line 308: EXPECT_TRUE > ASSERT_TRUE? Otherwise we will hit a NPE in DeleteBlocks(). Makes sense. This pattern of always using EXPECT is unfortunately everywhere in the unit tests. Mostly I think ASSERT makes the most sense. I just did a search-and-replace on this file to change it everywhere in this file where I could (it's only allowed in functions that return void). http://gerrit.cloudera.org:8080/#/c/4898/1/be/src/runtime/buffered-block-mgr.cc File be/src/runtime/buffered-block-mgr.cc: Line 246: ->Init(state->io_mgr(), tmp_file_mgr, profile, parent, mem_limit, scratch_limit); > This line break looks odd. If it was done by clang-format I'd keep it, but I agree that it looks weird. clang-format did it. http://gerrit.cloudera.org:8080/#/c/4898/1/be/src/runtime/tmp-file-mgr.cc File be/src/runtime/tmp-file-mgr.cc: Line 282: DCHECK_EQ(tmp_files_.size(), 0); > nit: DCHECK(tmp_files_.empty()); Done PS1, Line 290: by ignoring the return status of : // NewFile(). > we don't seem to actually ignore it, the comment looks wrong. True, made the comment more accurate. PS1, Line 296: spilling > "No scratch directories..."? We seem to use 'tmp', 'scratch', and 'spilling Good point, users have been confused by this before when the mixed terminology leaked into error messages: IMPALA-3866. We need to keep scratch as the user-facing term to match the command-line option, so I fixed the "spilling" term here. We could rename TmpFileMgr, etc to ScratchFileMgr, I'm unsure if it's worth the code churn though - what do you think? Line 335: scratch_space_bytes_used_counter_->Add(num_bytes); > can we handle current_bytes_allocated_ here, too? That's also the only dire That works out cleaner, thanks. Line 336: return Status::OK(); > nit: this could just be "return status". I prefer it this way since it's more obvious that it's an "successful" return path. I don't feel that strongly but I find this easier to parse. Line 351: err_status.MergeStatus(errs[i]); > nit: single line Done http://gerrit.cloudera.org:8080/#/c/4898/1/be/src/runtime/tmp-file-mgr.h File be/src/runtime/tmp-file-mgr.h: PS1, Line 133: > nit: double space Done PS1, Line 136: query_id > does this actually have to be the query_id or could we rename it to somethi I don't think it will ever be anything asides from the query id, but maybe "unique_id" makes more sense. -- To view, visit http://gerrit.cloudera.org:8080/4898 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0c56c195f3f28d520034f8c384494e566635fc62 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3202: refactor scratch file management into TmpFileMgr
Lars Volker has posted comments on this change. Change subject: IMPALA-3202: refactor scratch file management into TmpFileMgr .. Patch Set 1: (10 comments) http://gerrit.cloudera.org:8080/#/c/4898/1/be/src/runtime/buffered-block-mgr-test.cc File be/src/runtime/buffered-block-mgr-test.cc: PS1, Line 308: EXPECT_TRUE ASSERT_TRUE? Otherwise we will hit a NPE in DeleteBlocks(). http://gerrit.cloudera.org:8080/#/c/4898/1/be/src/runtime/buffered-block-mgr.cc File be/src/runtime/buffered-block-mgr.cc: Line 246: ->Init(state->io_mgr(), tmp_file_mgr, profile, parent, mem_limit, scratch_limit); This line break looks odd. If it was done by clang-format I'd keep it, but otherwise maybe break after Init( . http://gerrit.cloudera.org:8080/#/c/4898/1/be/src/runtime/tmp-file-mgr.cc File be/src/runtime/tmp-file-mgr.cc: Line 282: DCHECK_EQ(tmp_files_.size(), 0); nit: DCHECK(tmp_files_.empty()); PS1, Line 290: by ignoring the return status of : // NewFile(). we don't seem to actually ignore it, the comment looks wrong. PS1, Line 296: spilling "No scratch directories..."? We seem to use 'tmp', 'scratch', and 'spilling' interchangeably, maybe reduce to one or two of them? Line 335: scratch_space_bytes_used_counter_->Add(num_bytes); can we handle current_bytes_allocated_ here, too? That's also the only direct write access I could find from File to a field of FileGroup, so maybe we can unfriend them, too. Line 336: return Status::OK(); nit: this could just be "return status". Line 351: err_status.MergeStatus(errs[i]); nit: single line http://gerrit.cloudera.org:8080/#/c/4898/1/be/src/runtime/tmp-file-mgr.h File be/src/runtime/tmp-file-mgr.h: PS1, Line 133: nit: double space PS1, Line 136: query_id does this actually have to be the query_id or could we rename it to something like filegroup_id? -- To view, visit http://gerrit.cloudera.org:8080/4898 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0c56c195f3f28d520034f8c384494e566635fc62 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Lars Volker Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3202: refactor scratch file management into TmpFileMgr
Tim Armstrong has uploaded a new change for review. http://gerrit.cloudera.org:8080/4898 Change subject: IMPALA-3202: refactor scratch file management into TmpFileMgr .. IMPALA-3202: refactor scratch file management into TmpFileMgr This is a pure refactoring patch that moves all of the logic for allocating scratch file ranges into TmpFileMgr in anticipation of this logic being used by the new BufferPool. There should be no behavioural changes. Also remove a bunch of TODOs that we're not going to fix. Testing: Covered by TmpFileMgr and BufferedBlockMgr unit tests. Code is exercised heavily by spilling system tests and the stress test. Change-Id: I0c56c195f3f28d520034f8c384494e566635fc62 --- M be/src/runtime/buffered-block-mgr-test.cc M be/src/runtime/buffered-block-mgr.cc M be/src/runtime/buffered-block-mgr.h 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 6 files changed, 193 insertions(+), 203 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/98/4898/1 -- To view, visit http://gerrit.cloudera.org:8080/4898 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I0c56c195f3f28d520034f8c384494e566635fc62 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong
