You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by rnewson <gi...@git.apache.org> on 2015/11/12 10:50:36 UTC
[GitHub] couchdb-couch-replicator pull request: Fix couch_replicator_manage...
GitHub user rnewson opened a pull request:
https://github.com/apache/couchdb-couch-replicator/pull/23
Fix couch_replicator_manager rescans
When couch_replicator_manager starts it scans every _replicator database
looking for replications to start. When it starts the replication it
modifies a document in the _replicator database. This change ends up
sending a message back to couch_replicator_manager to rescan the
database. This message to rescan the database had no protection to be
unique. This would result in many processes re-scanning the same
database over and over.
To fix this we track the DbName for every scanning process so that if we
get a change to a database we can ignore the change because a scanner
pid is already running. However we also have to track if we need to
restart the scanning pid when it finishes so that we ensure that we
process any changes that occurred during the scan.
COUCHDB-2878
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/cloudant/couchdb-couch-replicator 2878-fix-rescan
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/couchdb-couch-replicator/pull/23.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 #23
----
commit daea81a617e99d452d09745830c905ed5e277cfe
Author: Paul J. Davis <pa...@gmail.com>
Date: 2015-11-05T00:48:07Z
Fix couch_replicator_manager rescans
When couch_replicator_manager starts it scans every _replicator database
looking for replications to start. When it starts the replication it
modifies a document in the _replicator database. This change ends up
sending a message back to couch_replicator_manager to rescan the
database. This message to rescan the database had no protection to be
unique. This would result in many processes re-scanning the same
database over and over.
To fix this we track the DbName for every scanning process so that if we
get a change to a database we can ignore the change because a scanner
pid is already running. However we also have to track if we need to
restart the scanning pid when it finishes so that we ensure that we
process any changes that occurred during the scan.
COUCHDB-2878
----
---
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: Fix couch_replicator_manage...
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-couch-replicator/pull/23#issuecomment-156075464
LGFM +1
---
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: Fix couch_replicator_manage...
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/23#discussion_r44645271
--- Diff: src/couch_replicator_manager.erl ---
@@ -232,14 +238,28 @@ handle_call(Msg, From, State) ->
{stop, {error, {unexpected_call, Msg}}, State}.
handle_cast({resume_scan, DbName}, State) ->
- Since = case ets:lookup(?DB_TO_SEQ, DbName) of
- [] -> 0;
- [{DbName, EndSeq}] -> EndSeq
+ Pids = State#state.rep_start_pids,
+ NewPids = case lists:keyfind(DbName, 1, Pids) of
+ {DbName, _Pid} ->
+ Entry = case ets:lookup(?DB_TO_SEQ, DbName) of
+ [] ->
+ {DbName, 0, true};
+ [{DbName, EndSeq, _Rescan}] ->
+ {DbName, EndSeq, true}
+ end,
+ true = ets:insert(?DB_TO_SEQ, Entry),
+ Pids;
+ false ->
+ Since = case ets:lookup(?DB_TO_SEQ, DbName) of
+ [] -> 0;
+ [{DbName, EndSeq, _Rescan}] -> EndSeq
+ end,
+ true = ets:insert(?DB_TO_SEQ, {DbName, Since, false}),
+ Pid = start_changes_reader(DbName, Since),
+ couch_log:debug("Scanning ~s from update_seq ~p", [DbName, Since]),
+ [{DbName, Pid} | Pids]
end,
- ensure_rep_ddoc_exists(DbName),
--- End diff --
Dear reader from future! This was fixed, please ignore (:
---
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: Fix couch_replicator_manage...
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/23#discussion_r44643645
--- Diff: src/couch_replicator_manager.erl ---
@@ -232,14 +238,28 @@ handle_call(Msg, From, State) ->
{stop, {error, {unexpected_call, Msg}}, State}.
handle_cast({resume_scan, DbName}, State) ->
- Since = case ets:lookup(?DB_TO_SEQ, DbName) of
- [] -> 0;
- [{DbName, EndSeq}] -> EndSeq
+ Pids = State#state.rep_start_pids,
+ NewPids = case lists:keyfind(DbName, 1, Pids) of
+ {DbName, _Pid} ->
+ Entry = case ets:lookup(?DB_TO_SEQ, DbName) of
+ [] ->
+ {DbName, 0, true};
+ [{DbName, EndSeq, _Rescan}] ->
+ {DbName, EndSeq, true}
+ end,
+ true = ets:insert(?DB_TO_SEQ, Entry),
+ Pids;
+ false ->
+ Since = case ets:lookup(?DB_TO_SEQ, DbName) of
+ [] -> 0;
+ [{DbName, EndSeq, _Rescan}] -> EndSeq
+ end,
+ true = ets:insert(?DB_TO_SEQ, {DbName, Since, false}),
+ Pid = start_changes_reader(DbName, Since),
--- End diff --
move it to here then
---
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: Fix couch_replicator_manage...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/couchdb-couch-replicator/pull/23
---
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: Fix couch_replicator_manage...
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/23#discussion_r44639994
--- Diff: src/couch_replicator_manager.erl ---
@@ -232,14 +238,28 @@ handle_call(Msg, From, State) ->
{stop, {error, {unexpected_call, Msg}}, State}.
handle_cast({resume_scan, DbName}, State) ->
- Since = case ets:lookup(?DB_TO_SEQ, DbName) of
- [] -> 0;
- [{DbName, EndSeq}] -> EndSeq
+ Pids = State#state.rep_start_pids,
+ NewPids = case lists:keyfind(DbName, 1, Pids) of
+ {DbName, _Pid} ->
+ Entry = case ets:lookup(?DB_TO_SEQ, DbName) of
+ [] ->
+ {DbName, 0, true};
+ [{DbName, EndSeq, _Rescan}] ->
+ {DbName, EndSeq, true}
+ end,
+ true = ets:insert(?DB_TO_SEQ, Entry),
+ Pids;
+ false ->
+ Since = case ets:lookup(?DB_TO_SEQ, DbName) of
+ [] -> 0;
+ [{DbName, EndSeq, _Rescan}] -> EndSeq
+ end,
+ true = ets:insert(?DB_TO_SEQ, {DbName, Since, false}),
+ Pid = start_changes_reader(DbName, Since),
+ couch_log:debug("Scanning ~s from update_seq ~p", [DbName, Since]),
+ [{DbName, Pid} | Pids]
end,
- ensure_rep_ddoc_exists(DbName),
--- End diff --
Basically, this means that I can remove _design/_replicator in any time and create malformed replication docs with any structure. Causing something like this:
```
{
"_id": "ebb651e46435ca60243060583b000234",
"_rev": "2-0f311c3a2590344c2c37f3e87b4f6ffd",
"owner": null,
"_replication_state": "error",
"_replication_state_time": "2015-11-12T13:05:36+03:00",
"_replication_state_reason": "{error,function_clause}"
}
```
Or even worse - I can break replication status workflow in any way, everyone may edit every docs, everything is ruined. We shouldn't let this happened.
---
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: Fix couch_replicator_manage...
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/23#discussion_r44638958
--- Diff: src/couch_replicator_manager.erl ---
@@ -232,14 +238,28 @@ handle_call(Msg, From, State) ->
{stop, {error, {unexpected_call, Msg}}, State}.
handle_cast({resume_scan, DbName}, State) ->
- Since = case ets:lookup(?DB_TO_SEQ, DbName) of
- [] -> 0;
- [{DbName, EndSeq}] -> EndSeq
+ Pids = State#state.rep_start_pids,
+ NewPids = case lists:keyfind(DbName, 1, Pids) of
+ {DbName, _Pid} ->
+ Entry = case ets:lookup(?DB_TO_SEQ, DbName) of
+ [] ->
+ {DbName, 0, true};
+ [{DbName, EndSeq, _Rescan}] ->
+ {DbName, EndSeq, true}
+ end,
+ true = ets:insert(?DB_TO_SEQ, Entry),
+ Pids;
+ false ->
+ Since = case ets:lookup(?DB_TO_SEQ, DbName) of
+ [] -> 0;
+ [{DbName, EndSeq, _Rescan}] -> EndSeq
+ end,
+ true = ets:insert(?DB_TO_SEQ, {DbName, Since, false}),
+ Pid = start_changes_reader(DbName, Since),
+ couch_log:debug("Scanning ~s from update_seq ~p", [DbName, Since]),
+ [{DbName, Pid} | Pids]
end,
- ensure_rep_ddoc_exists(DbName),
--- End diff --
Won't this hurt us? Other cases when we do ensure in this is on init() and on db create event. So if ddoc will be removed after creation, we loose him until next restart.
---
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.
---