You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by chewbranca <gi...@git.apache.org> on 2016/02/25 21:03:48 UTC
[GitHub] couchdb-couch pull request: Conditionally use fetch and ddoc_cache...
GitHub user chewbranca opened a pull request:
https://github.com/apache/couchdb-couch/pull/144
Conditionally use fetch and ddoc_cache logic
This fixes issues seen in https://github.com/apache/couchdb-couch-replicator/pull/26.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/couchdb-couch 2938-fix-5986-filtered-changes
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/couchdb-couch/pull/144.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #144
----
commit 9171d5a8f043f7108c148c2a495ed1a4514b1510
Author: Russell Branca <ch...@apache.org>
Date: 2016-02-25T19:47:13Z
Conditionally use fetch and ddoc_cache logic
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Conditionally use fetch and ddoc_cache...
Posted by eiri <gi...@git.apache.org>.
Github user eiri commented on the pull request:
https://github.com/apache/couchdb-couch/pull/144#issuecomment-189031083
+1 :shipit:
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Conditionally use fetch and ddoc_cache...
Posted by chewbranca <gi...@git.apache.org>.
Github user chewbranca commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/144#discussion_r54172209
--- Diff: src/couch_changes.erl ---
@@ -339,15 +348,8 @@ check_docids(_) ->
open_ddoc(#db{name=DbName, id_tree=undefined}, DDocId) ->
- {_, Ref} = spawn_monitor(fun() ->
- exit(fabric:open_doc(mem3:dbname(DbName), DDocId, [ejson_body]))
- end),
- receive
- {'DOWN', Ref, _, _, {ok, _}=Response} ->
- Response;
- {'DOWN', Ref, _, _, Response} ->
- throw(Response)
- end;
+ {ok, DDoc} = ddoc_cache:open_doc(mem3:dbname(DbName), DDocId),
--- End diff --
Sure, I usually like to be explicit about it, but switching to `{ok, _DDoc}` is fine with me.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Conditionally use fetch and ddoc_cache...
Posted by chewbranca <gi...@git.apache.org>.
Github user chewbranca commented on the pull request:
https://github.com/apache/couchdb-couch/pull/144#issuecomment-188956027
Companion PR out in https://github.com/apache/couchdb-chttpd/pull/103.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Conditionally use fetch and ddoc_cache...
Posted by eiri <gi...@git.apache.org>.
Github user eiri commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/144#discussion_r54163884
--- Diff: src/couch_changes.erl ---
@@ -339,15 +348,8 @@ check_docids(_) ->
open_ddoc(#db{name=DbName, id_tree=undefined}, DDocId) ->
- {_, Ref} = spawn_monitor(fun() ->
- exit(fabric:open_doc(mem3:dbname(DbName), DDocId, [ejson_body]))
- end),
- receive
- {'DOWN', Ref, _, _, {ok, _}=Response} ->
- Response;
- {'DOWN', Ref, _, _, Response} ->
- throw(Response)
- end;
+ {ok, DDoc} = ddoc_cache:open_doc(mem3:dbname(DbName), DDocId),
--- End diff --
You don't need to repeat output here, the assign will return `{ok, DDoc}` or throw on none-match
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Conditionally use fetch and ddoc_cache...
Posted by eiri <gi...@git.apache.org>.
Github user eiri commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/144#discussion_r54163577
--- Diff: src/couch_changes.erl ---
@@ -221,21 +226,25 @@ configure_filter("_view", Style, Req, Db) ->
Msg = "`view` must be of the form `designname/viewname`",
throw({bad_request, Msg})
end;
-configure_filter([$_ | _], _Style, _Req, _Db) ->
+configure_filter([$_ | _], _Style, _Req, _Db, _) ->
throw({bad_request, "unknown builtin filter name"});
-configure_filter("", main_only, _Req, _Db) ->
+configure_filter("", main_only, _Req, _Db, _) ->
{default, main_only};
-configure_filter("", all_docs, _Req, _Db) ->
+configure_filter("", all_docs, _Req, _Db, _) ->
{default, all_docs};
-configure_filter(FilterName, Style, Req, Db) ->
+configure_filter(FilterName, Style, Req, Db, UseFetch) ->
FilterNameParts = string:tokens(FilterName, "/"),
case [?l2b(couch_httpd:unquote(Part)) || Part <- FilterNameParts] of
[DName, FName] ->
- DesignId = <<"_design/", DName/binary>>,
- {ok, DDoc} = ddoc_cache:open_doc(fabric:dbname(Db), DesignId),
+ {ok, DDoc} = open_ddoc(Db, <<"_design/", DName/binary>>),
check_member_exists(DDoc, [<<"filters">>, FName]),
- DIR = fabric_util:doc_id_and_rev(DDoc),
- {fetch, Style, Req, DIR, FName};
+ case UseFetch of
--- End diff --
I believe we can use the same logic as in `open_ddoc/2` to determine if we want `fetch` or `custom`, i.e.
```
case Db#db.id_tree of
undefined ->
DIR = fabric_util:doc_id_and_rev(DDoc),
{fetch, Style, Req, DIR, FName};
_ ->
{custom, Style, Req, DDoc, FName}
end;
```
Granted, it's less explicit than using `UseFetch` tag, but consider that we want to get rid of the whole none-clustered business it worth of trade - we'll need to change less code and wouldn't need to touch couchdb-chttpd at all.
How do you think?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Conditionally use fetch and ddoc_cache...
Posted by chewbranca <gi...@git.apache.org>.
Github user chewbranca commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/144#discussion_r54172140
--- Diff: src/couch_changes.erl ---
@@ -221,21 +226,25 @@ configure_filter("_view", Style, Req, Db) ->
Msg = "`view` must be of the form `designname/viewname`",
throw({bad_request, Msg})
end;
-configure_filter([$_ | _], _Style, _Req, _Db) ->
+configure_filter([$_ | _], _Style, _Req, _Db, _) ->
throw({bad_request, "unknown builtin filter name"});
-configure_filter("", main_only, _Req, _Db) ->
+configure_filter("", main_only, _Req, _Db, _) ->
{default, main_only};
-configure_filter("", all_docs, _Req, _Db) ->
+configure_filter("", all_docs, _Req, _Db, _) ->
{default, all_docs};
-configure_filter(FilterName, Style, Req, Db) ->
+configure_filter(FilterName, Style, Req, Db, UseFetch) ->
FilterNameParts = string:tokens(FilterName, "/"),
case [?l2b(couch_httpd:unquote(Part)) || Part <- FilterNameParts] of
[DName, FName] ->
- DesignId = <<"_design/", DName/binary>>,
- {ok, DDoc} = ddoc_cache:open_doc(fabric:dbname(Db), DesignId),
+ {ok, DDoc} = open_ddoc(Db, <<"_design/", DName/binary>>),
check_member_exists(DDoc, [<<"filters">>, FName]),
- DIR = fabric_util:doc_id_and_rev(DDoc),
- {fetch, Style, Req, DIR, FName};
+ case UseFetch of
--- End diff --
Yeah sounds good to me. I wasn't a huge fan of adding `configure_filter/5`, so happy to switch it around.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Conditionally use fetch and ddoc_cache...
Posted by chewbranca <gi...@git.apache.org>.
Github user chewbranca commented on the pull request:
https://github.com/apache/couchdb-couch/pull/144#issuecomment-189013418
@kxepal @eiri alright I rewrote the patch using the `Db#db.id_tree == undefined` hack and rebased out the old commit.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Conditionally use fetch and ddoc_cache...
Posted by chewbranca <gi...@git.apache.org>.
Github user chewbranca commented on the pull request:
https://github.com/apache/couchdb-couch/pull/144#issuecomment-188960881
Agreed @kxepal. It would be good to hear from @eiri as to whether this fixes the issues he's scene in https://github.com/apache/couchdb-couch-replicator/pull/26.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Conditionally use fetch and ddoc_cache...
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-couch/pull/144#issuecomment-188958636
Given the yesterday conversation on IRC, this all happens because we have a problem with design. Actually, that we have backdoor interface as standalone entity which eventually receives cluster bits that it can't eat. The best solution will require a lot of work and global things refactoring => more time.
Since this hack helps to solve things in short term, I'm +1, but only if we won't let it live for too long. And since backdoor interface have to go in anyway, so shall it be.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Conditionally use fetch and ddoc_cache...
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-couch/pull/144#issuecomment-189014757
Nice!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Conditionally use fetch and ddoc_cache...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/couchdb-couch/pull/144
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---