Tim Armstrong has posted comments on this change. Change subject: IMPALA-4023: don't attach buffered tuple streams to batches ......................................................................
Patch Set 8: Not really - flush_resources without need_to_return implies 2a, whereas flush_resources with need_to_return implies 2b. Without an additional flag how does the operator know whether the batch is referencing non-attached resources and whether it is dealing with 2a or 2b? In case 2a blocking operators should acquire ownership if it hasn't already acquired it (the wrinkle is that there's no way to acquire a Block now so the accounting is wrong, but that will be addressed by the buffer pool). In case 2b, a copy-out is required. This behaviour was essentially already present, it was just implicit and some operators depended on it in subtle ways. Before this patch, attaching some resources (buffered tuple streams) forced a flush but attaching others did not force a flush (blocks, I/O buffers). Some of the operators relied on this implicit behaviour. E.g. the hash join node assumes that attaching a buffered tuple stream would mean that its reservations get freed up (this currently is true unless its feeding into the build of a NLJ). -- To view, visit http://gerrit.cloudera.org:8080/4448 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6471422f86ce71e4c6ab277a276000051bc2e8ff Gerrit-PatchSet: 8 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: No
