CouchDB
  1. CouchDB
  2. COUCHDB-992

replicator doesn't respect .ini settings for http sources

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.0.1
    • Fix Version/s: 1.0.2, 1.1
    • Component/s: Replication
    • Labels:
      None

      Description

      The replication reader ignores the settings in the config file.
      More accurately, couch_db.hrl sets them from couch_config but then couch_rep_reader overrides them.

        Activity

        Hide
        Randall Leeds added a comment -

        Here's the patch.

        couch_rep_reader no longer sets these settings when init is called, leaving it to the http_db record to hold the correct defaults.

        I changed the #http_db record to use the defaults that had been forced by couch_rep_reader before. I thought this was safest. Since each replication between two hosts will open as many as 100 requests at a time any change which lowered the default value of (sessions x pipeline_size) could cause user's replications to experience a regression of surprising back-off or even failure.

        Each replication reader registers on config changes too, so it should be possible to change how many sessions continuous or long-running replications are using without restarting them.

        Thankfully, I think most of this goes away with the new replicator, but since that's not default yet I propose this for backport to 1.0.2 and 1.1.

        Unfortunately, I actually have a situation where a pre-release xulrunner-devel on my system is preventing CouchDB from compiling. I'll have to open a separate ticket for that, but please forgive me if this patch contains some tiny syntax error. I wanted to file it before I forgot (again).

        Show
        Randall Leeds added a comment - Here's the patch. couch_rep_reader no longer sets these settings when init is called, leaving it to the http_db record to hold the correct defaults. I changed the #http_db record to use the defaults that had been forced by couch_rep_reader before. I thought this was safest. Since each replication between two hosts will open as many as 100 requests at a time any change which lowered the default value of (sessions x pipeline_size) could cause user's replications to experience a regression of surprising back-off or even failure. Each replication reader registers on config changes too, so it should be possible to change how many sessions continuous or long-running replications are using without restarting them. Thankfully, I think most of this goes away with the new replicator, but since that's not default yet I propose this for backport to 1.0.2 and 1.1. Unfortunately, I actually have a situation where a pre-release xulrunner-devel on my system is preventing CouchDB from compiling. I'll have to open a separate ticket for that, but please forgive me if this patch contains some tiny syntax error. I wanted to file it before I forgot (again).
        Hide
        Filipe Manana added a comment -

        Thanks Randall.

        In fact I would just remove the sessions and pipeline size options from the #http_db record in couch_bd.hrl. They're useless there.

        Can you update the patch?

        thanks

        Show
        Filipe Manana added a comment - Thanks Randall. In fact I would just remove the sessions and pipeline size options from the #http_db record in couch_bd.hrl. They're useless there. Can you update the patch? thanks
        Hide
        Filipe Manana added a comment -

        Looking more carefully at this, the setup of the number of http connections and pipeline size should be done in couch_rep_httpc:db_exists/3, which is called by couch_rep:open_db/3. Conceptually it's a more appropriate place to do it and it applies the settings either to a source or remote database (or both).

        Also, removed those settings from the #http_db record definition, to avoid setting new pipeline values on every request the replicator sends.

        I also noticed now a small typo in your patch Randall:

        + ("replicator", "max_http_piple_size", MaxPipe) ->

        If nobody has an objection against the proposed patch, I'll commit it by tomorrow.

        cheers

        Show
        Filipe Manana added a comment - Looking more carefully at this, the setup of the number of http connections and pipeline size should be done in couch_rep_httpc:db_exists/3, which is called by couch_rep:open_db/3. Conceptually it's a more appropriate place to do it and it applies the settings either to a source or remote database (or both). Also, removed those settings from the #http_db record definition, to avoid setting new pipeline values on every request the replicator sends. I also noticed now a small typo in your patch Randall: + ("replicator", "max_http_piple_size", MaxPipe) -> If nobody has an objection against the proposed patch, I'll commit it by tomorrow. cheers
        Hide
        Randall Leeds added a comment -

        Looks great. No complaints.
        +1 on committing to both 1.0.2 and 1.1.

        Show
        Randall Leeds added a comment - Looks great. No complaints. +1 on committing to both 1.0.2 and 1.1.
        Hide
        Filipe Manana added a comment -

        Applied to trunk, 1.0.x and 1.1.x.
        Thanks

        Show
        Filipe Manana added a comment - Applied to trunk, 1.0.x and 1.1.x. Thanks

          People

          • Assignee:
            Filipe Manana
            Reporter:
            Randall Leeds
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development