Uploaded image for project: 'Hadoop Common'
  1. Hadoop Common
  2. HADOOP-12116

Fix unrecommended syntax usages in hadoop/hdfs/yarn script for cygwin in branch-2

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.8.0, 3.0.0-alpha1
    • Component/s: scripts
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      We're using syntax like "if $cygwin; then" which may be errorounsly evaluated into true if cygwin is unset. We need to fix this in branch-2.

        Issue Links

          Activity

          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Mapreduce-trunk-Java8 #243 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/243/)
          HADOOP-12116. Fix unrecommended syntax usages in hadoop/hdfs/yarn script for cygwin in branch-2. Contributed by Li Lu. (cnauroth: rev b8e792cba257fdb0ca266ecb2f60f3f10c3a0c3b)

          • hadoop-common-project/hadoop-common/CHANGES.txt
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Mapreduce-trunk-Java8 #243 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/243/ ) HADOOP-12116 . Fix unrecommended syntax usages in hadoop/hdfs/yarn script for cygwin in branch-2. Contributed by Li Lu. (cnauroth: rev b8e792cba257fdb0ca266ecb2f60f3f10c3a0c3b) hadoop-common-project/hadoop-common/CHANGES.txt
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Mapreduce-trunk #2191 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2191/)
          HADOOP-12116. Fix unrecommended syntax usages in hadoop/hdfs/yarn script for cygwin in branch-2. Contributed by Li Lu. (cnauroth: rev b8e792cba257fdb0ca266ecb2f60f3f10c3a0c3b)

          • hadoop-common-project/hadoop-common/CHANGES.txt
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk #2191 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2191/ ) HADOOP-12116 . Fix unrecommended syntax usages in hadoop/hdfs/yarn script for cygwin in branch-2. Contributed by Li Lu. (cnauroth: rev b8e792cba257fdb0ca266ecb2f60f3f10c3a0c3b) hadoop-common-project/hadoop-common/CHANGES.txt
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #233 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/233/)
          HADOOP-12116. Fix unrecommended syntax usages in hadoop/hdfs/yarn script for cygwin in branch-2. Contributed by Li Lu. (cnauroth: rev b8e792cba257fdb0ca266ecb2f60f3f10c3a0c3b)

          • hadoop-common-project/hadoop-common/CHANGES.txt
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #233 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/233/ ) HADOOP-12116 . Fix unrecommended syntax usages in hadoop/hdfs/yarn script for cygwin in branch-2. Contributed by Li Lu. (cnauroth: rev b8e792cba257fdb0ca266ecb2f60f3f10c3a0c3b) hadoop-common-project/hadoop-common/CHANGES.txt
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Hdfs-trunk #2172 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2172/)
          HADOOP-12116. Fix unrecommended syntax usages in hadoop/hdfs/yarn script for cygwin in branch-2. Contributed by Li Lu. (cnauroth: rev b8e792cba257fdb0ca266ecb2f60f3f10c3a0c3b)

          • hadoop-common-project/hadoop-common/CHANGES.txt
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk #2172 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2172/ ) HADOOP-12116 . Fix unrecommended syntax usages in hadoop/hdfs/yarn script for cygwin in branch-2. Contributed by Li Lu. (cnauroth: rev b8e792cba257fdb0ca266ecb2f60f3f10c3a0c3b) hadoop-common-project/hadoop-common/CHANGES.txt
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Yarn-trunk-Java8 #245 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/245/)
          HADOOP-12116. Fix unrecommended syntax usages in hadoop/hdfs/yarn script for cygwin in branch-2. Contributed by Li Lu. (cnauroth: rev b8e792cba257fdb0ca266ecb2f60f3f10c3a0c3b)

          • hadoop-common-project/hadoop-common/CHANGES.txt
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Yarn-trunk-Java8 #245 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/245/ ) HADOOP-12116 . Fix unrecommended syntax usages in hadoop/hdfs/yarn script for cygwin in branch-2. Contributed by Li Lu. (cnauroth: rev b8e792cba257fdb0ca266ecb2f60f3f10c3a0c3b) hadoop-common-project/hadoop-common/CHANGES.txt
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Yarn-trunk #975 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/975/)
          HADOOP-12116. Fix unrecommended syntax usages in hadoop/hdfs/yarn script for cygwin in branch-2. Contributed by Li Lu. (cnauroth: rev b8e792cba257fdb0ca266ecb2f60f3f10c3a0c3b)

          • hadoop-common-project/hadoop-common/CHANGES.txt
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk #975 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/975/ ) HADOOP-12116 . Fix unrecommended syntax usages in hadoop/hdfs/yarn script for cygwin in branch-2. Contributed by Li Lu. (cnauroth: rev b8e792cba257fdb0ca266ecb2f60f3f10c3a0c3b) hadoop-common-project/hadoop-common/CHANGES.txt
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-trunk-Commit #8102 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8102/)
          HADOOP-12116. Fix unrecommended syntax usages in hadoop/hdfs/yarn script for cygwin in branch-2. Contributed by Li Lu. (cnauroth: rev b8e792cba257fdb0ca266ecb2f60f3f10c3a0c3b)

          • hadoop-common-project/hadoop-common/CHANGES.txt
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #8102 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8102/ ) HADOOP-12116 . Fix unrecommended syntax usages in hadoop/hdfs/yarn script for cygwin in branch-2. Contributed by Li Lu. (cnauroth: rev b8e792cba257fdb0ca266ecb2f60f3f10c3a0c3b) hadoop-common-project/hadoop-common/CHANGES.txt
          Hide
          cnauroth Chris Nauroth added a comment -

          +1 for the patch. I also tested this on Windows to confirm that the Cygwin support still works for all of the main entry points. I committed it to branch-2.

          Just to reiterate Allen's earlier comment, there really is no general guarantee of compatibility if multiple versions of the script files get mixed arbitrarily in a single invocation. It's fine to do a little defensive coding around this flag check, but it's just one example of the kind of thing that can go wrong.

          Thank you for the patch, Li.

          Show
          cnauroth Chris Nauroth added a comment - +1 for the patch. I also tested this on Windows to confirm that the Cygwin support still works for all of the main entry points. I committed it to branch-2. Just to reiterate Allen's earlier comment, there really is no general guarantee of compatibility if multiple versions of the script files get mixed arbitrarily in a single invocation. It's fine to do a little defensive coding around this flag check, but it's just one example of the kind of thing that can go wrong. Thank you for the patch, Li.
          Hide
          gtCarrera9 Li Lu added a comment -

          Hi Allen Wittenauer, I agree with you that in the use case, mixing the new script with old home dir may be problematic. We're fixing them right now. Meanwhile, the problem exposed here is valid since we need to conservatively treat unset $cygwin as false and not triggering the branch?

          Show
          gtCarrera9 Li Lu added a comment - Hi Allen Wittenauer , I agree with you that in the use case, mixing the new script with old home dir may be problematic. We're fixing them right now. Meanwhile, the problem exposed here is valid since we need to conservatively treat unset $cygwin as false and not triggering the branch?
          Hide
          hadoopqa Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          0 pre-patch 14m 34s Pre-patch branch-2 compilation is healthy.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 tests included 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
          +1 javac 5m 48s There were no new javac warning messages.
          +1 javadoc 9m 42s There were no new javadoc warning messages.
          +1 release audit 0m 23s The applied patch does not increase the total number of release audit warnings.
          +1 shellcheck 0m 4s There were no new shellcheck (v0.3.3) issues.
          +1 whitespace 0m 0s The patch has no lines that end in whitespace.
          +1 install 1m 21s mvn install still works.
          +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse.
          +1 common tests 21m 40s Tests passed in hadoop-common.
          +1 hdfs tests 159m 54s Tests passed in hadoop-hdfs.
              214m 5s  



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12741660/HADOOP-12116-branch-2.001.patch
          Optional Tests javadoc javac unit shellcheck
          git revision branch-2 / 1392d1a
          hadoop-common test log https://builds.apache.org/job/PreCommit-HADOOP-Build/7031/artifact/patchprocess/testrun_hadoop-common.txt
          hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HADOOP-Build/7031/artifact/patchprocess/testrun_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/7031/testReport/
          Java 1.7.0_55
          uname Linux asf906.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/7031/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 14m 34s Pre-patch branch-2 compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. -1 tests included 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac 5m 48s There were no new javac warning messages. +1 javadoc 9m 42s There were no new javadoc warning messages. +1 release audit 0m 23s The applied patch does not increase the total number of release audit warnings. +1 shellcheck 0m 4s There were no new shellcheck (v0.3.3) issues. +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 21s mvn install still works. +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse. +1 common tests 21m 40s Tests passed in hadoop-common. +1 hdfs tests 159m 54s Tests passed in hadoop-hdfs.     214m 5s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12741660/HADOOP-12116-branch-2.001.patch Optional Tests javadoc javac unit shellcheck git revision branch-2 / 1392d1a hadoop-common test log https://builds.apache.org/job/PreCommit-HADOOP-Build/7031/artifact/patchprocess/testrun_hadoop-common.txt hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HADOOP-Build/7031/artifact/patchprocess/testrun_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/7031/testReport/ Java 1.7.0_55 uname Linux asf906.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/7031/console This message was automatically generated.
          Hide
          aw Allen Wittenauer added a comment -

          if HADOOP_HOME is set to the old version of Hadoop but the current hdfs script is from the new version

          Why are you ending up in this, IMO, completely broken state? In other words, why are you running the new script on top of the old dir layout? There's no guarantees that will work and it will definitely break if you do this from 2.x->3.x.

          Show
          aw Allen Wittenauer added a comment - if HADOOP_HOME is set to the old version of Hadoop but the current hdfs script is from the new version Why are you ending up in this, IMO, completely broken state? In other words, why are you running the new script on top of the old dir layout? There's no guarantees that will work and it will definitely break if you do this from 2.x->3.x.
          Hide
          gtCarrera9 Li Lu added a comment -

          Let me elaborate a little bit more on this issue. We discovered this problem when testing rolling upgrades. When running a hadoop streaming job which runs "hdfs dfs -stat", if HADOOP_HOME is set to the old version of Hadoop but the current hdfs script is from the new version, we're losing all CLASSPATH and cannot find any classes. The reason for this is we did not set $cygwin in old scripts, so $cygwin remains unset when the new script evaluates "if $cygwin; then". Therefore, we'll set cygwin classpaths for all cases which will set classpath to empty (since we do not have cygwin).

          Given this fact, Vinod Kumar Vavilapalli, do you agree that we need to include this fix in 2.7.1? Thanks!

          Show
          gtCarrera9 Li Lu added a comment - Let me elaborate a little bit more on this issue. We discovered this problem when testing rolling upgrades. When running a hadoop streaming job which runs "hdfs dfs -stat", if HADOOP_HOME is set to the old version of Hadoop but the current hdfs script is from the new version, we're losing all CLASSPATH and cannot find any classes. The reason for this is we did not set $cygwin in old scripts, so $cygwin remains unset when the new script evaluates "if $cygwin; then". Therefore, we'll set cygwin classpaths for all cases which will set classpath to empty (since we do not have cygwin). Given this fact, Vinod Kumar Vavilapalli , do you agree that we need to include this fix in 2.7.1? Thanks!
          Hide
          jingzhao Jing Zhao added a comment -

          +1 pending Jenkins.

          Show
          jingzhao Jing Zhao added a comment - +1 pending Jenkins.
          Hide
          gtCarrera9 Li Lu added a comment -

          In this patch I enforced $cygwin to be true to trigger cygwin related env settings. In this way if $cygwin is unset we will not trigger those branches.

          Show
          gtCarrera9 Li Lu added a comment - In this patch I enforced $cygwin to be true to trigger cygwin related env settings. In this way if $cygwin is unset we will not trigger those branches.

            People

            • Assignee:
              gtCarrera9 Li Lu
              Reporter:
              gtCarrera9 Li Lu
            • Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development