Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.8.0
    • Fix Version/s: 0.8.0
    • Component/s: build
    • Labels:
      None

      Description

      Brock Noland filed a request for adding JDK7 to Bigtop toolchain so they can use it in their Hive pre-commit/integration test jobs. This JIRA will track that effort.

      1. BIGTOP-1218.1.patch
        3 kB
        Mark Grover
      2. BIGTOP-1218.2.patch
        3 kB
        Mark Grover
      3. BIGTOP-1218.3.patch
        3 kB
        Mark Grover

        Activity

        Hide
        mgrover Mark Grover added a comment -

        My goal here is to keep the default as JDK6 so everything still continues to build on JDK6. When the time is ripe, projects can one-by-one (or all together) switch to using JDK7. We are setting JDK7_HOME variable in Jenkins scripts so it can be leveraged by anyone who wants to use JDK7.

        Show
        mgrover Mark Grover added a comment - My goal here is to keep the default as JDK6 so everything still continues to build on JDK6. When the time is ripe, projects can one-by-one (or all together) switch to using JDK7. We are setting JDK7_HOME variable in Jenkins scripts so it can be leveraged by anyone who wants to use JDK7.
        Hide
        mgrover Mark Grover added a comment -

        And, FWIW, I had a tough time getting Oracle's JDK6 rpm to be installed side-by-side with Oracle's JDK6 rpm on RHEL/Centos, so I reverted to a tarball based install on RHEL/Centos and a regular package install in debian land (since JDK6 and JDK7 packages are compatible there). The tarball installation of JDK7 in rpm land is under /usr/lib/jdk1.7.0_45 currently in the patch I am about to post momentarily. If you have better ideas on where this should go, I would love to hear from you. The standard location /usr/java/jdk1.7.0_45 doesn't work because that confuses JDK package installs so they don't end up creating the latest and default symlinks like they usually do. So, at this point, I am open to installing JDK7 on rpms anywhere except under /usr/java. Do let me know if you object to /usr/lib/jdk1.7.0_45 though.

        Show
        mgrover Mark Grover added a comment - And, FWIW, I had a tough time getting Oracle's JDK6 rpm to be installed side-by-side with Oracle's JDK6 rpm on RHEL/Centos, so I reverted to a tarball based install on RHEL/Centos and a regular package install in debian land (since JDK6 and JDK7 packages are compatible there). The tarball installation of JDK7 in rpm land is under /usr/lib/jdk1.7.0_45 currently in the patch I am about to post momentarily. If you have better ideas on where this should go, I would love to hear from you. The standard location /usr/java/jdk1.7.0_45 doesn't work because that confuses JDK package installs so they don't end up creating the latest and default symlinks like they usually do. So, at this point, I am open to installing JDK7 on rpms anywhere except under /usr/java. Do let me know if you object to /usr/lib/jdk1.7.0_45 though.
        Hide
        mgrover Mark Grover added a comment -

        Review at https://reviews.apache.org/r/18389/

        And, also, Roman Shaposhnik since you have a ton more experience with this toolchain than I do, I'd really appreciate your input. Thank you in advance!

        Show
        mgrover Mark Grover added a comment - Review at https://reviews.apache.org/r/18389/ And, also, Roman Shaposhnik since you have a ton more experience with this toolchain than I do, I'd really appreciate your input. Thank you in advance!
        Hide
        cos Konstantin Boudnik added a comment -
        • split jdk manifest from jdk6 and jdk7 in two: not everyone wants to have jdk7 installed by default.
        • Let's use a permanently named link pointing to JAVA7_HOME instead of +export JAVA7_HOME=/usr/lib/jdk1.7.0_45. With the current approach someone will have to change this every single time when the JDK version is getting updated. Do something like /usr/lib/java7-latest instead

        And, FWIW, I had a tough time getting Oracle's JDK6 rpm to be installed side-by-side

        There's an easy way of getting binary distribution of JDK from Oracle website which doesn't require any complications with repos, etc. Check this post

        The rest seems ok. Thanks

        Show
        cos Konstantin Boudnik added a comment - split jdk manifest from jdk6 and jdk7 in two: not everyone wants to have jdk7 installed by default. Let's use a permanently named link pointing to JAVA7_HOME instead of +export JAVA7_HOME=/usr/lib/jdk1.7.0_45 . With the current approach someone will have to change this every single time when the JDK version is getting updated. Do something like /usr/lib/java7-latest instead And, FWIW, I had a tough time getting Oracle's JDK6 rpm to be installed side-by-side There's an easy way of getting binary distribution of JDK from Oracle website which doesn't require any complications with repos, etc. Check this post The rest seems ok. Thanks
        Hide
        mgrover Mark Grover added a comment -

        Thanks Cos, appreciate you taking the time to review this!

        1.

        split jdk manifest from jdk6 and jdk7 in two: not everyone wants to have jdk7 installed by default.


        I am 50/50 on this. The benefit of separating the two is that folks who don't want JDK7 wouldn't get it. The ill-effect is that our build slaves will start to look different. Some slaves would have JDK7 on it, some wouldn't. I am not a sysadmin but if consistency amongst slaves (atleast those who have the same OS) is desirable (and I would think it is), it would be good to have JDK7 installed on all. I would re-iterate here that installing JDK7 the way suggested in the patch shouldn't modify any of the existing workflows. Everything runs JDK6 and continues to do so, the goal is for JDK7 to sit there on the side and be available for anyone to use if they so desire.

        Cos, if you have any more thoughts on this, I'd love to hear them and if anyone else would like to share thoughts on this specifically, that'd be much appreciated!

        2.

        Let's use a permanently named link pointing to JAVA7_HOME instead of +export JAVA7_HOME=/usr/lib/jdk1.7.0_45. With the current approach someone will have to change this every single time when the JDK version is getting updated. Do something like /usr/lib/java7-latest instead
        And, FWIW, I had a tough time getting Oracle's JDK6 rpm to be installed side-by-side


        Will do.

        3.

        There's an easy way of getting binary distribution of JDK from Oracle website which doesn't require any complications with repos, etc. Check this post


        I was following the same method that we use for JDK6. However, this seems like a reasonable suggestion, I will create a separate follow-up JIRA to consider your suggestion for both JDK6 and JDK7.

        Show
        mgrover Mark Grover added a comment - Thanks Cos, appreciate you taking the time to review this! 1. split jdk manifest from jdk6 and jdk7 in two: not everyone wants to have jdk7 installed by default. I am 50/50 on this. The benefit of separating the two is that folks who don't want JDK7 wouldn't get it. The ill-effect is that our build slaves will start to look different. Some slaves would have JDK7 on it, some wouldn't. I am not a sysadmin but if consistency amongst slaves (atleast those who have the same OS) is desirable (and I would think it is), it would be good to have JDK7 installed on all. I would re-iterate here that installing JDK7 the way suggested in the patch shouldn't modify any of the existing workflows. Everything runs JDK6 and continues to do so, the goal is for JDK7 to sit there on the side and be available for anyone to use if they so desire. Cos, if you have any more thoughts on this, I'd love to hear them and if anyone else would like to share thoughts on this specifically, that'd be much appreciated! 2. Let's use a permanently named link pointing to JAVA7_HOME instead of +export JAVA7_HOME=/usr/lib/jdk1.7.0_45. With the current approach someone will have to change this every single time when the JDK version is getting updated. Do something like /usr/lib/java7-latest instead And, FWIW, I had a tough time getting Oracle's JDK6 rpm to be installed side-by-side Will do. 3. There's an easy way of getting binary distribution of JDK from Oracle website which doesn't require any complications with repos, etc. Check this post I was following the same method that we use for JDK6. However, this seems like a reasonable suggestion, I will create a separate follow-up JIRA to consider your suggestion for both JDK6 and JDK7.
        Hide
        cos Konstantin Boudnik added a comment -

        The ill-effect is that our build slaves will start to look different. Some slaves would have JDK7 on it, some

        Actually, they won't: just add jdk and jdk7 manifests to installer.pp and everything would be fine by default

        Show
        cos Konstantin Boudnik added a comment - The ill-effect is that our build slaves will start to look different. Some slaves would have JDK7 on it, some Actually, they won't: just add jdk and jdk7 manifests to installer.pp and everything would be fine by default
        Hide
        mackrorysd Sean Mackrory added a comment -

        What if we split the manifests and install both by default? Then it's an easy change for anyone who doesn't want JDK 7.

        Show
        mackrorysd Sean Mackrory added a comment - What if we split the manifests and install both by default? Then it's an easy change for anyone who doesn't want JDK 7.
        Hide
        mgrover Mark Grover added a comment -

        OK, I can do that.

        Show
        mgrover Mark Grover added a comment - OK, I can do that.
        Hide
        mgrover Mark Grover added a comment -

        I am attaching a new patch. This takes care of Cos' excellent suggestion about creating a versionless symlink for JDK7.

        I do plan to do the re-factoring of the code to have a separate JDK7 manifest but as a follow-up JIRA. The refactoring wouldn't change what our build slaves look like after we have deployed toolchain on them, it will just make it easier for people who don't want JDK7 to disable installation of JDK7 by commenting out a single line. Therefore, it's ok to do refactoring in a follow-up JIRA in my opinion. Is that ok with you Konstantin Boudnik and Sean Mackrory?

        Show
        mgrover Mark Grover added a comment - I am attaching a new patch. This takes care of Cos' excellent suggestion about creating a versionless symlink for JDK7. I do plan to do the re-factoring of the code to have a separate JDK7 manifest but as a follow-up JIRA. The refactoring wouldn't change what our build slaves look like after we have deployed toolchain on them, it will just make it easier for people who don't want JDK7 to disable installation of JDK7 by commenting out a single line. Therefore, it's ok to do refactoring in a follow-up JIRA in my opinion. Is that ok with you Konstantin Boudnik and Sean Mackrory ?
        Hide
        mackrorysd Sean Mackrory added a comment -

        +1 works for me. Since we're going to have installer.pp do both anyway, we mainly just want to save someone the trouble of knowing which parts of the manifests are applicable only to a particular JDK, so I'm fine with doing that part in a follow-up JIRA and enabling the pre-commit tests sooner.

        Show
        mackrorysd Sean Mackrory added a comment - +1 works for me. Since we're going to have installer.pp do both anyway, we mainly just want to save someone the trouble of knowing which parts of the manifests are applicable only to a particular JDK, so I'm fine with doing that part in a follow-up JIRA and enabling the pre-commit tests sooner.
        Hide
        cos Konstantin Boudnik added a comment -

        Therefore, it's ok to do refactoring in a follow-up JIRA in my opinion

        This seems like a bit of inverse logic to me, Mark. First, we introduce the behavior they forces JDK7 installation; then we split it and say - ok, now you can have your old way of being able to just have JDK6. Splitting the package doesn't seems like a lot of work - perhaps we'd better do it at once? Unless, there's something else standing in the path.

        Show
        cos Konstantin Boudnik added a comment - Therefore, it's ok to do refactoring in a follow-up JIRA in my opinion This seems like a bit of inverse logic to me, Mark. First, we introduce the behavior they forces JDK7 installation; then we split it and say - ok, now you can have your old way of being able to just have JDK6. Splitting the package doesn't seems like a lot of work - perhaps we'd better do it at once? Unless, there's something else standing in the path.
        Hide
        mgrover Mark Grover added a comment -

        Hey Cos, maybe I misunderstood you, sorry. If you call installing JDK7 forcing, then sure we are forcing and the refactoring is not going to change that forcing because the default installer would be installing the JDK7 by default, both before and after the refactoring.

        As far as what's standing in the path, I need to do some reading on how to express dependencies in puppet since JDK6 and JDK7 would have the same dependency of adding a repo, accepting the license, etc and make such actions idempotent, since they currently aren't. This page (http://docs.puppetlabs.com/learning/ordering.html) seems like a good start but I am swamped with some other stuff so wouldn't get a chance to do justice to it in the next few days.

        My rationale here is to be considerate to the folks that rely on us for their infrastructure and hive does so for their pre-commit jobs and as a member of the Hive community, I want to help out. My end goal is to unblock Brock Noland who's been waiting for a few weeks for this now. While I do think the refactoring is worthwhile and something I am planning to do real soon, I am personally in favor of committing this now and unblocking Brock and doing that refactoring in the next week or two.

        Show
        mgrover Mark Grover added a comment - Hey Cos, maybe I misunderstood you, sorry. If you call installing JDK7 forcing, then sure we are forcing and the refactoring is not going to change that forcing because the default installer would be installing the JDK7 by default, both before and after the refactoring. As far as what's standing in the path, I need to do some reading on how to express dependencies in puppet since JDK6 and JDK7 would have the same dependency of adding a repo, accepting the license, etc and make such actions idempotent, since they currently aren't. This page ( http://docs.puppetlabs.com/learning/ordering.html ) seems like a good start but I am swamped with some other stuff so wouldn't get a chance to do justice to it in the next few days. My rationale here is to be considerate to the folks that rely on us for their infrastructure and hive does so for their pre-commit jobs and as a member of the Hive community, I want to help out. My end goal is to unblock Brock Noland who's been waiting for a few weeks for this now. While I do think the refactoring is worthwhile and something I am planning to do real soon, I am personally in favor of committing this now and unblocking Brock and doing that refactoring in the next week or two.
        Hide
        cos Konstantin Boudnik added a comment -

        If you call installing JDK7 forcing, then sure we are forcing and the refactoring is not going to change that forcing

        Actually it will do the forcing, because once the current BIGTOP-1218 is committed there's no option of not installing JDK7. With my proposal I can at least avoid it by not running this special new jdk7 manifest. While I feel for Hive community, my foremost interest is to make sure that one group of users is forced in doing something that another group of users needs. Thus my request to preserve the semantics as much as possible.

        Now, if you use the curl trick to download jdk7 bits you won't be needing too deep knowledge of puppet nor fixing non-idempotent repo-insert operation. The whole modification will be quite non-intrusive and won't change much in the existing recipes. Am I making sense?

        Show
        cos Konstantin Boudnik added a comment - If you call installing JDK7 forcing, then sure we are forcing and the refactoring is not going to change that forcing Actually it will do the forcing, because once the current BIGTOP-1218 is committed there's no option of not installing JDK7. With my proposal I can at least avoid it by not running this special new jdk7 manifest. While I feel for Hive community, my foremost interest is to make sure that one group of users is forced in doing something that another group of users needs. Thus my request to preserve the semantics as much as possible. Now, if you use the curl trick to download jdk7 bits you won't be needing too deep knowledge of puppet nor fixing non-idempotent repo-insert operation. The whole modification will be quite non-intrusive and won't change much in the existing recipes. Am I making sense?
        Hide
        cos Konstantin Boudnik added a comment -

        Mark Grover, is there any chance this can be wrapped up?

        Show
        cos Konstantin Boudnik added a comment - Mark Grover , is there any chance this can be wrapped up?
        Hide
        mgrover Mark Grover added a comment -

        Hey Cos, I still think the attached patch is in reasonably good form and the suggested refactoring can be done in a follow on JIRA, arguably after 0.8. Let me know what you think.

        Show
        mgrover Mark Grover added a comment - Hey Cos, I still think the attached patch is in reasonably good form and the suggested refactoring can be done in a follow on JIRA, arguably after 0.8. Let me know what you think.
        Hide
        cos Konstantin Boudnik added a comment -

        Ok. I don't think this is too important to block the patch because of that. Could you please:

        • update jdk to the latest 7u55
        • create a jira to update the installation logic were discussed above?

        Thanks!

        Show
        cos Konstantin Boudnik added a comment - Ok. I don't think this is too important to block the patch because of that. Could you please: update jdk to the latest 7u55 create a jira to update the installation logic were discussed above? Thanks!
        Hide
        mgrover Mark Grover added a comment -

        Will do, thanks for your +1, Cos!

        Show
        mgrover Mark Grover added a comment - Will do, thanks for your +1, Cos!
        Hide
        cos Konstantin Boudnik added a comment -

        Any updates on this one? My understanding was that the patch is pretty much ready for the prime time?

        Show
        cos Konstantin Boudnik added a comment - Any updates on this one? My understanding was that the patch is pretty much ready for the prime time?
        Hide
        mgrover Mark Grover added a comment -

        It is, been super busy! Will try to do this tomorrow

        Show
        mgrover Mark Grover added a comment - It is, been super busy! Will try to do this tomorrow
        Hide
        mgrover Mark Grover added a comment -

        "Tomorrow" didn't obviously happen, sorry but please stay tuned!

        Show
        mgrover Mark Grover added a comment - "Tomorrow" didn't obviously happen, sorry but please stay tuned!
        Hide
        mgrover Mark Grover added a comment -

        Attached a new patch with the JDK7u60 upgrade and created BIGTOP-1352 for follow-up.
        This is ready to go, thanks!

        Show
        mgrover Mark Grover added a comment - Attached a new patch with the JDK7u60 upgrade and created BIGTOP-1352 for follow-up. This is ready to go, thanks!
        Hide
        mgrover Mark Grover added a comment -

        Any reviews?

        Show
        mgrover Mark Grover added a comment - Any reviews?
        Hide
        cos Konstantin Boudnik added a comment -

        I will work on it today, sorry

        Show
        cos Konstantin Boudnik added a comment - I will work on it today, sorry
        Hide
        mgrover Mark Grover added a comment -

        No worries, thanks!

        Show
        mgrover Mark Grover added a comment - No worries, thanks!
        Hide
        cos Konstantin Boudnik added a comment -

        I've tried the patch and it worked fine on my ubuntu laptop.
        +1 from me.
        If there's no other comments, please commit it. Thanks!

        Show
        cos Konstantin Boudnik added a comment - I've tried the patch and it worked fine on my ubuntu laptop. +1 from me. If there's no other comments, please commit it. Thanks!
        Hide
        mgrover Mark Grover added a comment -

        You're awesome, thanks! Committed!

        Show
        mgrover Mark Grover added a comment - You're awesome, thanks! Committed!

          People

          • Assignee:
            mgrover Mark Grover
            Reporter:
            mgrover Mark Grover
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development