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 2022/09/09 20:35:29 UTC
[GitHub] [couchdb] nickva opened a new pull request, #4172: Give the users the option to disable bulk_get attempts
nickva opened a new pull request, #4172:
URL: https://github.com/apache/couchdb/pull/4172
Let users have the option to revert to the previous behavior. They may have some odd load balancer setup, or a custom API implementation where repeated _bulk_get attempts may cause unexpected issues.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] jaydoane commented on a diff in pull request #4172: Give the users the option to disable bulk_get attempts
Posted by GitBox <gi...@apache.org>.
jaydoane commented on code in PR #4172:
URL: https://github.com/apache/couchdb/pull/4172#discussion_r967681064
##########
src/couch_replicator/test/eunit/couch_replicator_bulk_get_tests.erl:
##########
@@ -0,0 +1,74 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_replicator_bulk_get_tests).
+
+-include_lib("couch/include/couch_eunit.hrl").
+-include_lib("couch/include/couch_db.hrl").
+-include("couch_replicator_test.hrl").
+
+-define(DOCS, 10).
Review Comment:
Might be clearer as `DOC_COUNT`?
##########
rel/overlay/etc/default.ini:
##########
@@ -494,6 +494,8 @@ partitioned||* = true
;retries_per_request = 5
; Use checkpoints
;use_checkpoints = true
+; Attempt to use bulk_get for fetching documents from the source
+;use_bulk_get = true
Review Comment:
I suppose another approach would be to change the default to `false` for now to keep existing behavior, but switch it to `true` for a major version change?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a diff in pull request #4172: Give the users the option to disable bulk_get attempts
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4172:
URL: https://github.com/apache/couchdb/pull/4172#discussion_r967702523
##########
src/couch_replicator/test/eunit/couch_replicator_bulk_get_tests.erl:
##########
@@ -0,0 +1,74 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_replicator_bulk_get_tests).
+
+-include_lib("couch/include/couch_eunit.hrl").
+-include_lib("couch/include/couch_db.hrl").
+-include("couch_replicator_test.hrl").
+
+-define(DOCS, 10).
Review Comment:
Updated the PR accordingly
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on pull request #4172: Give the users the option to disable bulk_get attempts
Posted by GitBox <gi...@apache.org>.
nickva commented on PR #4172:
URL: https://github.com/apache/couchdb/pull/4172#issuecomment-1242804148
@jaydoane updated the PR with two more tests: one checks that a global disable and a per-job enable works, and the other checks that global disable works and overrides the code default.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a diff in pull request #4172: Give the users the option to disable bulk_get attempts
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4172:
URL: https://github.com/apache/couchdb/pull/4172#discussion_r967689389
##########
src/couch_replicator/test/eunit/couch_replicator_bulk_get_tests.erl:
##########
@@ -0,0 +1,74 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_replicator_bulk_get_tests).
+
+-include_lib("couch/include/couch_eunit.hrl").
+-include_lib("couch/include/couch_db.hrl").
+-include("couch_replicator_test.hrl").
+
+-define(DOCS, 10).
Review Comment:
Good idea
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a diff in pull request #4172: Give the users the option to disable bulk_get attempts
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4172:
URL: https://github.com/apache/couchdb/pull/4172#discussion_r967689346
##########
rel/overlay/etc/default.ini:
##########
@@ -494,6 +494,8 @@ partitioned||* = true
;retries_per_request = 5
; Use checkpoints
;use_checkpoints = true
+; Attempt to use bulk_get for fetching documents from the source
+;use_bulk_get = true
Review Comment:
Since there is a fallback already, in most cases it should work, and this would be for more exceptions or in rare situations.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva merged pull request #4172: Give the users the option to disable bulk_get attempts
Posted by GitBox <gi...@apache.org>.
nickva merged PR #4172:
URL: https://github.com/apache/couchdb/pull/4172
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on pull request #4172: Give the users the option to disable bulk_get attempts
Posted by GitBox <gi...@apache.org>.
nickva commented on PR #4172:
URL: https://github.com/apache/couchdb/pull/4172#issuecomment-1242787386
> It seems like there are two ways to configure a replication to use _bulk_get: one globally via the config option `[replicator] use_bulk_get` and the other as a parameter in the replication doc itself. Is that correct? If so, which takes precedence? It looks like the latter does, but it's not super clear from reading the code. Should we add a test that makes the precedence explicit?
The idea is to follow the same pattern as the rest of the replication jobs settings. For instance the existing “use_checkpoints” setting.
The precedence is the same as the other settings. The global default is in the ini file and the replication job may override it.
Good idea to add a test for it.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org