impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael Ho (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4180: Synchronize accesses to RuntimeState::reader contexts
Date Thu, 29 Sep 2016 17:52:41 GMT
Michael Ho has posted comments on this change.

Change subject: IMPALA-4180: Synchronize accesses to RuntimeState::reader_contexts_
......................................................................


Patch Set 2:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/4558/2/be/src/runtime/runtime-state.cc
File be/src/runtime/runtime-state.cc:

Line 302:   for (DiskIoRequestContext* context: reader_contexts_) {
> Nit clang-format puts a space before :, should do that or consistency.
Done


http://gerrit.cloudera.org:8080/#/c/4558/2/be/src/runtime/runtime-state.h
File be/src/runtime/runtime-state.h:

Line 165:   /// Takes ownership of a scan node's reader context and unregisters it when the
> "Takes ownership of the given reader context which may still hold used IO b
It's "automatic" from the perspective of the caller but I know what you mean.


Line 167:   /// TODO: Attach the reader context to the last row batch instead.
> Works for me. If the eventual direction is not clear, then let's remove the
Sure. Let's remove it for now but that sounds like an attractive option if unregistration
happens at the end of the lifetime of the IO buffer.


http://gerrit.cloudera.org:8080/#/c/4558/2/testdata/workloads/functional-query/queries/QueryTest/single-node-nlj.test
File testdata/workloads/functional-query/queries/QueryTest/single-node-nlj.test:

Line 215: ---- QUERY
> move this below the test for IMPALA-561 to cluster related tests together
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/4558
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I911fda526a99514b12f88a3e9fb5952ea4fe1973
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho <kwho@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Michael Ho <kwho@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message