Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/10275 )
Change subject: IMPALA-6949: Add the option to start the minicluster with EC enabled ...................................................................... Patch Set 3: Code-Review+2 (5 comments) For what you're doing right now, I think this is very close. A few nits below, but feel free to carry the +2 once you add the comments and one fail-fast that I asked for. Do any of the 3-node EC policies make any sense to test always? i.e., we're creating a new test axis here, but it's nice to test at least a little bit in the "regular" test axis. http://gerrit.cloudera.org:8080/#/c/10275/2/bin/impala-config.sh File bin/impala-config.sh: http://gerrit.cloudera.org:8080/#/c/10275/2/bin/impala-config.sh@451 PS2, Line 451: if [[ "${ERASURE_CODING}" = true ]]; then I think we should error out (or at least warn) if http://gerrit.cloudera.org:8080/#/c/10275/3/bin/impala-config.sh File bin/impala-config.sh: http://gerrit.cloudera.org:8080/#/c/10275/3/bin/impala-config.sh@450 PS3, Line 450: elif [ "${TARGET_FILESYSTEM}" == "hdfs" ]; then nit: we seem to use single = here (line below as well as line 434, etc.), so may as well keep doing it? http://gerrit.cloudera.org:8080/#/c/10275/3/bin/impala-config.sh@451 PS3, Line 451: if [[ "${ERASURE_CODING}" = true ]]; then I think we should error out or warn here if MINICLUSTER_PROFILE < 3. I.e., we know this doesn't make sense for Hadoop 2, and we should error out early. http://gerrit.cloudera.org:8080/#/c/10275/3/testdata/bin/setup-hdfs-env.sh File testdata/bin/setup-hdfs-env.sh: http://gerrit.cloudera.org:8080/#/c/10275/3/testdata/bin/setup-hdfs-env.sh@80 PS3, Line 80: HDFS_EC_PATH > This should be HDFS_ERASURECODE_PATH Please add: # This does not convert the existing files but affects new files created in this directory. That said--what's the point? Does flipping back and forth make any sense or are users just going to get the wrong thing, because they really need to redo data load, and the only normal way to do that is -format? http://gerrit.cloudera.org:8080/#/c/10275/2/testdata/cluster/node_templates/common/etc/hadoop/conf/hdfs-site.xml.tmpl File testdata/cluster/node_templates/common/etc/hadoop/conf/hdfs-site.xml.tmpl: http://gerrit.cloudera.org:8080/#/c/10275/2/testdata/cluster/node_templates/common/etc/hadoop/conf/hdfs-site.xml.tmpl@26 PS2, Line 26: <name>cloudera.erasure_coding.enabled</name> > This is needed because running the following command Add: <!-- The release of Hadoop we're depending on requires an explicit key to allow erasure coding. --> -- To view, visit http://gerrit.cloudera.org:8080/10275 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I397aed491354be21b0a8441ca671232dca25146c Gerrit-Change-Number: 10275 Gerrit-PatchSet: 3 Gerrit-Owner: Taras Bobrovytsky <tbobrovyt...@cloudera.com> Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com> Gerrit-Reviewer: Taras Bobrovytsky <tbobrovyt...@cloudera.com> Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com> Gerrit-Comment-Date: Thu, 03 May 2018 21:27:55 +0000 Gerrit-HasComments: Yes
