Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9344 )

Change subject: IMPALA-5518: Allocate KrpcDataStreamRecvr RowBatch tuples from 
BufferPool
......................................................................


Patch Set 8:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9344/6/be/src/runtime/row-batch.h
File be/src/runtime/row-batch.h:

http://gerrit.cloudera.org:8080/#/c/9344/6/be/src/runtime/row-batch.h@411
PS6, Line 411:   /// Creates an empty row batch based on the serialized row 
batch header. Called from
> Seems weird to set capacity_ at this point since it's not where the buffer
Fixed in PS8. Deferred setting capacity_ until the buffer is allocated.


http://gerrit.cloudera.org:8080/#/c/9344/7/be/src/runtime/row-batch.cc
File be/src/runtime/row-batch.cc:

http://gerrit.cloudera.org:8080/#/c/9344/7/be/src/runtime/row-batch.cc@206
PS7, Line 206:   row_batch->num_rows_ = header.num_rows();
> We're expecting that capacity_ == header.num_rows() here, right? Can we add
Good point. Actually, I took Dan's suggestion and deferred setting capacity_ 
until the buffer is allocated.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If4b1a45f68b9df0d3b539511e15aff15700246f2
Gerrit-Change-Number: 9344
Gerrit-PatchSet: 8
Gerrit-Owner: Michael Ho <k...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com>
Gerrit-Reviewer: Michael Ho <k...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Comment-Date: Tue, 20 Feb 2018 00:19:31 +0000
Gerrit-HasComments: Yes

Reply via email to