Hadoop Common
  1. Hadoop Common
  2. HADOOP-6605

Add JAVA_HOME detection to hadoop-config

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.22.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      The commands that source hadoop-config.sh currently bail with an error if JAVA_HOME is not set. Let's detect JAVA_HOME (from a list of locations on various OS types) if JAVA_HOME is not already set by hadoop-env.sh or the environment. This way users don't have to manually configure it.

      1. HADOOP-6605.patch
        2 kB
        Chad Metcalf
      2. hadoop-6605-1.patch
        1 kB
        Eli Collins
      3. hadoop-6605-2.patch
        2 kB
        Eli Collins
      4. hadoop-6605-3.patch
        1 kB
        Eli Collins
      5. hadoop-6605-4.patch
        1 kB
        Eli Collins
      6. hadoop-6605-5.patch
        1 kB
        Eli Collins

        Issue Links

          Activity

          Hide
          Chad Metcalf added a comment -

          This patch only applies to 0.20.2 not trunk.

          Show
          Chad Metcalf added a comment - This patch only applies to 0.20.2 not trunk.
          Hide
          Allen Wittenauer added a comment -

          Why doesn't this try the java that is the path? Why doesn't this verify that $JAVA_HOME/bin/java actually works? Shouldn't it verify that it found JDK6 or better? Why is the error message RH-based specific? Heck, why have the "please download" message at all?

          Show
          Allen Wittenauer added a comment - Why doesn't this try the java that is the path? Why doesn't this verify that $JAVA_HOME/bin/java actually works? Shouldn't it verify that it found JDK6 or better? Why is the error message RH-based specific? Heck, why have the "please download" message at all?
          Hide
          Allen Wittenauer added a comment -

          Also, rather than make this Linux specific, adding /usr/jdk/instances/(version) to the detection and you'll get Solaris support. I also notice that the patch already has Mac OS X.

          It also just occurred to me that in the past there has been a very big reluctance to put OS specific fixes/changes in place. I'm not sure if the community will take such a change here.

          Show
          Allen Wittenauer added a comment - Also, rather than make this Linux specific, adding /usr/jdk/instances/(version) to the detection and you'll get Solaris support. I also notice that the patch already has Mac OS X. It also just occurred to me that in the past there has been a very big reluctance to put OS specific fixes/changes in place. I'm not sure if the community will take such a change here.
          Hide
          steve_l added a comment -

          Which linux distros are we talking about here?

          Show
          steve_l added a comment - Which linux distros are we talking about here?
          Hide
          steve_l added a comment -
          1. Unless there is a need for JDK stuff, all that's needed is java on the path. Maybe check for it being an odd java and warn if its an unsupported version (e.g. gcj), but even there, that gets annoying, like Log4J telling you off for not having a log4j config.
          2. Not happy with telling people to d/l from sun. It's oracle now, URLs have changed and anyway, well managed clusters don't have people downloading and installing by hand.

          Ant's startup script is probably the most thorough in find-and-bind, handles cygwin, IBM JVMs, etc
          http://svn.apache.org/viewvc/ant/core/trunk/src/script/ant?view=markup
          but: people are scared of touching that code, it's brittle code that needs to be tested everywhere before you can be sure you've made changes.

          Adding this stuff is taking on maintenance problems, forever. I'd consider it very carefully.

          Show
          steve_l added a comment - Unless there is a need for JDK stuff, all that's needed is java on the path. Maybe check for it being an odd java and warn if its an unsupported version (e.g. gcj), but even there, that gets annoying, like Log4J telling you off for not having a log4j config. Not happy with telling people to d/l from sun. It's oracle now, URLs have changed and anyway, well managed clusters don't have people downloading and installing by hand. Ant's startup script is probably the most thorough in find-and-bind, handles cygwin, IBM JVMs, etc http://svn.apache.org/viewvc/ant/core/trunk/src/script/ant?view=markup but: people are scared of touching that code, it's brittle code that needs to be tested everywhere before you can be sure you've made changes. Adding this stuff is taking on maintenance problems, forever. I'd consider it very carefully.
          Hide
          Eli Collins added a comment -

          Patch attached. Implements the behavior in the (updated) description.

          It detects Java for a number of different version of java (Linux platform default, Sun Java)on a couple different host types (Linux, Mac, tar install). It looks for Sun Java first since Hadoop currently requires Sun java. This list can be updated for other host types (eg add /usr/local for Solaris).

          Show
          Eli Collins added a comment - Patch attached. Implements the behavior in the (updated) description. It detects Java for a number of different version of java (Linux platform default, Sun Java)on a couple different host types (Linux, Mac, tar install). It looks for Sun Java first since Hadoop currently requires Sun java. This list can be updated for other host types (eg add /usr/local for Solaris).
          Hide
          Eli Collins added a comment -

          Forgot to mention testing, since the unit tests don't cover this. I verified with this change that:

          1. JAVA_HOME if set in conf/hadoop-env.sh takes precedence over the term env and this detection
          2. JAVA_HOME if set by the term env takes precedence over this detection
          3. JAVA_HOME is now detected if installed w/o being set explicitly, and errs out if none is detected
          Show
          Eli Collins added a comment - Forgot to mention testing, since the unit tests don't cover this. I verified with this change that: JAVA_HOME if set in conf/hadoop-env.sh takes precedence over the term env and this detection JAVA_HOME if set by the term env takes precedence over this detection JAVA_HOME is now detected if installed w/o being set explicitly, and errs out if none is detected
          Hide
          Eli Collins added a comment -

          Allen raised some good questions earlier...

          Why doesn't this try the java that is the path?

          I assume you mean that if java is in the path it could infer JAVA_HOME by looking at the parent of the containing directory. That's reasonable, think it would make sense to do this in addition to looking in standard locations (since we do care about the java we're looking for). If you feel strongly I can incorporate that into this change.

          Why doesn't this verify that $JAVA_HOME/bin/java actually works?

          I think it's OK to assume that the java installed in java home works.

          Shouldn't it verify that it found JDK6 or better?

          That would be a good improvement, it also needs to check for Sun-compatible Java since Hadoop currently depends on Sun-specific classes. Let's handle this in a separate jira.

          Why is the error message RH-based specific? Heck, why have the "please download" message at all?

          You're right, it shouldn't be. The latest patch is just a sligh modification to the current error message.

          Show
          Eli Collins added a comment - Allen raised some good questions earlier... Why doesn't this try the java that is the path? I assume you mean that if java is in the path it could infer JAVA_HOME by looking at the parent of the containing directory. That's reasonable, think it would make sense to do this in addition to looking in standard locations (since we do care about the java we're looking for). If you feel strongly I can incorporate that into this change. Why doesn't this verify that $JAVA_HOME/bin/java actually works? I think it's OK to assume that the java installed in java home works. Shouldn't it verify that it found JDK6 or better? That would be a good improvement, it also needs to check for Sun-compatible Java since Hadoop currently depends on Sun-specific classes. Let's handle this in a separate jira. Why is the error message RH-based specific? Heck, why have the "please download" message at all? You're right, it shouldn't be. The latest patch is just a sligh modification to the current error message.
          Hide
          Allen Wittenauer added a comment -

          Why this shouldn't be in a one-time setup script rather than executed on every hadoop invocation? Are we going to bastardized all of the scripts to deal with all of the other OS inconsistencies?

          I'm definitely -1 at this point.

          Show
          Allen Wittenauer added a comment - Why this shouldn't be in a one-time setup script rather than executed on every hadoop invocation? Are we going to bastardized all of the scripts to deal with all of the other OS inconsistencies? I'm definitely -1 at this point.
          Hide
          Eli Collins added a comment -

          Why this shouldn't be in a one-time setup script rather than executed on every hadoop invocation?

          Because the java installation may change between invocations, eg you may eg install java between invocations of the hadoop command, or you may remove one java installation and then install a new one and you don't want the old one (which is no longer present) cached anywhere. Eg

          1. exec hadoop # fails due to lack of java
          2. yum install java-xyz
          3. exec hadoop # now picks up java

          This detection only kicks in if JAVA_HOME is not explicitly set elsewhere, if you want to always use a particular version of java, just set JAVA_HOME.

          Any remaining technical objection?

          Show
          Eli Collins added a comment - Why this shouldn't be in a one-time setup script rather than executed on every hadoop invocation? Because the java installation may change between invocations, eg you may eg install java between invocations of the hadoop command, or you may remove one java installation and then install a new one and you don't want the old one (which is no longer present) cached anywhere. Eg exec hadoop # fails due to lack of java yum install java-xyz exec hadoop # now picks up java This detection only kicks in if JAVA_HOME is not explicitly set elsewhere, if you want to always use a particular version of java, just set JAVA_HOME. Any remaining technical objection?
          Hide
          Daryn Sharp added a comment -

          -1: Only in that I don't like the logic as it would apply to OS X, but I do really like the concept. Minor point is /Library/Java/Home was deprecated in 10.5 in favor of /usr/libexec/java_home. Ignoring that, if for some reason I had a copy of java in any of the myriad of directories listed prior to /Library/Java/Home, then this patch will pick that (wrong) version of java, instead of the one I set in "Java Preferences".

          Show
          Daryn Sharp added a comment - -1: Only in that I don't like the logic as it would apply to OS X, but I do really like the concept. Minor point is /Library/Java/Home was deprecated in 10.5 in favor of /usr/libexec/java_home. Ignoring that, if for some reason I had a copy of java in any of the myriad of directories listed prior to /Library/Java/Home, then this patch will pick that (wrong) version of java, instead of the one I set in "Java Preferences".
          Hide
          Eli Collins added a comment -

          Thanks for the feedback Daryn. It looks like /usr/libexec/java_home is the right method for newer Mac systems as the paths it returns is not very generic (in the way the others are), eg has particular dot version numbers in the name. How about, if /usr/libexec/java_home is present, the path it returns is put in the list before /Library/Java/Home? This should work for both older and newer versions of OS X. I could remove /Library/Java/Home from the current version of the patch and we could do that as a change that applies atop this one (I don't have a Mac) or I could make this change as part of this patch and you could try it for me?

          (As an aside, I'm surprised -1s are coming out on rev 1 of a patch, we normally we go back and forth on the review before veto'ing a patch, ie if there are technical objections that are not addressed in subsequent versions. I will try to address any technical objections raised).

          Show
          Eli Collins added a comment - Thanks for the feedback Daryn. It looks like /usr/libexec/java_home is the right method for newer Mac systems as the paths it returns is not very generic (in the way the others are), eg has particular dot version numbers in the name. How about, if /usr/libexec/java_home is present, the path it returns is put in the list before /Library/Java/Home? This should work for both older and newer versions of OS X. I could remove /Library/Java/Home from the current version of the patch and we could do that as a change that applies atop this one (I don't have a Mac) or I could make this change as part of this patch and you could try it for me? (As an aside, I'm surprised -1s are coming out on rev 1 of a patch, we normally we go back and forth on the review before veto'ing a patch, ie if there are technical objections that are not addressed in subsequent versions. I will try to address any technical objections raised).
          Hide
          Allen Wittenauer added a comment -

          java_home is not a stable interface. It cannot be trusted to last, especially given the question marks that hang over Java on OS X.

          I'm -1 for a variety of reasons:

          a) we're getting into platform specific code without a good reason
          b) this could have significant performance ramifications as we add more and more paths to support more and more OSes
          c) there is a config option to specifically eliminate the need to do this

          Show
          Allen Wittenauer added a comment - java_home is not a stable interface. It cannot be trusted to last, especially given the question marks that hang over Java on OS X. I'm -1 for a variety of reasons: a) we're getting into platform specific code without a good reason b) this could have significant performance ramifications as we add more and more paths to support more and more OSes c) there is a config option to specifically eliminate the need to do this
          Hide
          Eli Collins added a comment -

          a) we're getting into platform specific code without a good reason

          The motivation is that users don't shouldn't have to manually set an environment variable that we can detect automatically. Multiple users have requested this. Why is this not a good reason? We make improvements both for new users (who want an easier out of the box experience) and experienced admins (who will manually configure HADOOP_HOME).

          b) this could have significant performance ramifications as we add more and more paths to support more and more OSes

          How so? The new code only executes if HADOOP_HOME is not already set, and the code just checks for the existence of bin/java in a set of directories, it bails as soon as it finds one. This is not an expensive operation.

          c) there is a config option to specifically eliminate the need to do this

          The existence of a configuration parameter does not mean we're not allowed to try to detect a reasonable default if it's not set. We also automatically set a default MAX_HEAP_SIZE and let users override it, this is not different.

          Show
          Eli Collins added a comment - a) we're getting into platform specific code without a good reason The motivation is that users don't shouldn't have to manually set an environment variable that we can detect automatically. Multiple users have requested this. Why is this not a good reason? We make improvements both for new users (who want an easier out of the box experience) and experienced admins (who will manually configure HADOOP_HOME). b) this could have significant performance ramifications as we add more and more paths to support more and more OSes How so? The new code only executes if HADOOP_HOME is not already set, and the code just checks for the existence of bin/java in a set of directories, it bails as soon as it finds one. This is not an expensive operation. c) there is a config option to specifically eliminate the need to do this The existence of a configuration parameter does not mean we're not allowed to try to detect a reasonable default if it's not set. We also automatically set a default MAX_HEAP_SIZE and let users override it, this is not different.
          Hide
          Daryn Sharp added a comment -

          (My apologies on the neg. I thought no number meant "I have general concerns but it's ok if someone else +1s", whereas -1 meant "I think there's something really wrong I want addressed" as in the path handling.)

          Personally, I don't think it's too messy if hadoop knows how to make a few reasonable guesses, based on conventions, for a few common OSes if JAVA_HOME isn't already set. The tiny performance impact is only taken by those that chose to not set JAVA_HOME.

          Something like the following could be used to restrict the search path for the appropriate os.

          case "$(uname -s)" in
            Darwin)
              candidates=($(/usr/libexec/java_home) /Library/Java/Home)
              ;;
            Windows)
              candidates=(...no idea...)
              ;;
            Linux)
              candidates=(/usr/java/default /usr/lib/jvm/default-java <maybe something "alternatives" based> ...)
              ...
              ;;
          esac
          
          Show
          Daryn Sharp added a comment - (My apologies on the neg. I thought no number meant "I have general concerns but it's ok if someone else +1s", whereas -1 meant "I think there's something really wrong I want addressed" as in the path handling.) Personally, I don't think it's too messy if hadoop knows how to make a few reasonable guesses, based on conventions, for a few common OSes if JAVA_HOME isn't already set. The tiny performance impact is only taken by those that chose to not set JAVA_HOME. Something like the following could be used to restrict the search path for the appropriate os. case "$(uname -s)" in Darwin) candidates=($(/usr/libexec/java_home) /Library/Java/Home) ;; Windows) candidates=(...no idea...) ;; Linux) candidates=(/usr/java/ default /usr/lib/jvm/ default -java <maybe something "alternatives" based> ...) ... ;; esac
          Hide
          Daryn Sharp added a comment -

          java_home is not a stable interface. It cannot be trusted to last, especially given the question marks that hang over Java on OS X.

          This is not true. Apple engineers tell devs on their java-dev list to use java_home. The following technote says to use java_home. The utility was added in part so future non-system provided jvms don't monkey with system controlled directories.

          http://developer.apple.com/library/mac/#qa/qa1170/_index.html

          Show
          Daryn Sharp added a comment - java_home is not a stable interface. It cannot be trusted to last, especially given the question marks that hang over Java on OS X. This is not true. Apple engineers tell devs on their java-dev list to use java_home. The following technote says to use java_home. The utility was added in part so future non-system provided jvms don't monkey with system controlled directories. http://developer.apple.com/library/mac/#qa/qa1170/_index.html
          Hide
          Allen Wittenauer added a comment -

          I'm sure that would remain true if Apple was still going to supply the JVM. Given that the responsibility has shifted to Oracle, all bets are off.

          Show
          Allen Wittenauer added a comment - I'm sure that would remain true if Apple was still going to supply the JVM. Given that the responsibility has shifted to Oracle, all bets are off.
          Hide
          Eli Collins added a comment -

          Updated patch attached. Incorporates Daryn's suggestion: builds the candidates array based on the OS type (Solaris, OSX and Linux).

          Show
          Eli Collins added a comment - Updated patch attached. Incorporates Daryn's suggestion: builds the candidates array based on the OS type (Solaris, OSX and Linux).
          Hide
          Daryn Sharp added a comment -

          Love it. Comments:

          SunOS - why not jdk1.6.* to get latest patch version, or jdk*.*? Using the last item in the glob expansion or search in reverse order should generally get the latest version.

          Linux - Same as SunOS. Also, why aren't /usr/java/default and /usr/lib/jvm/default-java checked first?

          Overall, maybe globs aren't a great idea since it may lead to a lot more paths being added which I think is Eli's concern. Would it make more sense to only support using the "standard" mechanisms of an OS if that OS provides one? In which case the functionality is to make hadoop a good citizen of that OS, and not to make pseudo-intelligent guesses?

          Show
          Daryn Sharp added a comment - Love it. Comments: SunOS - why not jdk1.6.* to get latest patch version, or jdk*.*? Using the last item in the glob expansion or search in reverse order should generally get the latest version. Linux - Same as SunOS. Also, why aren't /usr/java/default and /usr/lib/jvm/default-java checked first? Overall, maybe globs aren't a great idea since it may lead to a lot more paths being added which I think is Eli's concern. Would it make more sense to only support using the "standard" mechanisms of an OS if that OS provides one? In which case the functionality is to make hadoop a good citizen of that OS, and not to make pseudo-intelligent guesses?
          Hide
          Daryn Sharp added a comment -

          Correction: Eli's concern = Allen's concern.

          Show
          Daryn Sharp added a comment - Correction: Eli's concern = Allen's concern.
          Hide
          Allen Wittenauer added a comment -

          FYI: I'm not removing my -1 as long as this remains in the main hadoop bin code path.

          Show
          Allen Wittenauer added a comment - FYI: I'm not removing my -1 as long as this remains in the main hadoop bin code path.
          Hide
          Eli Collins added a comment -

          FYI: I'm not removing my -1 as long as this remains in the main hadoop bin code path.

          Allen - what is your technical objection? There is not a performance impact to hadoop-config.sh, and even if there were, hadoop-config.sh is not a measurable part of hadoop command execution.

          common1 (trunk)$ export JAVA_HOME=/home/eli/toolchain/jdk1.6.0_24-x64
          common1 (trunk)$ time for i in {1..10}; do . bin/hadoop-config.sh ; done
          
          real	0m0.817s
          user	0m0.500s
          sys	0m0.120s
          common1 (trunk)$ unset JAVA_HOME
          common1 (trunk)$ time for i in {1..10}; do . bin/hadoop-config.sh ; done
          
          real	0m0.806s
          user	0m0.550s
          sys	0m0.060s
          
          Show
          Eli Collins added a comment - FYI: I'm not removing my -1 as long as this remains in the main hadoop bin code path. Allen - what is your technical objection? There is not a performance impact to hadoop-config.sh, and even if there were, hadoop-config.sh is not a measurable part of hadoop command execution. common1 (trunk)$ export JAVA_HOME=/home/eli/toolchain/jdk1.6.0_24-x64 common1 (trunk)$ time for i in {1..10}; do . bin/hadoop-config.sh ; done real 0m0.817s user 0m0.500s sys 0m0.120s common1 (trunk)$ unset JAVA_HOME common1 (trunk)$ time for i in {1..10}; do . bin/hadoop-config.sh ; done real 0m0.806s user 0m0.550s sys 0m0.060s
          Hide
          Eli Collins added a comment -

          In the above the ". bin/hadoop-config.sh" should use "bash" so it tries to re-detect JAVA_HOME w/ each invocation, however the difference in run time is still tiny.

          Show
          Eli Collins added a comment - In the above the ". bin/hadoop-config.sh" should use "bash" so it tries to re-detect JAVA_HOME w/ each invocation, however the difference in run time is still tiny.
          Hide
          Eli Collins added a comment -

          Thanks for reviewing Daryn!

          SunOS - why not jdk1.6.* to get latest patch version, or jdk*.*? Using the last item in the glob expansion or search in reverse order should generally get the latest version.

          From googling it looked like "jdk1.6.0" was a stable path on Solaris, ie no glob necessary. Maybe someone who uses Solaris can verify, I'm fine punting Solaris to a future change as well.

          Linux - Same as SunOS. Also, why aren't /usr/java/default and /usr/lib/jvm/default-java checked first?

          The code looks for Sun Java 6 first because Hadoop depends on Sun Java 6, ie we don't necessarily want the system default Java.

          Overall, maybe globs aren't a great idea since it may lead to a lot more paths being added which I think is Allen's concern.

          These globs should not result in a lot more paths being added. Eg how many paths would you expect "/usr/java/jdk1.6*" to match on most systems? Probably none, a couple would be a lot right? Even if these globs matched 20 paths per above it shouldn't impact performance.

          Would it make more sense to only support using the "standard" mechanisms of an OS if that OS provides one? In which case the functionality is to make hadoop a good citizen of that OS, and not to make pseudo-intelligent guesses?

          There is no standard path across Linux distributions for Sun Java 6, these globs match Sun Java 6 on popular Linux distributions, that's how the list was generated.

          Show
          Eli Collins added a comment - Thanks for reviewing Daryn! SunOS - why not jdk1.6.* to get latest patch version, or jdk*.*? Using the last item in the glob expansion or search in reverse order should generally get the latest version. From googling it looked like "jdk1.6.0" was a stable path on Solaris, ie no glob necessary. Maybe someone who uses Solaris can verify, I'm fine punting Solaris to a future change as well. Linux - Same as SunOS. Also, why aren't /usr/java/default and /usr/lib/jvm/default-java checked first? The code looks for Sun Java 6 first because Hadoop depends on Sun Java 6, ie we don't necessarily want the system default Java. Overall, maybe globs aren't a great idea since it may lead to a lot more paths being added which I think is Allen's concern. These globs should not result in a lot more paths being added. Eg how many paths would you expect "/usr/java/jdk1.6*" to match on most systems? Probably none, a couple would be a lot right? Even if these globs matched 20 paths per above it shouldn't impact performance. Would it make more sense to only support using the "standard" mechanisms of an OS if that OS provides one? In which case the functionality is to make hadoop a good citizen of that OS, and not to make pseudo-intelligent guesses? There is no standard path across Linux distributions for Sun Java 6, these globs match Sun Java 6 on popular Linux distributions, that's how the list was generated.
          Hide
          Daryn Sharp added a comment -

          (btw, I previously attempted to imply my neg was withdrawn, but I just want to make it unambiguously clear. I really like the idea as evidenced by my jira dupped to this one)

          From googling it looked like "jdk1.6.0" was a stable path on Solaris, ie no glob necessary. Maybe someone who uses Solaris can verify, I'm fine punting Solaris to a future change as well.

          My only point/concern is that if a jdk1.6.1. or jdk1.6.2 is installed that this implementation would still default to 1.6.0. What if a 1.6.1, but not 1.6.0, is installed? Or if only jdk1.7.* is installed and perhaps hadoop works fine with either java 6 or 7? The detection is of little use to the user, however I don't know the Solaris conventions. Maybe they have a way to set a "default" java? Perhaps a symlink somewhere? I have no strong feelings, and punting would be fine, or maybe a Solaris user could chime in.

          These globs should not result in a lot more paths being added. Eg how many paths would you expect "/usr/java/jdk1.6*" to match on most systems? Probably none, a couple would be a lot right? Even if these globs matched 20 paths per above it shouldn't impact performance.

          Don't worry, I have no performance concerns. I fully agree a few stat calls is minuscule in the overall execution. My mild concern is the maintainability introduced by having to update paths for newer versions of java. However, being a stickler for consistency, I have a little more concern about how updating the paths introduces inconsistent behavior in how the jdk is selected. Then again, perhaps Linux folks are accustomed to inconsistency?

          bq. Linux - Same as SunOS. Also, why aren't /usr/java/default and /usr/lib/jvm/default-java checked first?

          The code looks for Sun Java 6 first because Hadoop depends on Sun Java 6, ie we don't necessarily want the system default Java.

          Only for the sake of discussion: If the goal is be a "good citizen" of the host os, then picking the system default is sufficient. This approach should eliminate the need to periodically update the script to pick the newer "preferred" java versions, and remove the inconsistency in how the java version is selected between hadoop releases. It would parallel the Mac/Darwin detection, ie. use the system's default method (if there is one), and if the user wants a different version then they must either change the system default or explicitly set JAVA_HOME.

          Show
          Daryn Sharp added a comment - (btw, I previously attempted to imply my neg was withdrawn, but I just want to make it unambiguously clear. I really like the idea as evidenced by my jira dupped to this one) From googling it looked like "jdk1.6.0" was a stable path on Solaris, ie no glob necessary. Maybe someone who uses Solaris can verify, I'm fine punting Solaris to a future change as well. My only point/concern is that if a jdk1.6.1. or jdk1.6.2 is installed that this implementation would still default to 1.6.0. What if a 1.6.1, but not 1.6.0, is installed? Or if only jdk1.7.* is installed and perhaps hadoop works fine with either java 6 or 7? The detection is of little use to the user, however I don't know the Solaris conventions. Maybe they have a way to set a "default" java? Perhaps a symlink somewhere? I have no strong feelings, and punting would be fine, or maybe a Solaris user could chime in. These globs should not result in a lot more paths being added. Eg how many paths would you expect "/usr/java/jdk1.6*" to match on most systems? Probably none, a couple would be a lot right? Even if these globs matched 20 paths per above it shouldn't impact performance. Don't worry, I have no performance concerns. I fully agree a few stat calls is minuscule in the overall execution. My mild concern is the maintainability introduced by having to update paths for newer versions of java. However, being a stickler for consistency, I have a little more concern about how updating the paths introduces inconsistent behavior in how the jdk is selected. Then again, perhaps Linux folks are accustomed to inconsistency? bq. Linux - Same as SunOS. Also, why aren't /usr/java/default and /usr/lib/jvm/default-java checked first? The code looks for Sun Java 6 first because Hadoop depends on Sun Java 6, ie we don't necessarily want the system default Java. Only for the sake of discussion: If the goal is be a "good citizen" of the host os, then picking the system default is sufficient. This approach should eliminate the need to periodically update the script to pick the newer "preferred" java versions, and remove the inconsistency in how the java version is selected between hadoop releases. It would parallel the Mac/Darwin detection, ie. use the system's default method (if there is one), and if the user wants a different version then they must either change the system default or explicitly set JAVA_HOME.
          Hide
          Allen Wittenauer added a comment -

          My technical objection is that there is a high likelihood of getting this wrong.

          For example, we should have a check to make sure that the java we find is actually 1.6.x. We need a check to make sure that the java we find actually works properly (i.e., I believe it was _18 or so that caused us issues). We have to make the determination if we want to allow heterogenous JREs. What if /usr/java/x isn't a Sun Java?

          The wildcard generates a whole spate of problems. Do we always pick the oldest version? Do we reverse the regex and grab the newest? No matter what we do there, this is going to cause major problems with those that want to do staged rollouts of new JREs.

          No, the "but you can always set it" is not an excuse for allowing surprising behavior.

          ...

          Again: This is way too much work to shove into the interactive hadoop commands. Until this moves out into a one-time setup, my binding -1 will remain.

          Show
          Allen Wittenauer added a comment - My technical objection is that there is a high likelihood of getting this wrong. For example, we should have a check to make sure that the java we find is actually 1.6.x. We need a check to make sure that the java we find actually works properly (i.e., I believe it was _18 or so that caused us issues). We have to make the determination if we want to allow heterogenous JREs. What if /usr/java/x isn't a Sun Java? The wildcard generates a whole spate of problems. Do we always pick the oldest version? Do we reverse the regex and grab the newest? No matter what we do there, this is going to cause major problems with those that want to do staged rollouts of new JREs. No, the "but you can always set it" is not an excuse for allowing surprising behavior. ... Again: This is way too much work to shove into the interactive hadoop commands. Until this moves out into a one-time setup, my binding -1 will remain.
          Hide
          Chris Douglas added a comment -

          This struck me as an obvious win, but I'm starting to see Allen's point. Incomplete detection may save some users a couple minutes, but it will cause weird bugs in other systems. The globbing/non-Sun JVM cases are especially salient. The benefit is clear to anyone who's set this variable dozens (if not hundreds) of times, but the potential cost is more subtle. As automatic behavior, this is trying to be too clever for a nominal benefit.

          That said, the MacOS tool seems designed to accomplish exactly this, and I'd have no problem if it were to show up in the script. If it stops being a reliable way of setting JAVA_HOME, we'll just fix it. If there are similar tools in Solaris/Linux (doesn't /etc/alternatives support similar functionality?), we can make use of them.

          Show
          Chris Douglas added a comment - This struck me as an obvious win, but I'm starting to see Allen's point. Incomplete detection may save some users a couple minutes, but it will cause weird bugs in other systems. The globbing/non-Sun JVM cases are especially salient. The benefit is clear to anyone who's set this variable dozens (if not hundreds) of times, but the potential cost is more subtle. As automatic behavior, this is trying to be too clever for a nominal benefit. That said, the MacOS tool seems designed to accomplish exactly this, and I'd have no problem if it were to show up in the script. If it stops being a reliable way of setting JAVA_HOME , we'll just fix it. If there are similar tools in Solaris/Linux (doesn't /etc/alternatives support similar functionality?), we can make use of them.
          Hide
          Bharath Mundlapudi added a comment -

          Most of the time, i do side-by-side jdk installations to see which jdk is better. Many times, default jdk location is not preferred, Esp. as Allen mentioned sometimes /usr/bin/java isn't Sun Java.

          IMO, having .hadooprc file might be useful thing. Again, this file should be part of the bundle. This is kind of creating virtual environment for hadoop installation. This will solve multiple version problem also.

          Sometimes, it is hard to know which jdk binary hadoop is using, part of the reason is one can set JAVA_HOME at multiple locations like shell, hadoop-env.sh etc Having consistent location for setting this also helpful. One has to do ps cmd to figure out which location java is running from.

          Also, having a helper script like checkhadoopenv.sh should let the user know its current virtual environment. This will be useful for debugging purpose.

          I think, we should have one consistent way to set environment for hadoop this avoids user confusion.

          Main point: set all the environment variables required for hadoop in .hadooprc file defaulting to certain values. User can edit this file depending on his/her case may be.

          Defaulting env can be a tricky thing if user didn't set in .hadooprc. If this happens, we should let user know that he/she should update .hadooprc file for the missing env variables.

          This file (.hadooprc) is user editable. We should document and make it the only place users can tinker their environment. This avoids most of the confusion. Do you agree?

          Show
          Bharath Mundlapudi added a comment - Most of the time, i do side-by-side jdk installations to see which jdk is better. Many times, default jdk location is not preferred, Esp. as Allen mentioned sometimes /usr/bin/java isn't Sun Java. IMO, having .hadooprc file might be useful thing. Again, this file should be part of the bundle. This is kind of creating virtual environment for hadoop installation. This will solve multiple version problem also. Sometimes, it is hard to know which jdk binary hadoop is using, part of the reason is one can set JAVA_HOME at multiple locations like shell, hadoop-env.sh etc Having consistent location for setting this also helpful. One has to do ps cmd to figure out which location java is running from. Also, having a helper script like checkhadoopenv.sh should let the user know its current virtual environment. This will be useful for debugging purpose. I think, we should have one consistent way to set environment for hadoop this avoids user confusion. Main point: set all the environment variables required for hadoop in .hadooprc file defaulting to certain values. User can edit this file depending on his/her case may be. Defaulting env can be a tricky thing if user didn't set in .hadooprc. If this happens, we should let user know that he/she should update .hadooprc file for the missing env variables. This file (.hadooprc) is user editable. We should document and make it the only place users can tinker their environment. This avoids most of the confusion. Do you agree?
          Hide
          Eli Collins added a comment -

          Thanks for the feedback everyone. Popping the stack, Hadoop requires the user set JAVA_HOME for two reasons:

          1. We want to add tools.jar to the classpath, and JAVA_HOME let's the user specify a base directory to look (other than the default java which may be from a JRE and therefore not have tools.jar). This is no longer an issue since HADOOP-7374 removed it.
          2. We want to respect JAVA_HOME even if there is already a java in the path. Ie users and admins can easily configure which java should be used with Hadoop that's different from the default system java. This makes sense given that Hadoop is picky. Therefore it makes sense to only auto-detect JAVA_HOME if it is not set (which all versions of the patch do) and we can determine a reasonable value.

          On OSX, they provide an API (java_home(1)) that does this (returns a path suitable for setting JAVA_HOME based on enabled/preferred JVM'S as set by Java Preferences). I think we agree it makes sense to use this.

          On Linux, there is no single API that works across distributions. Even though alternatives is widely available it works differently on different distriubtions (also, it indicates where the java binary lives, not where JAVA_HOME is, though you could determine that with readlink). There are well-known locations where JAVA_HOME is installed that you can check to reasonably detect it. This is the approach taken by the previous patch. I've provided data that shows that checking a set of directories does not measurably impact the execution time (therefore "too much work" sounds like a philosophical objection rather than a technical objection to me). I've found that globbing is not an issue in practice because the glob does not match more than one installation on a given system. This is because the JDK was resolved via a packaging dependency and the package updates itself rather than having multiple versions installed. People who manually install multiple JDKs typically set JAVA_HOME explicitly and therefore the detection is not used. There are no alternative proposals for autodetecting JAVA_HOME on Linux, and I'm not going to spend any more time on this part for now so I'm dropping this case from the patch.

          In any case (ha), there is consensus on the OSX approach so let's just go with this for now. We can easily implement cases for other OS types in the future if there's an approach that's acceptable. Patch attached.

          Show
          Eli Collins added a comment - Thanks for the feedback everyone. Popping the stack, Hadoop requires the user set JAVA_HOME for two reasons: We want to add tools.jar to the classpath, and JAVA_HOME let's the user specify a base directory to look (other than the default java which may be from a JRE and therefore not have tools.jar). This is no longer an issue since HADOOP-7374 removed it. We want to respect JAVA_HOME even if there is already a java in the path. Ie users and admins can easily configure which java should be used with Hadoop that's different from the default system java. This makes sense given that Hadoop is picky. Therefore it makes sense to only auto-detect JAVA_HOME if it is not set (which all versions of the patch do) and we can determine a reasonable value. On OSX, they provide an API (java_home(1)) that does this (returns a path suitable for setting JAVA_HOME based on enabled/preferred JVM'S as set by Java Preferences). I think we agree it makes sense to use this. On Linux, there is no single API that works across distributions. Even though alternatives is widely available it works differently on different distriubtions (also, it indicates where the java binary lives, not where JAVA_HOME is, though you could determine that with readlink). There are well-known locations where JAVA_HOME is installed that you can check to reasonably detect it. This is the approach taken by the previous patch. I've provided data that shows that checking a set of directories does not measurably impact the execution time (therefore "too much work" sounds like a philosophical objection rather than a technical objection to me). I've found that globbing is not an issue in practice because the glob does not match more than one installation on a given system. This is because the JDK was resolved via a packaging dependency and the package updates itself rather than having multiple versions installed. People who manually install multiple JDKs typically set JAVA_HOME explicitly and therefore the detection is not used. There are no alternative proposals for autodetecting JAVA_HOME on Linux, and I'm not going to spend any more time on this part for now so I'm dropping this case from the patch. In any case (ha), there is consensus on the OSX approach so let's just go with this for now. We can easily implement cases for other OS types in the future if there's an approach that's acceptable. Patch attached.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12482283/hadoop-6605-3.patch
          against trunk revision 1134861.

          +1 @author. The patch does not contain any @author tags.

          -1 tests included. 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 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 system test framework. The patch passed system test framework compile.

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/616//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/616//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/616//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12482283/hadoop-6605-3.patch against trunk revision 1134861. +1 @author. The patch does not contain any @author tags. -1 tests included. 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 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/616//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/616//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/616//console This message is automatically generated.
          Hide
          Daryn Sharp added a comment -

          +1 Beautiful! My only suggestion would be to consider use double brackets (ie. [[ ... ]]) so you don't have to add quotes around the variables. Double brackets do 1-pass expansion, as opposed to single brackets that expanded and re-parse which leads to problem with variables contains space or other metachars unless quotes are added.

          Show
          Daryn Sharp added a comment - +1 Beautiful! My only suggestion would be to consider use double brackets (ie. [[ ... ]]) so you don't have to add quotes around the variables. Double brackets do 1-pass expansion, as opposed to single brackets that expanded and re-parse which leads to problem with variables contains space or other metachars unless quotes are added.
          Hide
          Allen Wittenauer added a comment -

          I still think this is a bad idea, even if we are limiting to OS X.

          BTW, the patch should use -x, not -e.

          Show
          Allen Wittenauer added a comment - I still think this is a bad idea, even if we are limiting to OS X. BTW, the patch should use -x, not -e.
          Hide
          Eli Collins added a comment -

          Thanks for the feedback guys. Patch attached uses double brackets and -x.

          Allen, I think this patch addresses the specific concern you raised (searching too many directories, the Linux stuff). This patch does pretty minimal work (just calls out to java_home). It does still execute on each command, that's intentional and IMO OK because per above it's minimal work and otherwise Hadoop would not notice when the user changed their JAVA_HOME preference. Lemme know if you're -1 or -0.

          Show
          Eli Collins added a comment - Thanks for the feedback guys. Patch attached uses double brackets and -x. Allen, I think this patch addresses the specific concern you raised (searching too many directories, the Linux stuff). This patch does pretty minimal work (just calls out to java_home). It does still execute on each command, that's intentional and IMO OK because per above it's minimal work and otherwise Hadoop would not notice when the user changed their JAVA_HOME preference. Lemme know if you're -1 or -0.
          Hide
          Daryn Sharp added a comment -

          I'd like to succinctly reassert that it's unexpected behavior, on OS X, for a java app to not honor the user's preference. FWIW, I've never had to explicitly define JAVA_HOME for an app. For instance, Tomcat doesn't require the user to explicitly define JAVA_HOME to start the server.

          Show
          Daryn Sharp added a comment - I'd like to succinctly reassert that it's unexpected behavior, on OS X, for a java app to not honor the user's preference. FWIW, I've never had to explicitly define JAVA_HOME for an app. For instance, Tomcat doesn't require the user to explicitly define JAVA_HOME to start the server.
          Hide
          Allen Wittenauer added a comment -

          I'll change my vote to 0 for this particular change to add support for OS X. (I keep meaning to check Lion but keep forgetting. Too busy working on other Lion bugs...). This means we should change the case statement to a single if test.

          Show
          Allen Wittenauer added a comment - I'll change my vote to 0 for this particular change to add support for OS X. (I keep meaning to check Lion but keep forgetting. Too busy working on other Lion bugs...). This means we should change the case statement to a single if test.
          Hide
          Chris Douglas added a comment -

          +1 on the OS X detection

          Show
          Chris Douglas added a comment - +1 on the OS X detection
          Hide
          Daryn Sharp added a comment -

          Great, thanks Allen! From everything I've read, Java Prefs and /usr/libexec/java_home are still present in Lion. Apple is a major contributor to OpenJDK so I don't think Java will be fading soon. Jobs arm twisted Oracle into releasing and supporting future Java releases.

          Show
          Daryn Sharp added a comment - Great, thanks Allen! From everything I've read, Java Prefs and /usr/libexec/java_home are still present in Lion. Apple is a major contributor to OpenJDK so I don't think Java will be fading soon. Jobs arm twisted Oracle into releasing and supporting future Java releases.
          Hide
          Eli Collins added a comment -

          Thanks guys. Patch attached, same as the last, just changes the case stmt to an if stmt.

          Show
          Eli Collins added a comment - Thanks guys. Patch attached, same as the last, just changes the case stmt to an if stmt.
          Hide
          Daryn Sharp added a comment -

          +1 I'd recommend double square brackets again to eliminate need to quote, but not a huge deal. Can't wait to enjoy this patch!

          Show
          Daryn Sharp added a comment - +1 I'd recommend double square brackets again to eliminate need to quote, but not a huge deal. Can't wait to enjoy this patch!
          Hide
          Eli Collins added a comment -

          Thanks. I left the if as single bracket to be consistent w the rest of the file. Perhaps file a new jira to convert bin/* over to double brackets?

          Show
          Eli Collins added a comment - Thanks. I left the if as single bracket to be consistent w the rest of the file. Perhaps file a new jira to convert bin/* over to double brackets?
          Hide
          Eli Collins added a comment -

          I've committed this. Thanks guys!

          Show
          Eli Collins added a comment - I've committed this. Thanks guys!
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk-Commit #652 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/652/)
          HADOOP-6605. Add JAVA_HOME detection to hadoop-config. Contributed by Eli Collins

          eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1135333
          Files :

          • /hadoop/common/trunk/common/bin/hadoop-config.sh
          • /hadoop/common/trunk/common/CHANGES.txt
          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #652 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/652/ ) HADOOP-6605 . Add JAVA_HOME detection to hadoop-config. Contributed by Eli Collins eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1135333 Files : /hadoop/common/trunk/common/bin/hadoop-config.sh /hadoop/common/trunk/common/CHANGES.txt
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk-maven #17 (See https://builds.apache.org/job/Hadoop-Common-trunk-maven/17/)

          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk-maven #17 (See https://builds.apache.org/job/Hadoop-Common-trunk-maven/17/ )
          Hide
          hanasaki jiji added a comment -

          installed hadoop 1.0.0 .deb package on debian squeeze with openjdk6 already installed
          /etc/init.d/hadoop-datanode start results in "/usr/lib/jvm/java6-sun/bin/java not found

          this same error is reported with:
          1. just openjdk 6 headless
          2. full openjdk 6
          3. even if JAVA_HOME is exported before running the above command

          Show
          hanasaki jiji added a comment - installed hadoop 1.0.0 .deb package on debian squeeze with openjdk6 already installed /etc/init.d/hadoop-datanode start results in "/usr/lib/jvm/java6-sun/bin/java not found this same error is reported with: 1. just openjdk 6 headless 2. full openjdk 6 3. even if JAVA_HOME is exported before running the above command

            People

            • Assignee:
              Eli Collins
              Reporter:
              Chad Metcalf
            • Votes:
              0 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development