You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@couchdb.apache.org by iilyak <gi...@git.apache.org> on 2015/03/11 00:14:53 UTC

[GitHub] couchdb pull request: Configure `db_definitions` key in config

GitHub user iilyak opened a pull request:

    https://github.com/apache/couchdb/pull/309

    Configure `db_definitions` key in config

    COUCHDB-2635

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/iilyak/couchdb 2635-systems_dbs

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/couchdb/pull/309.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #309
    
----
commit 4bdab2e69b4934938b0610a8b188dbe7a6acfc09
Author: ILYA Khlopotov <ii...@ca.ibm.com>
Date:   2015-03-10T23:13:48Z

    Configure `db_definitions` key in config
    
    COUCHDB-2635

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb pull request: Configure `db_definitions` key in config

Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on a diff in the pull request:

    https://github.com/apache/couchdb/pull/309#discussion_r26305273
  
    --- Diff: rel/overlay/etc/default.ini ---
    @@ -23,6 +23,7 @@ file_compression = snappy
     ; and/or more OS page cache hits, but they can also increase overall response
     ; time for writes when there are many attachment write requests in parallel.
     attachment_stream_buffer_size = 4096
    +db_definitions = [couch_dbs, mem3_dbs, couch_replicator_dbs, cassim_dbs]
    --- End diff --
    
    > Why application environment case doesn't works? 
    
    Because it relies on [a hack](https://github.com/apache/couchdb/blob/master/dev/boot_node.erl#L20:L21) which makes [testing harder](https://github.com/iilyak/couchdb-couch/blob/2547-fix-broken-tests/src/test_util.erl#L257:L260).
    
    > Can we relay on that if some app wanted to define own system database it mush have specially named module with implemented behaviour?
    
    Using convention might work but needs to be very well documented and explained.
    
    > But still...is there no way to avoid exposing such kind of internal information in config file?
    
    There is also a possibility to discover all modules implementing given behavior using
    `lists:keyfind(behaviour, 1, <module>:module_info(attributes))`. However we would need to iterate through all modules using `code:all_loaded`. Which gives us the same problem really. Modules have to be loaded.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb pull request: Configure `db_definitions` key in config

Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on a diff in the pull request:

    https://github.com/apache/couchdb/pull/309#discussion_r26222443
  
    --- Diff: rel/overlay/etc/default.ini ---
    @@ -23,6 +23,7 @@ file_compression = snappy
     ; and/or more OS page cache hits, but they can also increase overall response
     ; time for writes when there are many attachment write requests in parallel.
     attachment_stream_buffer_size = 4096
    +db_definitions = [couch_dbs, mem3_dbs, couch_replicator_dbs, cassim_dbs]
    --- End diff --
    
    It is plural because `couch_define_db` behavior defined multiple dbs ([see](https://github.com/iilyak/couchdb-mem3/blob/2635-systems_dbs/src/mem3_dbs.erl#L8)). 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb issue #309: Configure `db_definitions` key in config

Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on the issue:

    https://github.com/apache/couchdb/pull/309
  
    Implemented using different means


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb pull request: Configure `db_definitions` key in config

Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on a diff in the pull request:

    https://github.com/apache/couchdb/pull/309#discussion_r26262786
  
    --- Diff: rel/overlay/etc/default.ini ---
    @@ -23,6 +23,7 @@ file_compression = snappy
     ; and/or more OS page cache hits, but they can also increase overall response
     ; time for writes when there are many attachment write requests in parallel.
     attachment_stream_buffer_size = 4096
    +db_definitions = [couch_dbs, mem3_dbs, couch_replicator_dbs, cassim_dbs]
    --- End diff --
    
    @kxepal Where do you propose to store registered plugins? We need some form of persistent storage. Considered options are:
      1. well known key in application environment. 
          - Didn't quite work. Due to the requirement that all applications need to be loaded.
      2. configuration file similar to priv/stats_descriptions.cfg.
         - It is awkward to scan a filesystem in startup time
      3. default.ini
        - easiest
    
    None of the options considered support one click install.
    
    I think we have a misconception about the scope. Vendors do compile `couchdb` locally and include applications they need. They do not use single click install. Their goal is to plug the features they've implemented into `couchdb` without changing `couchdb` code. It can be thought of as embedding `couchdb` into a bigger ecosystem of applications. While you are thinking about people who want to download `couchdb` and few plugins with additional functionality they need and run it on their servers. Which requires different level of expertise. Making downloadable plugins infrastructure is a much bigger scope in terms of developing, testing and support. I don't think I have resources to do that alone in a reasonable time frame. Besides this might be quite complex and rejected by community on this ground.  


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb pull request: Configure `db_definitions` key in config

Posted by janl <gi...@git.apache.org>.
Github user janl commented on a diff in the pull request:

    https://github.com/apache/couchdb/pull/309#discussion_r26223044
  
    --- Diff: rel/overlay/etc/default.ini ---
    @@ -23,6 +23,7 @@ file_compression = snappy
     ; and/or more OS page cache hits, but they can also increase overall response
     ; time for writes when there are many attachment write requests in parallel.
     attachment_stream_buffer_size = 4096
    +db_definitions = [couch_dbs, mem3_dbs, couch_replicator_dbs, cassim_dbs]
    --- End diff --
    
    thanks, now I get it!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb pull request: Configure `db_definitions` key in config

Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on a diff in the pull request:

    https://github.com/apache/couchdb/pull/309#discussion_r26263046
  
    --- Diff: rel/overlay/etc/default.ini ---
    @@ -23,6 +23,7 @@ file_compression = snappy
     ; and/or more OS page cache hits, but they can also increase overall response
     ; time for writes when there are many attachment write requests in parallel.
     attachment_stream_buffer_size = 4096
    +db_definitions = [couch_dbs, mem3_dbs, couch_replicator_dbs, cassim_dbs]
    --- End diff --
    
    I want to mention that I do work on something bigger than the scope of this PR ([see](https://github.com/iilyak/plugerl)). However it wouldn't be one-click install either.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb pull request: Configure `db_definitions` key in config

Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on a diff in the pull request:

    https://github.com/apache/couchdb/pull/309#discussion_r26222086
  
    --- Diff: rel/overlay/etc/default.ini ---
    @@ -23,6 +23,7 @@ file_compression = snappy
     ; and/or more OS page cache hits, but they can also increase overall response
     ; time for writes when there are many attachment write requests in parallel.
     attachment_stream_buffer_size = 4096
    +db_definitions = [couch_dbs, mem3_dbs, couch_replicator_dbs, cassim_dbs]
    --- End diff --
    
    It defines a list of modules which implement `couch_define_db` behavior. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb pull request: Configure `db_definitions` key in config

Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on a diff in the pull request:

    https://github.com/apache/couchdb/pull/309#discussion_r26263819
  
    --- Diff: rel/overlay/etc/default.ini ---
    @@ -23,6 +23,7 @@ file_compression = snappy
     ; and/or more OS page cache hits, but they can also increase overall response
     ; time for writes when there are many attachment write requests in parallel.
     attachment_stream_buffer_size = 4096
    +db_definitions = [couch_dbs, mem3_dbs, couch_replicator_dbs, cassim_dbs]
    --- End diff --
    
    Maybe we should use `dets` for persistent things. ;-)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb pull request: Configure `db_definitions` key in config

Posted by janl <gi...@git.apache.org>.
Github user janl commented on a diff in the pull request:

    https://github.com/apache/couchdb/pull/309#discussion_r26203891
  
    --- Diff: rel/overlay/etc/default.ini ---
    @@ -23,6 +23,7 @@ file_compression = snappy
     ; and/or more OS page cache hits, but they can also increase overall response
     ; time for writes when there are many attachment write requests in parallel.
     attachment_stream_buffer_size = 4096
    +db_definitions = [couch_dbs, mem3_dbs, couch_replicator_dbs, cassim_dbs]
    --- End diff --
    
    what does this define? Why is `_dbs` plural?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb pull request: Configure `db_definitions` key in config

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:

    https://github.com/apache/couchdb/pull/309#discussion_r26267421
  
    --- Diff: rel/overlay/etc/default.ini ---
    @@ -23,6 +23,7 @@ file_compression = snappy
     ; and/or more OS page cache hits, but they can also increase overall response
     ; time for writes when there are many attachment write requests in parallel.
     attachment_stream_buffer_size = 4096
    +db_definitions = [couch_dbs, mem3_dbs, couch_replicator_dbs, cassim_dbs]
    --- End diff --
    
    No, I agree with you that plugins system itself is a very much more bigger question, but there is no need to think about it too much to have problems I mentioned. You may always assume that couch_mrview and couch_replicator are these plugins (however, only the latter fits this role nicely) which are just available out of the box. 
    
    Let's limit our scope of "plugins" by these two apps as we know them well. Both adds own system databases, both could be optional (let's dream a bit here), both vendor may want to replace with own solution (in additional to the others it has). 
    
    The biggest problem here is unavailability to gracefully handle CouchDB upgrade when you'd add own `db_definitions` to local.ini as overlay system demands. If you'd edit them in default.ini you'll end with manual list merge. So if CouchDB ships with none of them, vendor adds own `couch_foo`, user installs `couch_replicator` and later CouchDB adds `couch_mrview` everything gets eventually broken.
    
    This problem actually could be easily solved by turning horizontal list into vertical:
    
    ```
    [dbs_definitions]
    couch_mrview = couch_mrview_dbs
    mem3 = mem3_dbs
    ```
    
    Where key is application name and value is a related module.  Overlay mechanism will provide all the features to turn on / turn off specific modules with easy.
    
    But still...is there no way to avoid exposing such kind of internal information in config file?
    
    Why application environment case doesn't works? Once application is loaded it publish the module with the behaviour about system databases it needs. If it's not loaded then it's not used.
    
    Can we relay on that if some app wanted to define own system database it mush have specially named module with implemented behaviour?
    
    > Maybe we should use dets for persistent things. ;-)
    
    No, let's create another system database which stores other system databases names (; And we need to go deeper (:


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb pull request: Configure `db_definitions` key in config

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:

    https://github.com/apache/couchdb/pull/309#discussion_r26230606
  
    --- Diff: rel/overlay/etc/default.ini ---
    @@ -23,6 +23,7 @@ file_compression = snappy
     ; and/or more OS page cache hits, but they can also increase overall response
     ; time for writes when there are many attachment write requests in parallel.
     attachment_stream_buffer_size = 4096
    +db_definitions = [couch_dbs, mem3_dbs, couch_replicator_dbs, cassim_dbs]
    --- End diff --
    
    Can we not have this list in config at all? 
    Here is the problem: you're going to install couch_foo plugin which has `couch_define_db` behaviour. Plugin install happens in one-single-click way, so user don't have to touch any configs for that. But now, to let this plugin works it need to add himself to that list. That literally means: config:get => put itself these or ensure that it's defined once and only => serialize list => config:set...mmm, ok. Now we want to uninstall that plugin...
    
    You got the idea? Having `couch_define_db` behaviour isn't enough to let your (abstract) app work correctly: you have to modify this config value. So, technically, we still have a SYSTEM_DATABASES list, not in Erlang  code, but in ini file. We cannot define own system databases with it, but enforces to keep this list in actual state for all our apps we have.
    
    P.S. Few edge cases:
    - In default.ini we have `[couch_dbs, mem3_dbs, couch_replicator_dbs, cassim_dbs]`
    - User install a couch_foo plugin which puts a new value in local.ini `[couch_dbs, mem3_dbs, couch_replicator_dbs, cassim_dbs, couch_foo]`
    - In new CouchDB release we add new app which also defines own system database, so default.ini changed in the way to have `[couch_dbs, mem3_dbs, couch_replicator_dbs, cassim_dbs, couch_new_app]`
    Question: how we'll deal with upgrade, since values in local.ini overrides default.ini?
    
    What if I have a non existed module in this list from the plugin I just removed?
    What if I have it defined here twice or more?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb pull request #309: Configure `db_definitions` key in config

Posted by iilyak <gi...@git.apache.org>.
Github user iilyak closed the pull request at:

    https://github.com/apache/couchdb/pull/309


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---