You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by GitBox <gi...@apache.org> on 2020/01/15 22:57:37 UTC
[GitHub] [couchdb] nickva opened a new pull request #2458: Fix fabric worker
failures for partition requests
nickva opened a new pull request #2458: Fix fabric worker failures for partition requests
URL: https://github.com/apache/couchdb/pull/2458
### Overview
Previously any failed node or rexi worker error resulted in requests failing
immediately even though there were available workers to keep handling the
request. This was because the progress check function didn't account for the
fact that partition requests only use a handful of shards which, by design, do
not complete the full ring.
Here we fix both partition info queries and dreyfus search functionality. We
follow the pattern from fabric and pass through a set of "ring options" that
let the progress function know it is dealing with partitions instead of a full
ring.
## Testing recommendations
make eunit
### Checklist
- [x] Code is written and works correctly
- [x] Changes are covered by tests
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [couchdb] nickva commented on a change in pull request #2458: Fix
fabric worker failures for partition requests
Posted by GitBox <gi...@apache.org>.
nickva commented on a change in pull request #2458: Fix fabric worker failures for partition requests
URL: https://github.com/apache/couchdb/pull/2458#discussion_r367173914
##########
File path: src/dreyfus/src/dreyfus_util.erl
##########
@@ -59,6 +59,15 @@ use_ushards(#index_query_args{stable=true}) ->
use_ushards(#index_query_args{}) ->
false.
+
+get_ring_opts(#index_query_args{partition = nil}, _Shards) ->
+ [];
+get_ring_opts(#index_query_args{}, Shards) ->
+ Shards1 = lists:map(fun(#shard{} = S) ->
+ S#shard{ref = undefined}
Review comment:
Since we want to match those shards exactly we set the worker references to `undefined`.
In `fabric_db_partition` we get them directly from mem:shards(...) so they won't have references set. In case of dreyfus in one case we get the shards from the workers fabric_dict which have references so we clear them out in `get_ring_opts`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [couchdb] chewbranca commented on a change in pull request #2458:
Fix fabric worker failures for partition requests
Posted by GitBox <gi...@apache.org>.
chewbranca commented on a change in pull request #2458: Fix fabric worker failures for partition requests
URL: https://github.com/apache/couchdb/pull/2458#discussion_r367169485
##########
File path: src/dreyfus/src/dreyfus_util.erl
##########
@@ -59,6 +59,15 @@ use_ushards(#index_query_args{stable=true}) ->
use_ushards(#index_query_args{}) ->
false.
+
+get_ring_opts(#index_query_args{partition = nil}, _Shards) ->
+ [];
+get_ring_opts(#index_query_args{}, Shards) ->
+ Shards1 = lists:map(fun(#shard{} = S) ->
+ S#shard{ref = undefined}
Review comment:
What's the purpose of clearing the refs here for _only_ the dreyfus shards? I haven't looked exhaustively, but I didn't see a reason as to why you would not want to clear out the ref in the list of shards for Dreyfus, but below in `fabric_db_partition_info.erl` you're basically doing `[{any, mem3:shards(DbName, couch_partition:shard_key(Partition))}]`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [couchdb] nickva merged pull request #2458: Fix fabric worker
failures for partition requests
Posted by GitBox <gi...@apache.org>.
nickva merged pull request #2458: Fix fabric worker failures for partition requests
URL: https://github.com/apache/couchdb/pull/2458
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [couchdb] nickva commented on a change in pull request #2458: Fix
fabric worker failures for partition requests
Posted by GitBox <gi...@apache.org>.
nickva commented on a change in pull request #2458: Fix fabric worker failures for partition requests
URL: https://github.com/apache/couchdb/pull/2458#discussion_r367174966
##########
File path: src/dreyfus/src/dreyfus_util.erl
##########
@@ -59,6 +59,15 @@ use_ushards(#index_query_args{stable=true}) ->
use_ushards(#index_query_args{}) ->
false.
+
+get_ring_opts(#index_query_args{partition = nil}, _Shards) ->
+ [];
+get_ring_opts(#index_query_args{}, Shards) ->
+ Shards1 = lists:map(fun(#shard{} = S) ->
+ S#shard{ref = undefined}
Review comment:
Re: `_all_docs` there a `fabric_streams` abstraction used by those types of requests and there `RingOpts` is already passed in
https://github.com/apache/couchdb/blob/master/src/fabric/src/fabric_view_all_docs.erl#L26
https://github.com/apache/couchdb/blob/master/src/fabric/src/fabric_streams.erl#L49
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [couchdb] nickva commented on a change in pull request #2458: Fix
fabric worker failures for partition requests
Posted by GitBox <gi...@apache.org>.
nickva commented on a change in pull request #2458: Fix fabric worker failures for partition requests
URL: https://github.com/apache/couchdb/pull/2458#discussion_r367174966
##########
File path: src/dreyfus/src/dreyfus_util.erl
##########
@@ -59,6 +59,15 @@ use_ushards(#index_query_args{stable=true}) ->
use_ushards(#index_query_args{}) ->
false.
+
+get_ring_opts(#index_query_args{partition = nil}, _Shards) ->
+ [];
+get_ring_opts(#index_query_args{}, Shards) ->
+ Shards1 = lists:map(fun(#shard{} = S) ->
+ S#shard{ref = undefined}
Review comment:
Re: `_all_docs` there is a `fabric_streams` abstraction used by those types of requests and there `RingOpts` is already passed in
https://github.com/apache/couchdb/blob/master/src/fabric/src/fabric_view_all_docs.erl#L26
https://github.com/apache/couchdb/blob/master/src/fabric/src/fabric_streams.erl#L49
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services