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