CouchDB
  1. CouchDB
  2. COUCHDB-216

Make couchdb adhere more to OTP guidelines

    Details

    • Type: Improvement Improvement
    • Status: Reopened
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None
    • Skill Level:
      Committers Level (Medium to Hard)

      Description

      CouchDB could adhere to otp standards in a better way.
      Currently we have:

      • couch.app is not uptodate
      • couch_server.erl is an amalgam of 2 behaviours, which is considered being "not good".

      From my beginner's perception of OTP, it seems CouchDB is not treated as a running application when it is actually running. E.g. appmon doesn't show the application once CouchDB is started.

        Activity

        Hide
        Martin S added a comment - - edited

        The three patches were created by GIT and should apply cleanly onto github's couchdb as of 76b1dd9bffe80eeabe5135c170f6d9527cf1e4f1 / http://svn.apache.org/repos/asf/couchdb/trunk@735875.

        The same changes are part of branch merge/otp at http://github.com/zeitgeist/couchdb.

        Show
        Martin S added a comment - - edited The three patches were created by GIT and should apply cleanly onto github's couchdb as of 76b1dd9bffe80eeabe5135c170f6d9527cf1e4f1 / http://svn.apache.org/repos/asf/couchdb/trunk@735875 . The same changes are part of branch merge/otp at http://github.com/zeitgeist/couchdb .
        Hide
        Martin S added a comment -

        jan___ / #couchdb told me that the release-file way of starting would break things on erlang upgrade.

        Show
        Martin S added a comment - jan___ / #couchdb told me that the release-file way of starting would break things on erlang upgrade.
        Hide
        Ben Browning added a comment -


        The modules section of couch.app was outdated. Instead of updating it manually, I've modified the Makefile to populate this based on the modules actually present when building. This should keep it from getting out of date in the future.

        Show
        Ben Browning added a comment - The modules section of couch.app was outdated. Instead of updating it manually, I've modified the Makefile to populate this based on the modules actually present when building. This should keep it from getting out of date in the future.
        Hide
        Chris Anderson added a comment -

        It's generally agreed that we should make CouchDB more OTP-like.

        I'm not sure about the implementation or the timeline for doing it, but I think it's the best path to auto-sharding.

        Show
        Chris Anderson added a comment - It's generally agreed that we should make CouchDB more OTP-like. I'm not sure about the implementation or the timeline for doing it, but I think it's the best path to auto-sharding.
        Hide
        Paul Joseph Davis added a comment -

        This tarball includes a script and a patch to update the CouchDB source tree fairly extensively. Among the important bits:

        The root directory now follows the: "src, ebin, priv, include" structure. The dependencies are pushed into a deps directory and each dependency gets the "src/ebin/include" treatement.

        the main CouchDB source directory has been split into a handful of directories to start organizing the modules by related functionality. This pattern is copied from the monaco application in the stdlib.

        couchspawnkillable and couchjs are now in priv/bin. couchjs still gets its /usr/local/bin script.

        couch_erl_driver is in priv/lib where it should be. I think we should be able to even remove the paths when loading this, but again I'd want to double check on whether someone might have a reason for installing this elsewhere.

        I started building beam files with a call to make:all() within Erlang. This actually builds stuff quite a bit faster and so far is pretty useful. That said it kind of doesn't fit into the Autotools process because it never rebuilds beam files on change because the call is never triggered because I couldn't figure out how to add the proper dependency.

        Also, there's a weirdness in that I symlink deps/couch to the root directory so that ./utils/run can work with ERL_LIBS instead of four specifications of -pa.

        Anyway, this is an initial layout.

        Show
        Paul Joseph Davis added a comment - This tarball includes a script and a patch to update the CouchDB source tree fairly extensively. Among the important bits: The root directory now follows the: "src, ebin, priv, include" structure. The dependencies are pushed into a deps directory and each dependency gets the "src/ebin/include" treatement. the main CouchDB source directory has been split into a handful of directories to start organizing the modules by related functionality. This pattern is copied from the monaco application in the stdlib. couchspawnkillable and couchjs are now in priv/bin. couchjs still gets its /usr/local/bin script. couch_erl_driver is in priv/lib where it should be. I think we should be able to even remove the paths when loading this, but again I'd want to double check on whether someone might have a reason for installing this elsewhere. I started building beam files with a call to make:all() within Erlang. This actually builds stuff quite a bit faster and so far is pretty useful. That said it kind of doesn't fit into the Autotools process because it never rebuilds beam files on change because the call is never triggered because I couldn't figure out how to add the proper dependency. Also, there's a weirdness in that I symlink deps/couch to the root directory so that ./utils/run can work with ERL_LIBS instead of four specifications of -pa. Anyway, this is an initial layout.
        Hide
        Robert Newson added a comment -


        Consider making couchdb sufficiently OTP-like to support zero time 'hot' upgrades between releases.

        Show
        Robert Newson added a comment - Consider making couchdb sufficiently OTP-like to support zero time 'hot' upgrades between releases.
        Hide
        Adam Kocoloski added a comment -

        Here's an updated patch inspired by Martin's and Ben's work. It stops short using a boot file to start CouchDB; hopefully that allays some of Jan's concerns regarding a build surviving an Erlang VM upgrade. Highlights:

        • remove application behavior from couch_server and add it to couch_app instead
        • starts/stops the server using OTP application controls
        • couch.erl convenience module exports start(), stop(), restart(), reload()
        • modules list in couch.app auto-generated using Ben's scary sed command
        • server startup command in shell script is much simpler now

        I don't know if anyone was using couch_server:dev_start(), but I tried to update that correctly, too.

        Finally, I dropped `icu-config --invoke` from the startup command. In my experience couch finds the ICU libraries just fine without it, and it causes serious problems on Mac OS X when MacPorts is used to satisfy dependencies. In particular, it utterly breaks couchdb-lucene.

        Show
        Adam Kocoloski added a comment - Here's an updated patch inspired by Martin's and Ben's work. It stops short using a boot file to start CouchDB; hopefully that allays some of Jan's concerns regarding a build surviving an Erlang VM upgrade. Highlights: remove application behavior from couch_server and add it to couch_app instead starts/stops the server using OTP application controls couch.erl convenience module exports start(), stop(), restart(), reload() modules list in couch.app auto-generated using Ben's scary sed command server startup command in shell script is much simpler now I don't know if anyone was using couch_server:dev_start(), but I tried to update that correctly, too. Finally, I dropped `icu-config --invoke` from the startup command. In my experience couch finds the ICU libraries just fine without it, and it causes serious problems on Mac OS X when MacPorts is used to satisfy dependencies. In particular, it utterly breaks couchdb-lucene.
        Hide
        Adam Kocoloski added a comment -

        A future improvement in this area would be to include crypto, ibrowse, mochiweb, etc. in couch's supervision tree. At the moment we just start them if they're not already started and leave them running if couch stops. That's not what OTP intends by "included_applications" in the .app file.

        Show
        Adam Kocoloski added a comment - A future improvement in this area would be to include crypto, ibrowse, mochiweb, etc. in couch's supervision tree. At the moment we just start them if they're not already started and leave them running if couch stops. That's not what OTP intends by "included_applications" in the .app file.
        Hide
        Noah Slater added a comment -

        There is no point updating the app file until Erlang has a way of not breaking applications on upgrade.

        Show
        Noah Slater added a comment - There is no point updating the app file until Erlang has a way of not breaking applications on upgrade.
        Hide
        Paul Joseph Davis added a comment -

        Noah,

        I think you mean the rel file. The app file is a description of the application and isn't tied to the Erlang VM. I don't have any experience with Erlang's release tools, but I'd agree that if it binds to a specific VM then at most we should just provide a non-standard make target to produce a release.

        Keeping the app file up to date is a priority as it's used for other things like detecting conflicting registered processes.

        Show
        Paul Joseph Davis added a comment - Noah, I think you mean the rel file. The app file is a description of the application and isn't tied to the Erlang VM. I don't have any experience with Erlang's release tools, but I'd agree that if it binds to a specific VM then at most we should just provide a non-standard make target to produce a release. Keeping the app file up to date is a priority as it's used for other things like detecting conflicting registered processes.
        Hide
        Adam Kocoloski added a comment -

        Hmm, some folks might be packaging CouchDB as part of an Erlang release, in which case they'll be aware of the Erlang version issue. I know at least one person who does this

        I think maintaining the .app file is generally a sign of good behavior. That said, if we're committed to not maintaining it we should remove it from the distribution.

        Show
        Adam Kocoloski added a comment - Hmm, some folks might be packaging CouchDB as part of an Erlang release, in which case they'll be aware of the Erlang version issue. I know at least one person who does this I think maintaining the .app file is generally a sign of good behavior. That said, if we're committed to not maintaining it we should remove it from the distribution.
        Hide
        Paul Joseph Davis added a comment -

        I'd agree that people doing releases are probably advanced enough to know what they're getting into. Unless someone can figure out a sane way to make releases work, I don't think its something we should worry too much about.

        To me, removing the .app file is unacceptable. The fact that its broken is just a reminder that we've been Doing It Wrong and should fix things, not break them even further.

        Show
        Paul Joseph Davis added a comment - I'd agree that people doing releases are probably advanced enough to know what they're getting into. Unless someone can figure out a sane way to make releases work, I don't think its something we should worry too much about. To me, removing the .app file is unacceptable. The fact that its broken is just a reminder that we've been Doing It Wrong and should fix things, not break them even further.
        Hide
        Adam Kocoloski added a comment -

        Mark, can you take a look and see how badly I broke the Windows build with this patch? I updated couchdb.bat.tpl.in but I don't know if the build system actually loads the correct paths into the .app file on Windows.

        Show
        Adam Kocoloski added a comment - Mark, can you take a look and see how badly I broke the Windows build with this patch? I updated couchdb.bat.tpl.in but I don't know if the build system actually loads the correct paths into the .app file on Windows.
        Hide
        Robert Newson added a comment -

        broken on Ubuntu as basename doesn't accept -s

        from src/couchdb/Makefile.am

        couch.app: couch.app.tpl
        modules=`find . -name "*.erl" -exec basename -s .erl {} \; | tr '\n' ',' | sed "s/,$$//"`; \

        basename: invalid option – 's'
        Try `basename --help' for more information.

        Show
        Robert Newson added a comment - broken on Ubuntu as basename doesn't accept -s from src/couchdb/Makefile.am couch.app: couch.app.tpl modules=`find . -name "*.erl" -exec basename -s .erl {} \; | tr '\n' ',' | sed "s/,$$//"`; \ basename: invalid option – 's' Try `basename --help' for more information.
        Hide
        Noah Slater added a comment -

        "I think you mean the rel file."

        Yes, I did.

        "Keeping the app file up to date is a priority as it's used for other things like detecting conflicting registered processes."

        Really does it do anything concrete for us? It seems like a very silly and pointless file to me.

        "Hmm, some folks might be packaging CouchDB as part of an Erlang release, in which case they'll be aware of the Erlang version issue."

        I was packaging CouchDB as part of the Debian Erlang packages, and unless you tie the CouchDB package to a specific version of the Erlang package, and update them in tandem, you're hosed.

        "To me, removing the .app file is unacceptable. The fact that its broken is just a reminder that we've been Doing It Wrong and should fix things, not break them even further."

        The fact that we've only just noticed should indicate that it probably doesn't matter.

        Show
        Noah Slater added a comment - "I think you mean the rel file." Yes, I did. "Keeping the app file up to date is a priority as it's used for other things like detecting conflicting registered processes." Really does it do anything concrete for us? It seems like a very silly and pointless file to me. "Hmm, some folks might be packaging CouchDB as part of an Erlang release, in which case they'll be aware of the Erlang version issue." I was packaging CouchDB as part of the Debian Erlang packages, and unless you tie the CouchDB package to a specific version of the Erlang package, and update them in tandem, you're hosed. "To me, removing the .app file is unacceptable. The fact that its broken is just a reminder that we've been Doing It Wrong and should fix things, not break them even further." The fact that we've only just noticed should indicate that it probably doesn't matter.
        Hide
        Paul Joseph Davis added a comment -

        There are quite a few Erlang tools that require the .app file to be present. Things like application:start(couchdb) require it. I'm pretty sure appmon requires it. It may seem unimportant but it breaks a whole bunch of things. Consider not having a .app file like not having a setup.py in a distributed Python module.

        The .rel stuff I think is orthogonal to making a package for Debian. From what I've read about Erlang's release tools, it seems like they were designed for pushing packages to a homogenous environment for hot code upgrades and the like. As you saw, that kinda breaks down in a Debian style distribution environment.

        Bottom line, the .rel is unimportant, but the .app is quite important.

        Show
        Paul Joseph Davis added a comment - There are quite a few Erlang tools that require the .app file to be present. Things like application:start(couchdb) require it. I'm pretty sure appmon requires it. It may seem unimportant but it breaks a whole bunch of things. Consider not having a .app file like not having a setup.py in a distributed Python module. The .rel stuff I think is orthogonal to making a package for Debian. From what I've read about Erlang's release tools, it seems like they were designed for pushing packages to a homogenous environment for hot code upgrades and the like. As you saw, that kinda breaks down in a Debian style distribution environment. Bottom line, the .rel is unimportant, but the .app is quite important.
        Hide
        Adam Kocoloski added a comment -

        Thanks Robert, hopefully fixed in r809407

        Show
        Adam Kocoloski added a comment - Thanks Robert, hopefully fixed in r809407
        Hide
        Paul Joseph Davis added a comment -

        This patch breaks ./utils/run because of the use of ERL_LIBS now which is incompatible with how our source directory is setup. Quickest fix I can think of would be to get a copy of the old couchdb.tpl.in and use that for the utils/run. Longer term when we get around to cleaning up the tree we can revert back to using couchdb.tpl

        Show
        Paul Joseph Davis added a comment - This patch breaks ./utils/run because of the use of ERL_LIBS now which is incompatible with how our source directory is setup. Quickest fix I can think of would be to get a copy of the old couchdb.tpl.in and use that for the utils/run. Longer term when we get around to cleaning up the tree we can revert back to using couchdb.tpl
        Hide
        Steven R. Loomis added a comment -

        re: icu-config --invoke - it would be better to use pkg-config and standard locations instead of icu-config. Better to not use icu-config at all. http://userguide.icu-project.org/howtouseicu#TOC-pkg-config

        -srl, ICU project

        Show
        Steven R. Loomis added a comment - re: icu-config --invoke - it would be better to use pkg-config and standard locations instead of icu-config. Better to not use icu-config at all. http://userguide.icu-project.org/howtouseicu#TOC-pkg-config -srl, ICU project

          People

          • Assignee:
            Unassigned
            Reporter:
            Martin S
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:

              Development