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 <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
