Details

    • Type: New Feature New Feature
    • Status: In Progress
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: new service
    • Labels:
      None

      Description

      "Riak is a Dynamo-inspired database that is being used in production by companies like Mozilla and Comcast. Riak scales predictably and easily and simplifies development by giving users the ability to quickly prototype, test, and deploy their applications.

      A truly fault-tolerant system, Riak has no single point of failure. No machines are special or central in Riak, so developers and operations professionals can decide exactly how fault-tolerant they want and need their applications to be."

      http://wiki.basho.com/Home.html

      1. WHIRR-273-Add-Riak-as-a-Service.patch
        99 kB
        Jhey Tompkins
      2. WHIRR-273-Add-Riak-as-a-Service.patch
        25 kB
        Jhey Tompkins
      3. WHIRR-273-Add-Riak-as-a-Service.patch
        16 kB
        Jhey Tompkins
      4. WHIRR-273-Add-Riak-as-a-Service.patch
        20 kB
        Jhey Tompkins
      5. WHIRR-273-Add-Riak-as-a-Service.patch
        20 kB
        Jhey Tompkins
      6. WHIRR-273-Add-Riak-as-a-Service.patch
        20 kB
        Jhey Tompkins
      7. WHIRR-273-Add-Riak-as-a-Service.patch
        25 kB
        Jhey Tompkins

        Activity

        Hide
        Jhey Tompkins added a comment -

        Patch for adding Riak as a Service. Was working fine but for some reason after installation, configuration is not behaving properly.

        Show
        Jhey Tompkins added a comment - Patch for adding Riak as a Service. Was working fine but for some reason after installation, configuration is not behaving properly.
        Hide
        Andrei Savu added a comment -

        Jhey some of the things I noticed while taking a quick look at the patch:

        • please do not include *.log files and .swp files in the patch
        • we've switched back to using regular instances for integration testing (remove whirr.aws-ec2-spot-price)
        • in start_riak() you've wrote "bin riak start" - what is "bin" used for?
        • we are using curl to download artefacts - take a look at install_tarball() - the important part is that you should have some sort of retry loop.
        • in configure_riak() you've wrote "bin riak-admin reip riak@127.0.0.1 riak@$PRIVATE_IP". What is "bin" for?
        • remove unused methods like getHosts() and the RiakCluster class
        • I don't think we need to include in the changeset riak-local-nodes-byon.yaml

        As I said before (offline) the easies way to try scrips is on local VMs line by line and as you get them working you can move to BYON and after that to cloud providers.

        As you update the patch towards making it work I will get back to you with more updates.

        Show
        Andrei Savu added a comment - Jhey some of the things I noticed while taking a quick look at the patch: please do not include *.log files and .swp files in the patch we've switched back to using regular instances for integration testing (remove whirr.aws-ec2-spot-price) in start_riak() you've wrote "bin riak start" - what is "bin" used for? we are using curl to download artefacts - take a look at install_tarball() - the important part is that you should have some sort of retry loop. in configure_riak() you've wrote "bin riak-admin reip riak@127.0.0.1 riak@$PRIVATE_IP". What is "bin" for? remove unused methods like getHosts() and the RiakCluster class I don't think we need to include in the changeset riak-local-nodes-byon.yaml As I said before (offline) the easies way to try scrips is on local VMs line by line and as you get them working you can move to BYON and after that to cloud providers. As you update the patch towards making it work I will get back to you with more updates.
        Hide
        Jhey Tompkins added a comment -

        New Patch added. Working properly this time but still some minor refinements to make.

        Show
        Jhey Tompkins added a comment - New Patch added. Working properly this time but still some minor refinements to make.
        Hide
        Alex Heneveld added a comment - - edited

        My suggestions:

        • patch hygiene: should not include Zookeeper files (esp with just imports rearranged)
        • Riak recipes: just make them like all the others in there
          • byon recipe should refer to standard file://.../nodes.byon like the others
          • need an ec2 recipe and testing
        • Riak scripts:
          • lots of risks of endless loops waiting for files, trying to join, and no logging/warning in such cases;
            and a long delay within the loop; better to have a short delay (1s) and a max num attempts,
            loop terminate if return code 0 or num remaining attempts goes to 0,
            with an error logged and non-zero exit if max attempts goes to zero;
            and helpful to add a comment about why the testing is needed
            (these scripts should be happening sequentially so can assume files exist, that is what is done elsewhere)
          • the default config file refers to a property whirr.riak.binary.url but that is never used;
            what's the point of having that file?
        Show
        Alex Heneveld added a comment - - edited My suggestions: patch hygiene: should not include Zookeeper files (esp with just imports rearranged) Riak recipes: just make them like all the others in there byon recipe should refer to standard file://.../nodes.byon like the others need an ec2 recipe and testing Riak scripts: lots of risks of endless loops waiting for files, trying to join, and no logging/warning in such cases; and a long delay within the loop; better to have a short delay (1s) and a max num attempts, loop terminate if return code 0 or num remaining attempts goes to 0, with an error logged and non-zero exit if max attempts goes to zero; and helpful to add a comment about why the testing is needed (these scripts should be happening sequentially so can assume files exist, that is what is done elsewhere) the default config file refers to a property whirr.riak.binary.url but that is never used; what's the point of having that file?
        Hide
        Jhey Tompkins added a comment -

        Added new patch which now deals with platform recognition and download retries for Riak binaries. Also added extra logging around various configuration events.

        Still needs RiakServiceTest to be populated with tests.

        This has been tested however with a 2 node ring on BYON and a 3 node ring on Amazon EC2 using the recipe file in the patch. Both cases were succesful.

        Show
        Jhey Tompkins added a comment - Added new patch which now deals with platform recognition and download retries for Riak binaries. Also added extra logging around various configuration events. Still needs RiakServiceTest to be populated with tests. This has been tested however with a 2 node ring on BYON and a 3 node ring on Amazon EC2 using the recipe file in the patch. Both cases were succesful.
        Hide
        Jhey Tompkins added a comment -

        Forgot missing POM diffs in patch. Have added them and resubmitted patch.

        Show
        Jhey Tompkins added a comment - Forgot missing POM diffs in patch. Have added them and resubmitted patch.
        Hide
        Andrei Savu added a comment - - edited

        Nice work! I'm glad to see this working.

        Here are some things I've noticed while looking at the patch:

        • remove the zookeeper tarball url from the riak byon recipe
        • let the user specify the version for the install artefacts (e.g. whirr.riak.version=0.1.0) - I assume the naming convention is consistent for all releases
        • do not make the config files (app.config, vm.args) writable by all users (chomod 777)
        • extract the retry loop from install_riak() as a function to reduce code duplication

        I will now start a cluster and get back with more feedback about integration testing.

        Show
        Andrei Savu added a comment - - edited Nice work! I'm glad to see this working. Here are some things I've noticed while looking at the patch: remove the zookeeper tarball url from the riak byon recipe let the user specify the version for the install artefacts (e.g. whirr.riak.version=0.1.0) - I assume the naming convention is consistent for all releases do not make the config files (app.config, vm.args) writable by all users (chomod 777) extract the retry loop from install_riak() as a function to reduce code duplication I will now start a cluster and get back with more feedback about integration testing.
        Hide
        Andrei Savu added a comment -

        Unfortunately I'm unable to apply the patch:

        patch: **** malformed patch at line 27: diff --git recipes/riak-byon.properties recipes/riak-byon.properties
        

        Before submitting try applying on a clean branch using: patch -p0 --dry-run < PATCH_FILE.patch

        Show
        Andrei Savu added a comment - Unfortunately I'm unable to apply the patch: patch: **** malformed patch at line 27: diff --git recipes/riak-byon.properties recipes/riak-byon.properties Before submitting try applying on a clean branch using: patch -p0 --dry-run < PATCH_FILE.patch
        Hide
        Andrei Savu added a comment -

        One more thing: I think in install_riak() you want to replace if [ 'which dpkg | egrep 'dpkg'' ] with if [ `which dpkg | egrep 'dpkg'` ]. Note ` versus '. if [ 'something' ] is always true.

        Show
        Andrei Savu added a comment - One more thing: I think in install_riak() you want to replace if [ 'which dpkg | egrep 'dpkg'' ] with if [ `which dpkg | egrep 'dpkg'` ] . Note ` versus '. if [ 'something' ] is always true.
        Hide
        Jhey Tompkins added a comment -

        Forgive me for leaving the Zookeeper Tarball URL in there. It was very late, I've removed it and now the patch should work, I will make those changes you suggested and apply a newer patch as soon as.

        Show
        Jhey Tompkins added a comment - Forgive me for leaving the Zookeeper Tarball URL in there. It was very late, I've removed it and now the patch should work, I will make those changes you suggested and apply a newer patch as soon as.
        Hide
        Andrei Savu added a comment -

        For integration testing IMO you should use the Riak java client library (added as a test dependency), create a bucket, write & read a key, value pair. We should also find a way of checking that the ring has the expected size.

        [1] https://github.com/basho/riak-java-client

        Show
        Andrei Savu added a comment - For integration testing IMO you should use the Riak java client library (added as a test dependency), create a bucket, write & read a key, value pair. We should also find a way of checking that the ring has the expected size. [1] https://github.com/basho/riak-java-client
        Hide
        Andrei Savu added a comment -

        I've done a quick test on cloudsevers-us and it works as expected.

        Show
        Andrei Savu added a comment - I've done a quick test on cloudsevers-us and it works as expected.
        Hide
        Jhey Tompkins added a comment -

        Made Andreis suggested changes minus integration testing which is being worked on currently.

        Show
        Jhey Tompkins added a comment - Made Andreis suggested changes minus integration testing which is being worked on currently.
        Hide
        Jhey Tompkins added a comment -

        Foundations of test classes added.

        Show
        Jhey Tompkins added a comment - Foundations of test classes added.

          People

          • Assignee:
            Jhey Tompkins
            Reporter:
            Andrei Savu
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:

              Development