Stop dreyfus_index processes on ddoc update #21
Conversation
Currently when ddoc is modified, dreyfus_index and dreyfus_index_updater
processes corresponding to the previous version of ddoc will keep running
until all indexing processing initiated by them is done.
When ddoc of a big database is rapidly modified, this puts a lot
of unnecessary strain on database resources.
This commit brings the following changes:
1. When opening a dreyfus_index, always add a record
{DbName, {DDocId, Sig}} to ?BY_DB.
2. When ddoc_updated, check if there other ddocs in ?BY_DB with the same Sig.
If there are other, only remove {DbName, {DDocId, Sig}}
record from ?BY_DB for this ddoc.
If there are no, stop dreyfus_index processes:
* all dreyfus_index processes for the prev. version of ddoc will be shutdown
* all linked to them dreyfus_index_updater processes will die as well
* all waiters for indexing activity to be finished will receive an immediate
reply: ddoc_updated. Interactive query requests will get response:
{404, <<"not_found">>, <<"Design document was updated or deleted.">>}
BugzID: 85718
src/dreyfus_index_manager.erl
Outdated
| end | ||
| end, ets:match_object(?BY_DB, {DbShard, {DDocId, '_'}})) | ||
| end, DbShards), | ||
| {ok, nil}; |
There was a problem hiding this comment.
It is confusing to return nil. I think it is better to return St as we do in the next clause.
src/dreyfus_index.erl
Outdated
| handle_cast(_Msg, State) -> | ||
| {noreply, State}. | ||
|
|
||
|
|
src/dreyfus_index.erl
Outdated
| @@ -214,8 +234,14 @@ handle_info({'DOWN',_,_,Pid,Reason}, #state{ | |||
| [gen_server:reply(P, {error, Reason}) || {P, _} <- WaitList], | |||
| {stop, normal, State}. | |||
|
|
|||
| terminate(_Reason, _State) -> | |||
There was a problem hiding this comment.
maybe rewrite this as
terminate({shutdown, ddoc_updated}, State) ->
Waiters = State#state.waiting_list,
[gen_server:reply(From, ddoc_updated) || {From, _} <- Waiters];
termiante(_Reason, _State) ->
ok.
src/dreyfus_index.erl
Outdated
|
|
||
| handle_cast({ddoc_updated, DDocResult}, #state{} = State) -> | ||
| #index{sig = Sig} = State#state.index, | ||
| KeepIndex = case DDocResult of |
There was a problem hiding this comment.
Should this be called KeepIndexes? If I understand correctly, if one index in the new design doc has the same signature, then we don't kill the existing indexes. So if I had 1000 indexes, and I update 999 of them with a new definition (and thus a new signature), but only 1 still was unchanged, then that would mean 999 old indexes would still keep running right?
There was a problem hiding this comment.
@tonysun83 KeepIndex here means if we should keep the current dreyfus_index process or kill it. It refers to a current single dreyfus_index process. May be a better name could be KeepIndexProcess?
There was a problem hiding this comment.
@tonysun83 The intention is to shutdown all changed indexes, so in your case all 999 old indexes will be killed
There was a problem hiding this comment.
@mayya-sharipova : understand now, had to re-read it again, not I got it
| @@ -86,6 +90,9 @@ handle_call({open_error, DbName, Sig, Error}, {OpenerPid, _}, State) -> | |||
|
|
|||
| handle_cast({cleanup, DbName}, State) -> | |||
| clouseau_rpc:cleanup(DbName), | |||
| {noreply, State}; | |||
There was a problem hiding this comment.
@tonysun83 Are you referring to the line 93? If yes, we did not add this here, it was here before - this a standard way to finish handle_cast. This line has changed as we added a new handle_cast clause.
BugzID: 85718
|
Looks pretty good. I saw in the tests that deletion was the method to see if index processes were killed or updated. Can one we add one test where we add a design doc with two index names, then update the definition for one index, and check to see that the old process with the old signature is killed? That coverage would be great. |
BugzID: 85718
|
LGTM, +1 |
Overview
Currently when ddoc is modified, dreyfus_index and dreyfus_index_updater
processes corresponding to the previous version of ddoc will keep running
until all indexing processing initiated by them is done.
When ddoc of a big database is rapidly modified, this puts a lot
of unnecessary strain on database resources.
This commit brings the following changes:
{DbName, {DDocId, Sig}}to?BY_DB.?BY_DBwith the same Sig.If there are other, only remove
{DbName, {DDocId, Sig}}record from
?BY_DBfor this ddoc.If there are no, stop dreyfus_index processes:
reply: ddoc_updated. Interactive query requests will get response:
{404, <<"not_found">>, <<"Design document was updated or deleted.">>}
Testing recommendations
make check apps=dreyfus tests=ddoc_update_test_GitHub issue number
This is to rewrite: #16
To follow the same style as in: apache/couchdb@2e7ca45
BugzID: 85718