impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tim Armstrong (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5085: large rows in BufferedTupleStreamV2
Date Mon, 05 Jun 2017 14:34:25 GMT
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5085: large rows in BufferedTupleStreamV2
......................................................................


Patch Set 7:

(2 comments)

I ended up reworking this a lot so that it does lazy page allocation and explicitly saves
read and write reservations when they're not being used to hold a buffer in memory.

http://gerrit.cloudera.org:8080/#/c/6638/5/be/src/runtime/buffered-tuple-stream-v2.cc
File be/src/runtime/buffered-tuple-stream-v2.cc:

Line 340:     return has_write_iterator && num_pages == 0;
> Doesn't the large page case already break that? i.e. how do we ensure the r
Yeah that's fair - the client definitely needs to be involved here. I'm assuming clients would
handle failure to append a large page to an unpinned stream in a similar way to handling failure
to append to a pinned stream - by freeing up memory by unpinning a pinned stream.

The handling of large pages in this patch makes it easier on the client because the large
page in an unpinned stream doesn't hold onto excess reservation.


PS5, Line 380: ONSISTENCY();
> Yeah that's true. It would make sense if appending a larger row was "best e
I removed this increase logic from the read page, agree that it doesn't make sense on the
read path. 

The write path handles both pinned and unpinned cases so it makes a bit more sense to try
to increase there.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2861c58efa7bc1aeaa5b7e2f043c97cb3985c8f5
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Jim Apple <jbapple-impala@apache.org>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message