You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@couchdb.apache.org by mikewallace1979 <gi...@git.apache.org> on 2015/06/02 19:33:49 UTC

[GitHub] couchdb-couch-replicator pull request: 2707 merge couch replicator...

GitHub user mikewallace1979 opened a pull request:

    https://github.com/apache/couchdb-couch-replicator/pull/11

    2707 merge couch replicator fixes from cloudant fork

    This PR brings in a number of fixes for couch_replicator from the Cloudant fork at https://github.com/cloudant/couch_replicator
    
    Closes COUCHDB-2707.

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

    $ git pull https://github.com/apache/couchdb-couch-replicator 2707-merge-couch_replicator-fixes-from-cloudant-fork

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

    https://github.com/apache/couchdb-couch-replicator/pull/11.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 #11
    
----
commit dcb38869bdb1ed44bb90ed45bc993a6452be1cb6
Author: Robert Newson <rn...@apache.org>
Date:   2014-12-01T11:11:00Z

    Use randomized, truncated exponential backoff in event of conflict
    
    BugzID: 42053
    
    This is a cherry-pick of:
    
    https://github.com/cloudant/couch_replicator/commit/6e8fbb6a3f2622c14ae605c18ec54cbad7d389f3
    
    Conflicts:
    	src/couch_replicator_manager.erl

commit 2e6d5c1a6a74629509c247b620a877ddfce1daf5
Author: Robert Newson <rn...@apache.org>
Date:   2014-12-03T11:30:51Z

    Remove anonymous fun when starting replications
    
    This is a cherry-pick of:
    
    https://github.com/cloudant/couch_replicator/commit/faa28a6e7f5b460b1d3ca2f77b00ab7d5371021d

commit 74da56e5d396a7bc739c1ebb50e6d98a52dac858
Author: Robert Newson <rn...@apache.org>
Date:   2014-12-03T11:41:48Z

    Verify that url really points to a database
    
    This is a cherry-pick of:
    
    https://github.com/cloudant/couch_replicator/commit/e73714196936c345d54158e674ab36cab20beeec

commit ea39d118a08781f003b8756dee3324d65038f7a9
Author: Robert Newson <rn...@apache.org>
Date:   2014-12-03T12:14:10Z

    delay and splay replication starts
    
    This is a cherry-pick of:
    
    https://github.com/cloudant/couch_replicator/commit/d279150d959cfd46cbd77c5dd17f14d6dc3d0291

commit dab36db0c829ca391b774181cd22aa140852335c
Author: Robert Newson <rn...@apache.org>
Date:   2015-05-13T18:40:55Z

    Ensure Live node set is consistent with up/down messages
    
    BugzID: 46617
    
    This is a cherry-pick of:
    
    https://github.com/cloudant/couch_replicator/commit/2418c26b0fa7cffb97c2d8348654c42d6a0f1a06
    
    Conflicts:
    	src/couch_replicator_manager.erl

commit 5dc5c415a570d90918814eaa743747faf449d71d
Author: Robert Newson <rn...@apache.org>
Date:   2015-05-15T15:30:34Z

    Return owner to improve logging output
    
    This is a cherry-pick of:
    
    https://github.com/cloudant/couch_replicator/commit/3287da36a24b5d061a64cc93814f2f4580fdd4f9
    
    Conflicts:
    	src/couch_replicator_manager.erl

commit 36757640ea892b0f083f6584172b3e0dc16857ad
Author: Robert Newson <rn...@apache.org>
Date:   2015-05-15T15:30:55Z

    Log when node up/down events occur
    
    This is a cherry-pick of:
    
    https://github.com/cloudant/couch_replicator/commit/d40bb6f2e603a7c81f777cc1c4c200ad34c3db42

commit 44e755e5108262877c2aad48f3846e7139c877b7
Author: Robert Newson <rn...@apache.org>
Date:   2015-05-15T15:31:24Z

    Cleanly stop replication at checkpoint time if no longer owner
    
    This is a cherry-pick of:
    
    https://github.com/cloudant/couch_replicator/commit/e5ef7c8a0ee2566b9cd4c02397ee94883d015fa0

