Alex Behm has posted comments on this change.

Change subject: IMPALA-4180: Synchronize accesses to 

Patch Set 1:

File be/src/runtime/

Line 304:   }
File be/src/runtime/runtime-state.h:

Line 167:   void DeferIOContextUnregistration(DiskIoRequestContext* 

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 

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();

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).
File tests/query_test/

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
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I911fda526a99514b12f88a3e9fb5952ea4fe1973
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho <>
Gerrit-Reviewer: Alex Behm <>
Gerrit-HasComments: Yes

Reply via email to