Dan Hecht has posted comments on this change. Change subject: IMPALA-3452: S3: Disable Impala staging for INSERTs via flag for speedup ......................................................................
Patch Set 4: (3 comments) The code change looks fine, but can you add a test for this? Maybe we can rerun some of the insert tests with the query option enabled when on S3? http://gerrit.cloudera.org:8080/#/c/2905/4/be/src/exec/hdfs-table-sink.h File be/src/exec/hdfs-table-sink.h: Line 214: This is set if we want to Returns TRUE if the staging step should be skipped for this partition. http://gerrit.cloudera.org:8080/#/c/2905/4/be/src/runtime/coordinator.cc File be/src/runtime/coordinator.cc: Line 865: } else if (!is_s3_path || !query_ctx_.request.query_options.s3_skip_insert_staging) { > Added this to the patch. This skips making unnecessary calls to S3 to check Is this even needed when the option is not set? i.e. does it do anything useful on S3 in either case? Line 866: we set the S3_SKIP_INSERT_STAGING query option if the ... query option is set (I.e. we don't do the setting of this option, the user does). -- 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: 4 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
