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

Change subject: IMPALA-13509: Copy rows directly to OutboundRowBatch during 
hash partitioning
......................................................................


Patch Set 10: Code-Review+1

(9 comments)

http://gerrit.cloudera.org:8080/#/c/21932/9//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21932/9//COMMIT_MSG@21
PS9, Line 21: PartitionRowCollector's OutboundRowBatch (collector_batch_), which
> Done
Ack


http://gerrit.cloudera.org:8080/#/c/21932/9/be/src/runtime/krpc-data-stream-sender-ir.cc
File be/src/runtime/krpc-data-stream-sender-ir.cc:

http://gerrit.cloudera.org:8080/#/c/21932/9/be/src/runtime/krpc-data-stream-sender-ir.cc@55
PS9, Line 55:   num_rows_++;
> Done
Ack


http://gerrit.cloudera.org:8080/#/c/21932/9/be/src/runtime/krpc-data-stream-sender.cc
File be/src/runtime/krpc-data-stream-sender.cc:

http://gerrit.cloudera.org:8080/#/c/21932/9/be/src/runtime/krpc-data-stream-sender.cc@194
PS9, Line 194:   // Shutdowns the channel and frees the row batch allocation. 
Any in-flight RPC will
> Thanks for spotting, this function is actually no longer used. Reused some
Ack


http://gerrit.cloudera.org:8080/#/c/21932/9/be/src/runtime/krpc-data-stream-sender.cc@602
PS9, Line 602:   DCHECK(rpc_in_flight_batch_ == nullptr);
> It counts all TransmitData attempts.
That symmetry sounds nice.


http://gerrit.cloudera.org:8080/#/c/21932/9/be/src/runtime/krpc-data-stream-sender.cc@1240
PS9, Line 1240:     }
> Done (the block was added for an extra timer which turned out to be ~useles
Ack


http://gerrit.cloudera.org:8080/#/c/21932/9/be/src/runtime/krpc-data-stream-sender.cc@1351
PS9, Line 1351: }
> fixed indentation
Ack


http://gerrit.cloudera.org:8080/#/c/21932/9/be/src/runtime/krpc-data-stream-sender.cc@1353
PS9, Line 1353: } // namespace impala
> This is only used with HASH a and KUDU partitioning, and in this case there
Ack


http://gerrit.cloudera.org:8080/#/c/21932/9/be/src/runtime/outbound-row-batch.h
File be/src/runtime/outbound-row-batch.h:

http://gerrit.cloudera.org:8080/#/c/21932/9/be/src/runtime/outbound-row-batch.h@77
PS9, Line 77:   Status PrepareForSend(int num_tuples_per_row, TrackedString* 
compression_scratch,
> Done
Ack


http://gerrit.cloudera.org:8080/#/c/21932/9/be/src/runtime/outbound-row-batch.inline.h
File be/src/runtime/outbound-row-batch.inline.h:

http://gerrit.cloudera.org:8080/#/c/21932/9/be/src/runtime/outbound-row-batch.inline.h@53
PS9, Line 53:       // resizing to the exact size, similarly to vector. It 
would be clearer to use a
> Done
Ack



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I81a16c2f0fcfc1f3adef7077b3932a29a0f15a8f
Gerrit-Change-Number: 21932
Gerrit-PatchSet: 10
Gerrit-Owner: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Comment-Date: Tue, 05 Nov 2024 21:51:11 +0000
Gerrit-HasComments: Yes

Reply via email to