Alex Behm has posted comments on this change. Change subject: IMPALA-4180: Synchronize accesses to RuntimeState::reader_contexts_ ......................................................................
Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/4558/1/be/src/runtime/runtime-state.cc File be/src/runtime/runtime-state.cc: Line 304: } reader_contexts_.clear() http://gerrit.cloudera.org:8080/#/c/4558/1/be/src/runtime/runtime-state.h File be/src/runtime/runtime-state.h: Line 167: void DeferIOContextUnregistration(DiskIoRequestContext* reader_context); AcquireReaderContext()? I think the detail that these can only be reader contexts is important. "IOContext" seems more general, e.g. read/write contexts are not interesting here. The "Acquire" terminology is more similar to what we use elsewhere. We are transferring ownership of the context to the runtime state because the context lives until the end of the fragment. Might be good to add a brief TODO to indicate what the eventual solution should look like, e.g., attach the resources to the final row batch. Line 170: void UnregisterIOContexts(); UnregisterReaderContexts()? http://gerrit.cloudera.org:8080/#/c/4558/1/testdata/workloads/functional-query/queries/QueryTest/single-node-topn.test File testdata/workloads/functional-query/queries/QueryTest/single-node-topn.test: Line 4: # whose children are scan nodes. When scan nodes are closed concurrently, make I don't think the last sentence is necessary. It's likely to become stale pretty soon (at least I hope so). http://gerrit.cloudera.org:8080/#/c/4558/1/tests/query_test/test_queries.py File tests/query_test/test_queries.py: Line 131: def test_single_node_topn(self, vector): This new test seems kind of misleading. The cause of this are two scan nodes within the same fragment, the top-n nodes just happen to trigger the concurrent Close() pattern. This bug seems more in line with IMPALA-561 where you added a test to single-node-nlj.test. -- 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: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-HasComments: Yes