commit 89d316e51d49c74b6afe5fc5c7de64c77d299df2
Author: Paul J. Davis <pa...@gmail.com>
Date:   2015-05-20T22:04:31Z

    Ensure that ibrowse streams are ended properly
    
    I found a situation where we had live lock on a running application due
    to an ibrowse request that hadn't been properly terminated. This
    manifested as a cesation of updates to the _active_tasks information.
    Debugging this lead me to see that the main couch_replicator pid was
    stuck on a call to get_pending_changes. This call was stuck because the
    ibrowse_http_client process being used was stuck waiting for a changes
    request to complete.
    
    This changes request as it turns out had been abandoned by the
    couch_replicator_changes_reader. The changes reader was then stuck
    trying to do a gen_server:call/2 back to the main couch_replicator
    process with the report_seq_done message.
    
    Given all this, it became apparent that the changes feed improperly
    ending its ibrowse streams was the underlying culprit. Issuing a call to
    ibrowse:stream_next/1 with the abandoned ibrowse stream id resulted in
    the replication resuming.
    
    This bug was introduced in this commit:
    bfa020b43be20c54ab166c51f5c6e55c34d844c2
    
    BugzId: 47306
    
    This is a cherry-pick of:
    
    https://github.com/cloudant/couch_replicator/commit/f9db37a9b293f5f078681e7539fd35a92eb3adec

commit c595e5721a7b9483554d578e9891426ad4609f8d
Author: Paul J. Davis <pa...@gmail.com>
Date:   2015-05-20T22:26:59Z

    Be more explicit on values of ?STREAM_STATUS
    
    Also I should add a note about how the changes ending due to a throw
    when processing the last_seq leads to the un-consumed stream messages.
    
    This is a cherry-pick of:
    
    https://github.com/cloudant/couch_replicator/commit/20d11c7d342ea77ffd5384d75e9cd570cbcbf5ba

commit 508460488656e171ac8a020a9f72d949444cd225
Author: Paul J. Davis <pa...@gmail.com>
Date:   2015-05-21T15:02:53Z

    Fix stream cleanup timeouts
    
    The first part of this is adding the `after 0` clause. The issue here is
    that ibrowse sends the `ibrowse_async_response_end` message without
    waiting for a call to `ibrowse:stream_next/1`. This means that the
    continuous changes feed may or may not get this message in
    `couch_replicator_httpc:accumulate_messages/3`. If it does then we would
    end up on an infinite timeout waiting for it. This was a typo in the
    original patch in that I meant to include it but forgot.
    
    The second timeout is so that we don't end up halted waiting for a
    changes request to finish. If it takes longer than 30s we just crash the
    replication and let the manager restart things.
    
    BugzId: 47306
    
    This is a cherry-pick of:
    
    https://github.com/cloudant/couch_replicator/commit/2caf39040e4e50c38a7758d4d09e7a8b22ea92d4

commit 9259784d589bd27f54ae388355e0e78e5be413ac
Author: Robert Newson <rn...@apache.org>
Date:   2015-05-21T13:08:42Z

    continue jobs that aren't _replicator docs
    
    This is a cherry-pick of:
    
    https://github.com/cloudant/couch_replicator/commit/ff1ab1b840019601c3e3e04a1d931db6f2ccd2d1

