Matthew Jacobs has posted comments on this change. Change subject: IMPALA-4134,IMPALA-3704: Kudu INSERT improvements ......................................................................
Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/4728/2/be/src/exec/kudu-table-sink.cc File be/src/exec/kudu-table-sink.cc: Line 36: DEFINE_int32(kudu_mutation_buffer_size, 100 * 1024 * 1024, "The size (bytes) of the " > Can the user disable this or do we always want to have it enabled? My understanding from the Kudu team is that we always want this enabled, and that 100mb is very reasonable for most use cases. Line 128: KUDU_RETURN_IF_ERROR(session_->SetFlushMode( > When would we hit these error conditions? Does the user have a recourse or I don't think these configuration operations should fail unless we called this while operations are pending, which would be a bug on our side since this is called in Open(). Line 133: // The user specifies the total amount of memory used for buffering, but we don't > Please add a comment to the class header describing how the buffering works Done Line 275: // then PK conflicts are not treated as errors. Only the first real error is returned > Why would not use our conventional RuntimeState::LogError() mechanism with Whoops, yes, good call. -- To view, visit http://gerrit.cloudera.org:8080/4728 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5542b9a061b01c543a139e8722560b1365f06595 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs <m...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Matthew Jacobs <m...@cloudera.com> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
