couchdb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Adam Kocoloski (Commented) (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (COUCHDB-1323) couch_replicator
Date Sun, 06 Nov 2011 01:06:51 GMT

    [ https://issues.apache.org/jira/browse/COUCHDB-1323?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13144882#comment-13144882
] 

Adam Kocoloski commented on COUCHDB-1323:
-----------------------------------------

Thanks for doing this, Benoit, it seems like a really good idea. I have a few questions from
reading the patch:

* Why is couch_replicator.app committed to version control?  The app.src should be sufficient.

* The list of registered processes in the app.src file is incomplete.  At the very least couch_replicator_manager
belongs in there.

* I have to say it: our application startup procedure is a huge mess.  This patch doesn't
necessarily make matters any worse in that regard, but unfortunately it doesn't improve matters
either.  It starts the top-level supervisor of the couch_replicator application from inside
the couch application, but never actually starts the couch_replicator application.  You and
I both know what the Right Way looks like at this point, hopefully we'll bring Apache CouchDB
up to snuff soon.  The current patch might be OK, though if I'm reading it correctly application:start(couch_replicator)
will crash if the couch application is already running.  Yuck.

* Are the record definitions in couch_replicator.hrl meant for use by other OTP applications?
 If not I think we should move the .hrl file to src/.  Reserving include/ for headers needed
to use the application is a good practice.

* Nitpick - I prefer to name children in the supervision tree after the callback module. 
I made the mistake of calling the old child couch_replication_supervisor instead of couch_rep_sup
a long time ago and it's bugged me ever since.  Can we rename the new child to couch_replicator_tasks_sup
instead?

* Now that the replicator is shaped a little more like an OTP application it bugs me that
couch_replicator is both the API module and a gen_server.  I don't want to see it changed
in this commit, but down the line I'd like to split that out so we have a different module
that actually manages the various processes associated with a replication task.

Again, this is a big step in the right direction.  Thanks for getting it going.
                
> couch_replicator
> ----------------
>
>                 Key: COUCHDB-1323
>                 URL: https://issues.apache.org/jira/browse/COUCHDB-1323
>             Project: CouchDB
>          Issue Type: Improvement
>          Components: Replication
>    Affects Versions: 2.0, 1.3
>            Reporter: Benoit Chesneau
>              Labels: replication
>         Attachments: 0001-move-couchdb-replication-to-a-standalone-application.patch,
0001-move-couchdb-replication-to-a-standalone-application.patch, 0001-move-couchdb-replication-to-a-standalone-application.patch,
0001-move-couchdb-replication-to-a-standalone-application.patch
>
>
> patch to move couch replication modules in in a standalone couch_replicator application.
It's also available on my github:
> https://github.com/benoitc/couchdb/compare/master...couch_replicator

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Mime
View raw message