commit 367562b4120a4ff0ba1a01dfa96534cf8d02d881
Author: Robert Newson <rn...@apache.org>
Date:   2015-05-26T15:04:24Z

    Infinity timeout, just like all the others :(
    
    This is a cherry-pick of:
    
    https://github.com/cloudant/couch_replicator/commit/e947b392db1eb2de22aac4a4fa12da118fe114b3

----


---
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-couch-replicator pull request: 2707 merge couch replicator...

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

    https://github.com/apache/couchdb-couch-replicator/pull/11#discussion_r31550867
  
    --- Diff: src/couch_replicator_api_wrap.erl ---
    @@ -83,9 +83,16 @@ db_open(#httpdb{} = Db1, _Options, Create) ->
                         ok
                     end)
             end,
    -        send_req(Db, [{method, head}],
    -            fun(200, _, _) ->
    -                {ok, Db};
    +        send_req(Db, [{method, get}],
    +            fun(200, _, {Props}) ->
    +                case get_value(<<"update_seq">>, Props) of
    --- End diff --
    
    I know that, but assume that remote Source/Target is not a CouchDB. Then, this check ensures that remote is database by `update_seq` presence, runs replication and fill logs full of [out of sync](https://github.com/apache/couchdb-couch-replicator/blob/master/src/couch_replicator.erl#L777-L785) errors because `instance_start_time` also returns by `/_ensure_full_commit` endpoint what is [mandotarry](https://github.com/apache/couchdb-couch-replicator/blob/master/src/couch_replicator_api_wrap.erl#L184-L188). 


---
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-couch-replicator pull request: 2707 merge couch replicator...

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

    https://github.com/apache/couchdb-couch-replicator/pull/11#discussion_r31819938
  
    --- Diff: src/couch_replicator_api_wrap.erl ---
    @@ -83,9 +83,16 @@ db_open(#httpdb{} = Db1, _Options, Create) ->
                         ok
                     end)
             end,
    -        send_req(Db, [{method, head}],
    -            fun(200, _, _) ->
    -                {ok, Db};
    +        send_req(Db, [{method, get}],
    +            fun(200, _, {Props}) ->
    +                case get_value(<<"update_seq">>, Props) of
    --- End diff --
    
    @kxepal seems like a sensible check - I added a commit to the PR


---
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-couch-replicator pull request: 2707 merge couch replicator...

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

    https://github.com/apache/couchdb-couch-replicator/pull/11#discussion_r31550497
  
    --- Diff: src/couch_replicator_api_wrap.erl ---
    @@ -83,9 +83,16 @@ db_open(#httpdb{} = Db1, _Options, Create) ->
                         ok
                     end)
             end,
    -        send_req(Db, [{method, head}],
    -            fun(200, _, _) ->
    -                {ok, Db};
    +        send_req(Db, [{method, get}],
    +            fun(200, _, {Props}) ->
    +                case get_value(<<"update_seq">>, Props) of
    --- End diff --
    
    instance_start_time is always 0 in couchdb 2.0 (because of the multiple nodes involved, if we used the actual start times we'd end up restarting replication continuously).


---
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-couch-replicator pull request: 2707 merge couch replicator...

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

    https://github.com/apache/couchdb-couch-replicator/pull/11


---
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-couch-replicator pull request: 2707 merge couch replicator...

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

    https://github.com/apache/couchdb-couch-replicator/pull/11#discussion_r31571434
  
    --- Diff: src/couch_replicator_api_wrap.erl ---
    @@ -83,9 +83,16 @@ db_open(#httpdb{} = Db1, _Options, Create) ->
                         ok
                     end)
             end,
    -        send_req(Db, [{method, head}],
    -            fun(200, _, _) ->
    -                {ok, Db};
    +        send_req(Db, [{method, get}],
    +            fun(200, _, {Props}) ->
    +                case get_value(<<"update_seq">>, Props) of
    --- End diff --
    
    Broken checkpoints is just a side effect of missing that field in the response. As like as if `update_seq` will be missed, replication will start and fail with some error in the middle. That's why both are required for clean start.


---
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-couch-replicator pull request: 2707 merge couch replicator...

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

    https://github.com/apache/couchdb-couch-replicator/pull/11#issuecomment-109324320
  
    +1 Don't forget squash 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-couch-replicator pull request: 2707 merge couch replicator...

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

    https://github.com/apache/couchdb-couch-replicator/pull/11#discussion_r31549741
  
    --- Diff: src/couch_replicator_api_wrap.erl ---
    @@ -83,9 +83,16 @@ db_open(#httpdb{} = Db1, _Options, Create) ->
                         ok
                     end)
             end,
    -        send_req(Db, [{method, head}],
    -            fun(200, _, _) ->
    -                {ok, Db};
    +        send_req(Db, [{method, get}],
    +            fun(200, _, {Props}) ->
    +                case get_value(<<"update_seq">>, Props) of
    --- End diff --
    
    This check is not enough as `instance_start_time` is also required. Without it checkpoints will never be made.


---
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-couch-replicator pull request: 2707 merge couch replicator...

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

    https://github.com/apache/couchdb-couch-replicator/pull/11#discussion_r31570119
  
    --- Diff: src/couch_replicator_api_wrap.erl ---
    @@ -83,9 +83,16 @@ db_open(#httpdb{} = Db1, _Options, Create) ->
                         ok
                     end)
             end,
    -        send_req(Db, [{method, head}],
    -            fun(200, _, _) ->
    -                {ok, Db};
    +        send_req(Db, [{method, get}],
    +            fun(200, _, {Props}) ->
    +                case get_value(<<"update_seq">>, Props) of
    --- End diff --
    
    I can't make sense of that. This check is refining the check from 'did I get a 200' to 'did I get a 200 with a json body with an update_seq in it' in order to crash if source or target is not couchdb. It has nothing to do with whether checkpoints would work.


---
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-couch-replicator pull request: 2707 merge couch replicator...

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

    https://github.com/apache/couchdb-couch-replicator/pull/11#discussion_r31548914
  
    --- Diff: src/couch_replicator_manager.erl ---
    @@ -403,13 +442,10 @@ process_update(State, DbName, {Change}) ->
             end
         end.
     
    -owner(<<"shards/", _/binary>> = DbName, DocId) ->
    -    Live = [node()|nodes()],
    +owner(<<"shards/", _/binary>> = DbName, DocId, Live) ->
         Nodes = lists:sort([N || #shard{node=N} <- mem3:shards(mem3:dbname(DbName), DocId),
     			     lists:member(N, Live)]),
    -    node() =:= hd(mem3_util:rotate_list({DbName, DocId}, Nodes));
    -owner(_DbName, _DocId) ->
    --- End diff --
    
    @rnewson @davisp I'm a little concerned by the fact that the second function clause for owner/2 disappears - it doesn't exist over in cloudant/couch_replicator and I wasn't quite sure what the right thing to do here was, as I don't have the context of why the second clause existed here.


---
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-couch-replicator pull request: 2707 merge couch replicator...

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

    https://github.com/apache/couchdb-couch-replicator/pull/11#discussion_r31549393
  
    --- Diff: src/couch_replicator_manager.erl ---
    @@ -403,13 +442,10 @@ process_update(State, DbName, {Change}) ->
             end
         end.
     
    -owner(<<"shards/", _/binary>> = DbName, DocId) ->
    -    Live = [node()|nodes()],
    +owner(<<"shards/", _/binary>> = DbName, DocId, Live) ->
         Nodes = lists:sort([N || #shard{node=N} <- mem3:shards(mem3:dbname(DbName), DocId),
     			     lists:member(N, Live)]),
    -    node() =:= hd(mem3_util:rotate_list({DbName, DocId}, Nodes));
    -owner(_DbName, _DocId) ->
    --- End diff --
    
    which we need...


---
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-couch-replicator pull request: 2707 merge couch replicator...

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

    https://github.com/apache/couchdb-couch-replicator/pull/11#discussion_r31550558
  
    --- Diff: src/couch_replicator_manager.erl ---
    @@ -451,7 +487,14 @@ maybe_start_replication(State, DbName, DocId, RepDoc) ->
             true = ets:insert(?DOC_TO_REP, {{DbName, DocId}, RepId}),
             couch_log:notice("Attempting to start replication `~s` (document `~s`).",
                 [pp_rep_id(RepId), DocId]),
    -        Pid = spawn_link(fun() -> start_replication(Rep, 0) end),
    +        StartDelaySecs = erlang:max(0, list_to_integer(
    +            config:get("replicator", "start_delay", "10"))),
    --- End diff --
    
    nice.


---
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-couch-replicator pull request: 2707 merge couch replicator...

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

    https://github.com/apache/couchdb-couch-replicator/pull/11#discussion_r31549334
  
    --- Diff: src/couch_replicator_manager.erl ---
    @@ -403,13 +442,10 @@ process_update(State, DbName, {Change}) ->
             end
         end.
     
    -owner(<<"shards/", _/binary>> = DbName, DocId) ->
    -    Live = [node()|nodes()],
    +owner(<<"shards/", _/binary>> = DbName, DocId, Live) ->
         Nodes = lists:sort([N || #shard{node=N} <- mem3:shards(mem3:dbname(DbName), DocId),
     			     lists:member(N, Live)]),
    -    node() =:= hd(mem3_util:rotate_list({DbName, DocId}, Nodes));
    -owner(_DbName, _DocId) ->
    --- End diff --
    
    That's 9640ccf2e9182bcc7119c4cbde01714fad71b24a.


---
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-couch-replicator pull request: 2707 merge couch replicator...

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

    https://github.com/apache/couchdb-couch-replicator/pull/11#discussion_r31820430
  
    --- Diff: src/couch_replicator_api_wrap.erl ---
    @@ -83,9 +83,16 @@ db_open(#httpdb{} = Db1, _Options, Create) ->
                         ok
                     end)
             end,
    -        send_req(Db, [{method, head}],
    -            fun(200, _, _) ->
    -                {ok, Db};
    +        send_req(Db, [{method, get}],
    +            fun(200, _, {Props}) ->
    +                case get_value(<<"update_seq">>, Props) of
    --- End diff --
    
    @mikewallace1979 thank you! (:


---
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-couch-replicator pull request: 2707 merge couch replicator...

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

    https://github.com/apache/couchdb-couch-replicator/pull/11#discussion_r31549934
  
    --- Diff: src/couch_replicator_manager.erl ---
    @@ -403,13 +442,10 @@ process_update(State, DbName, {Change}) ->
             end
         end.
     
    -owner(<<"shards/", _/binary>> = DbName, DocId) ->
    -    Live = [node()|nodes()],
    +owner(<<"shards/", _/binary>> = DbName, DocId, Live) ->
         Nodes = lists:sort([N || #shard{node=N} <- mem3:shards(mem3:dbname(DbName), DocId),
     			     lists:member(N, Live)]),
    -    node() =:= hd(mem3_util:rotate_list({DbName, DocId}, Nodes));
    -owner(_DbName, _DocId) ->
    --- End diff --
    
    Darn. So would the right thing be to return `node()` now?


---
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-couch-replicator pull request: 2707 merge couch replicator...

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

    https://github.com/apache/couchdb-couch-replicator/pull/11#discussion_r31550134
  
    --- Diff: src/couch_replicator_manager.erl ---
    @@ -403,13 +442,10 @@ process_update(State, DbName, {Change}) ->
             end
         end.
     
    -owner(<<"shards/", _/binary>> = DbName, DocId) ->
    -    Live = [node()|nodes()],
    +owner(<<"shards/", _/binary>> = DbName, DocId, Live) ->
         Nodes = lists:sort([N || #shard{node=N} <- mem3:shards(mem3:dbname(DbName), DocId),
     			     lists:member(N, Live)]),
    -    node() =:= hd(mem3_util:rotate_list({DbName, DocId}, Nodes));
    -owner(_DbName, _DocId) ->
    --- End diff --
    
    yes, return `node()` 


---
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-couch-replicator pull request: 2707 merge couch replicator...

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the pull request:

    https://github.com/apache/couchdb-couch-replicator/pull/11#issuecomment-108030339
  
    I'm +1 after fixing the owner/2 issue.


---
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-couch-replicator pull request: 2707 merge couch replicator...

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

    https://github.com/apache/couchdb-couch-replicator/pull/11#discussion_r31549841
  
    --- Diff: src/couch_replicator_manager.erl ---
    @@ -451,7 +487,14 @@ maybe_start_replication(State, DbName, DocId, RepDoc) ->
             true = ets:insert(?DOC_TO_REP, {{DbName, DocId}, RepId}),
             couch_log:notice("Attempting to start replication `~s` (document `~s`).",
                 [pp_rep_id(RepId), DocId]),
    -        Pid = spawn_link(fun() -> start_replication(Rep, 0) end),
    +        StartDelaySecs = erlang:max(0, list_to_integer(
    +            config:get("replicator", "start_delay", "10"))),
    --- End diff --
    
    There is nicer helper [config:get_integer(Key, Value, Default)](https://github.com/apache/couchdb-config/blob/master/src/config.erl#L60-L66).


---
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-couch-replicator pull request: 2707 merge couch replicator...

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

    https://github.com/apache/couchdb-couch-replicator/pull/11#discussion_r31609446
  
    --- Diff: src/couch_replicator_api_wrap.erl ---
    @@ -83,9 +83,16 @@ db_open(#httpdb{} = Db1, _Options, Create) ->
                         ok
                     end)
             end,
    -        send_req(Db, [{method, head}],
    -            fun(200, _, _) ->
    -                {ok, Db};
    +        send_req(Db, [{method, get}],
    +            fun(200, _, {Props}) ->
    +                case get_value(<<"update_seq">>, Props) of
    --- End diff --
    
    I'm not quite sure I'm following this. The current PR code is using the presence of `update_seq` as a proxy for "this is a response from an actual database" (as opposed to a response from a resource which returns a 200 but isn't a database at all).
    
    @kxepal Is your suggestion that we should also check for the presence of `instance_start_time` in order to detect the case where a resource happens to provide an `update_seq` but no `instance_start_time`?


---
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-couch-replicator pull request: 2707 merge couch replicator...

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

    https://github.com/apache/couchdb-couch-replicator/pull/11#discussion_r31619297
  
    --- Diff: src/couch_replicator_api_wrap.erl ---
    @@ -83,9 +83,16 @@ db_open(#httpdb{} = Db1, _Options, Create) ->
                         ok
                     end)
             end,
    -        send_req(Db, [{method, head}],
    -            fun(200, _, _) ->
    -                {ok, Db};
    +        send_req(Db, [{method, get}],
    +            fun(200, _, {Props}) ->
    +                case get_value(<<"update_seq">>, Props) of
    --- End diff --
    
    @mikewallace1979 yes, because only these two fields are required for replication process. If the remote peer is a CouchDB database, it will provide both with no questions. If the remote is third party HTTP endpoint which claims to support replication protocol, this check will force it provides required for start information.


---
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.
---