fetch local docs in fold_docs function #2869
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
davisp
requested changes
May 5, 2020
src/fabric/src/fabric2_db.erl
Outdated
| user_fun := UserFun | ||
| } = Acc, | ||
|
|
||
| NewUserAcc = maybe_stop(UserFun(DocId, DocResp, UserAcc)), |
There was a problem hiding this comment.
This is broken. You can't mix synchronous and asynchronous fetches like this. POST /dbname/_all_docs has a (perhaps implicit?) contract for returning docs in the order specified in the body. Mixing these operations means that we'd be returning any _local docs in some random position within the previous 100 or so docs returned.
nickva
requested changes
May 5, 2020
nickva
reviewed
May 6, 2020
src/fabric/src/fabric2_db.erl
Outdated
| @@ -996,12 +986,12 @@ fold_docs(Db, DocIds, UserFun, UserAcc0, Options) -> | |||
| user_fun => UserFun | |||
| }, | |||
|
|
|||
| FinalAcc1 = lists:foldl(fun(DocId, Acc) -> | |||
| FinalAcc1 = lists:foldl(fun (DocId, Acc) -> | |||
There was a problem hiding this comment.
Minor nit: whitespace change
nickva
requested changes
May 6, 2020
There was a problem hiding this comment.
Looks much better, @garrensmith
I think we need to account for deleted docs as well:
http post $DB1/db/_all_docs?include_docs=true keys:='["d1", "_local/l1", "d3"]'
HTTP/1.1 200 OK
Cache-Control: must-revalidate
Content-Type: application/json
Date: Wed, 06 May 2020 17:47:36 GMT
Server: CouchDB/3.0.0-97227c4 (Erlang OTP/21)
Transfer-Encoding: chunked
X-Couch-Request-ID: 7ce49a7c4b
X-CouchDB-Body-Time: 0
{
"offset": null,
"rows": [
{
"doc": {
"_id": "d1",
"_rev": "1-935564bc7ec7e86aa82ecec3face18f6",
"x": "y"
},
"id": "d1",
"key": "d1",
"value": {
"rev": "1-935564bc7ec7e86aa82ecec3face18f6"
}
},
{
"doc": {
"_id": "_local/l1",
"_rev": "0-1",
"l1doc": "x"
},
"id": "_local/l1",
"key": "_local/l1",
"value": {
"rev": "0-1"
}
},
{
"error": "not_found",
"key": "d3"
}
],
"total_rows": 2
}
nvatama@nvm asf (pr/2869) $ http delete $DB1/db/_local/l1
HTTP/1.1 200 OK
Cache-Control: must-revalidate
Content-Length: 41
Content-Type: application/json
Date: Wed, 06 May 2020 17:47:43 GMT
ETag: "DKGUB9HWEX2KQNSCSMV0JG1KK"
Server: CouchDB/3.0.0-97227c4 (Erlang OTP/21)
X-Couch-Request-ID: 352a9b2160
X-CouchDB-Body-Time: 0
{
"id": "_local/l1",
"ok": true,
"rev": "0-0"
}
$ http post $DB1/db/_all_docs?include_docs=true keys:='["d1", "_local/l1", "d3"]'
HTTP/1.1 500 Internal Server Error
Cache-Control: must-revalidate
Content-Length: 77
Content-Type: application/json
Date: Wed, 06 May 2020 17:47:44 GMT
Server: CouchDB/3.0.0-97227c4 (Erlang OTP/21)
X-Couch-Request-ID: cba45e587e
X-Couch-Stack-Hash: 1247935461
X-CouchDB-Body-Time: 0
{
"error": "case_clause",
"reason": "{ok,{not_found,missing}}",
"ref": 1247935461
}
The logs show the place where error happens:
[error] 2020-05-06T17:47:45.194056Z [email protected] <0.8976.0> cba45e587e req_err(1247935461) case_clause : {ok,{not_found,missing}}
[<<"chttpd_db:-send_all_docs_keys/3-fun-0-/5 L902">>,<<"fabric2_db:drain_one_fold_docs_body_future/2 L1361">>,<<"fabric2_db:drain_all_fold_docs_body_futures
/2 L1342">>,<<"fabric2_db:-fold_docs/5-fun-1-/5 L1003">>,<<"fabric2_fdb:execute_transaction/3 L1915">>,<<"fabric2_fdb:-do_transaction/2-fun-1-/3 L173">>,<<"erlf
db:do_transaction/2 L655">>,<<"fabric2_fdb:do_transaction/2 L158">>]
|
@nickva thanks I've updated with a fix for the deleted doc. Great spot on the deleted doc. I've updated the elixir test to test for that. |
nickva
reviewed
May 7, 2020
test/elixir/test/all_docs_test.exs
Outdated
|
|
||
| assert resp.status_code == 200 | ||
| rows = resp.body["rows"] | ||
| IO.inspect rows |
There was a problem hiding this comment.
A debug line? I notice elixir's credo test didn't like that:
Checking 74 source files (this might take a while) ...
Warnings - please take a look
┃
┃ [W] ↗ There should be no calls to IO.inspect/1.
┃ test/elixir/test/all_docs_test.exs:343:5 #(AllDocsTest)
Please report incorrect results: https://github.com/rrrene/credo/issues
nickva
reviewed
May 7, 2020
src/fabric/src/fabric2_db.erl
Outdated
| fold_docs_get_doc_body_wait(Db, <<?LOCAL_DOC_PREFIX, _/binary>> = DocId, [Rev], | ||
| _DocOpts, BodyFuture) -> | ||
| case fabric2_fdb:get_local_doc_body_wait(Db, DocId, Rev, BodyFuture) of | ||
| {not_found,missing} -> {not_found, missing}; |
There was a problem hiding this comment.
Minor nit: space between not_found and missing
nickva
approved these changes
May 7, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
Testing recommendations
Related Issues or Pull Requests
Checklist
rel/overlay/etc/default.ini