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/08/26 21:27:31 UTC

[GitHub] [couchdb] tonysun83 opened a new pull request #3105: bypass partition query limit for mango

tonysun83 opened a new pull request #3105:
URL: https://github.com/apache/couchdb/pull/3105


   ## Overview
   When partition_query_limit is set for couch_mrview, it limits how many
   docs can be scanned when executing partitioned queries. But this limits
   mango's doc scans internally. This leads to documents not being scanned
   to fulfill a query. This fixes:
   https://github.com/apache/couchdb/issues/2795
   
   <!-- Please give a short brief for the pull request,
        what problem it solves or how it makes things better. -->
   
   ## Testing recommendations
   Unit Test added.
   ## Related Issues or Pull Requests
   
   <!-- If your changes affects multiple components in different
        repositories please put links to those issues or pull requests here.  -->
   
   ## Checklist
   
   - [X] Code is written and works correctly
   - [X] Changes are covered by tests
   - [ ] Any new configurable parameters are documented in `rel/overlay/etc/default.ini`
   - [ ] A PR for documentation changes has been made in https://github.com/apache/couchdb-documentation
   


----------------------------------------------------------------
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

[GitHub] [couchdb] wohali commented on pull request #3105: bypass partition query limit for mango

Posted by GitBox <gi...@apache.org>.
wohali commented on pull request #3105:
URL: https://github.com/apache/couchdb/pull/3105#issuecomment-683009289


   @tonysun83 Would you backport this (mango) fix to 3.x too?


----------------------------------------------------------------
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

[GitHub] [couchdb] wohali commented on a change in pull request #3105: bypass partition query limit for mango

Posted by GitBox <gi...@apache.org>.
wohali commented on a change in pull request #3105:
URL: https://github.com/apache/couchdb/pull/3105#discussion_r478700383



##########
File path: dev/run
##########
@@ -427,7 +427,10 @@ def boot_haproxy(ctx):
 def hack_default_ini(ctx, node, contents):
 
     contents = re.sub(
-        "^\[httpd\]$", "[httpd]\nenable = true", contents, flags=re.MULTILINE,
+        "^\[httpd\]$",
+        "[httpd]\nenable = true",
+        contents,
+        flags=re.MULTILINE,

Review comment:
       Line's too long, required change to make python black happy. It is unrelated but needs to go in because otherwise tests fail. Don't give him grief on this one.




----------------------------------------------------------------
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

[GitHub] [couchdb] tonysun83 commented on pull request #3105: bypass partition query limit for mango

Posted by GitBox <gi...@apache.org>.
tonysun83 commented on pull request #3105:
URL: https://github.com/apache/couchdb/pull/3105#issuecomment-682174100


   thx @wohali , it looks good!


----------------------------------------------------------------
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

[GitHub] [couchdb] davisp commented on a change in pull request #3105: bypass partition query limit for mango

Posted by GitBox <gi...@apache.org>.
davisp commented on a change in pull request #3105:
URL: https://github.com/apache/couchdb/pull/3105#discussion_r478694604



##########
File path: dev/run
##########
@@ -427,7 +427,10 @@ def boot_haproxy(ctx):
 def hack_default_ini(ctx, node, contents):
 
     contents = re.sub(
-        "^\[httpd\]$", "[httpd]\nenable = true", contents, flags=re.MULTILINE,
+        "^\[httpd\]$",
+        "[httpd]\nenable = true",
+        contents,
+        flags=re.MULTILINE,

Review comment:
       What's this change for? Looks like styling which we shouldn't mix.




----------------------------------------------------------------
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

[GitHub] [couchdb] tonysun83 merged pull request #3105: bypass partition query limit for mango

Posted by GitBox <gi...@apache.org>.
tonysun83 merged pull request #3105:
URL: https://github.com/apache/couchdb/pull/3105


   


----------------------------------------------------------------
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

[GitHub] [couchdb] davisp commented on a change in pull request #3105: bypass partition query limit for mango

Posted by GitBox <gi...@apache.org>.
davisp commented on a change in pull request #3105:
URL: https://github.com/apache/couchdb/pull/3105#discussion_r478536289



##########
File path: test/elixir/test/partition_mango_test.exs
##########
@@ -502,6 +502,9 @@ defmodule PartitionMangoTest do
     create_partition_docs(db_name)
     create_index(db_name, ["value"])
 
+    # this is to test that we bypass partition_query_limit for mango
+    set_config({"query_server_config", "partition_query_limit", "1"})
+

Review comment:
       Lets create a specific test for this rather than mixing our assertions.

##########
File path: src/mango/src/mango_cursor_view.erl
##########
@@ -116,7 +116,8 @@ base_args(#cursor{index = Idx, selector = Selector} = Cursor) ->
         start_key = StartKey,
         end_key = EndKey,
         include_docs = true,
-        extra = [{callback, {?MODULE, view_cb}}, {selector, Selector}]
+        extra = [{callback, {?MODULE, view_cb}}, {selector, Selector},

Review comment:
       Style nit, but make this multi-lined:
   
   ```erlang
   extra = [
       {callback, {?MODULE, view_cb}},
       {selector, Selector},
       {ignore_partition_query_limit, true}
   ]
   ```




----------------------------------------------------------------
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

[GitHub] [couchdb] tonysun83 commented on pull request #3105: bypass partition query limit for mango

Posted by GitBox <gi...@apache.org>.
tonysun83 commented on pull request #3105:
URL: https://github.com/apache/couchdb/pull/3105#issuecomment-682148612


   @wohali CI seems to be failing with `python-black` issues. I'm not sure if it's related to this PR as I have not made any changes to python files. I've rebuilt it a few times already with no luck. See anything I'm missing?


----------------------------------------------------------------
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

[GitHub] [couchdb] davisp commented on a change in pull request #3105: bypass partition query limit for mango

Posted by GitBox <gi...@apache.org>.
davisp commented on a change in pull request #3105:
URL: https://github.com/apache/couchdb/pull/3105#discussion_r478708609



##########
File path: dev/run
##########
@@ -427,7 +427,10 @@ def boot_haproxy(ctx):
 def hack_default_ini(ctx, node, contents):
 
     contents = re.sub(
-        "^\[httpd\]$", "[httpd]\nenable = true", contents, flags=re.MULTILINE,
+        "^\[httpd\]$",
+        "[httpd]\nenable = true",
+        contents,
+        flags=re.MULTILINE,

Review comment:
       Ah! Fair enough.




----------------------------------------------------------------
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

[GitHub] [couchdb] wohali commented on pull request #3105: bypass partition query limit for mango

Posted by GitBox <gi...@apache.org>.
wohali commented on pull request #3105:
URL: https://github.com/apache/couchdb/pull/3105#issuecomment-682157191


   Hey @tonysun83 ,
   
   ```
   [2020-08-27T19:38:24.347Z] would reformat /home/****/workspace/****-cm1_PullRequests_PR-3105/20.3.8.11/build/apache-couchdb-3.0.0-7dbd0ad/dev/run
   ```
   
   Looking at the commit history, `dev/run` got changed 5 months ago, and my change to ensure black was run across all files dates from April, and commits have passed since then. So this must be related to a new check in python black.
   
   If you run `make python-black-update` you can fix the problem, then commit the change it makes to this PR.
   
   I realize this, strictly speaking, is an unrelated change to fixing mango, but your help is greatly appreciated :) and a single PR is faster than 2.


----------------------------------------------------------------
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