Henry Robinson has posted comments on this change.

Change subject: IMPALA-3452: S3: Disable Impala staging for INSERTs via flag 
for speedup
......................................................................


Patch Set 3: Code-Review+1

(3 comments)

Looks good to me, pending Dan's comments and the following: You should add at 
least one test where you turn this option off for OVERWRITE and plain INSERT.

http://gerrit.cloudera.org:8080/#/c/2905/3/be/src/exec/hdfs-table-sink.cc
File be/src/exec/hdfs-table-sink.cc:

Line 295: filename << output_partition->tmp_hdfs_file_name_prefix
        :              << "." << output_partition->num_files
        :              << "." << output_partition->writer->file_extension();
mixing your metaphors a bit :) Convert this to Substitute() as well, and just 
assign directly to output_partition->current_file_name


Line 342: stringstream dest;
        :     dest << final_location;
dest is completely unnecessary now


http://gerrit.cloudera.org:8080/#/c/2905/3/common/thrift/ImpalaService.thrift
File common/thrift/ImpalaService.thrift:

Line 216:   // the files there.
Maybe add a TODO to fix this: could easily imagine a scheme where the 
coordinator kicks off the delete before starting the backends or something.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iff9620d41ba0d5fb1aa0c9f4abb48866fc2b0698
Gerrit-PatchSet: 3
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Sailesh Mukil <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: Henry Robinson <[email protected]>
Gerrit-Reviewer: Marcel Kornacker <[email protected]>
Gerrit-Reviewer: Mostafa Mokhtar <[email protected]>
Gerrit-Reviewer: Sailesh Mukil <[email protected]>
Gerrit-HasComments: Yes

Reply via email to