Matthew Jacobs has posted comments on this change. Change subject: WIP: Use AUTO_FLUSH_BACKGROUND for Kudu sink ......................................................................
Patch Set 1: (3 comments) I'm taking over the patch and will update it soon. http://gerrit.cloudera.org:8080/#/c/4670/1/be/src/exec/kudu-table-sink.cc File be/src/exec/kudu-table-sink.cc: PS1, Line 225: // TODO: instead of adding them here and subtracting errors in CheckForErrors, : // would it be better to _set_ the value to the total number of written rows : // minus the errors minus the current buffer size? > Todd: Can we even get the current buffer size? On second though, I don't think we care about updating this constantly. We can just update it once in FinalFlush with total num written - total num errors. PS1, Line 234: if (session_->CountPendingErrors() == 0) { : return Status::OK(); : } > nit: 1 line Done PS1, Line 281: // TODO: these counters don't make so much sense anymore. : SCOPED_TIMER(kudu_flush_timer_); : COUNTER_ADD(kudu_flush_counter_, 1); > can you remove them then? Done -- To view, visit http://gerrit.cloudera.org:8080/4670 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I665ce82d1caf64cf5bdf4bc0e15edbf9f1dc5ba0 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Mostafa Mokhtar <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
