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/01/21 05:12:13 UTC
[GitHub] [couchdb] tonysun83 opened a new pull request #3909: Fix new_edits:false and VDU function_clause
tonysun83 opened a new pull request #3909:
URL: https://github.com/apache/couchdb/pull/3909
In a rare, unsupported scenario, a user can call _bulk_docs with
new_edits:false and a VDU that denies the update of certain documents.
When nodes are out of sync (under load and mem3_repl can't keep up),
an update may or may not update the revision tree.
When two nodes extend the revision tree and forbids the update,
but one node is behind, an {error, W, [{Doc, FirstReply} | Acc]}
is returned instead of {ok, _} or {accepted, _}. The _bulk_docs
request does not accept {error, _} which leads to a function_clause.
This fix changes the return value to:
{accepted, W, [{Doc, {forbidden, Msg}} | Acc]}
when any of the nodes forbids the update.
Updating one document is also addressed so if the same issue occurs
during one document update, a 403 is still returned.
This should not affect replication since replication understands
forbidden return values and should ignore the update respectively.
<!-- Thank you for your contribution!
Please file this form by replacing the Markdown comments
with your text. If a section needs no action - remove it.
Also remember, that CouchDB uses the Review-Then-Commit (RTC) model
of code collaboration. Positive feedback is represented +1 from committers
and negative is a -1. The -1 also means veto, and needs to be addressed
to proceed. Once there are no objections, the PR can be merged by a
CouchDB committer.
See: http://couchdb.apache.org/bylaws.html#decisions for more info. -->
## Overview
<!-- Please give a short brief for the pull request,
what problem it solves or how it makes things better. -->
## Testing recommendations
<!-- Describe how we can test your changes.
Does it provides any behaviour that the end users
could notice? -->
## 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
- [ ] Code is written and works correctly
- [ ] 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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] tonysun83 edited a comment on pull request #3909: Fix new_edits:false and VDU function_clause
Posted by GitBox <gi...@apache.org>.
tonysun83 edited a comment on pull request #3909:
URL: https://github.com/apache/couchdb/pull/3909#issuecomment-1020539556
> In other words, we'd assert that there is at least one forbidden error (and all the rest of forbidden should match) and all the other responses are noreply.
@nickva I'm trying to figure out in what situations where we wouldn't want to forbid the document even if the replies were not `noreply`.
```
[{_, {forbidden, _}, Reply2, Reply3]
```
Reply2, and Reply3 can override forbidden only if they are equal to successful `Reply1 = {ok, Doc1}`, `Reply2 = {ok, Doc2}`. That doesn't seem like it can happen because a successful update implies an update to the revision tree, and the VDU would prevent it.
I can't think of any other scenarios where and Reply2 and Reply3 should override `forbidden`
--
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 edited a comment on pull request #3909: Fix new_edits:false and VDU function_clause
Posted by GitBox <gi...@apache.org>.
nickva edited a comment on pull request #3909:
URL: https://github.com/apache/couchdb/pull/3909#issuecomment-1018659332
We should be specific that this affects the `new_edits:true` _and_ VDU pass corner case, and explicitly check that other replies are all `noreply`, meaning VDU passed or VDU might have failed but we just didn't extended the revision tree so it returned a pass.
In other words, we'd assert that there is at least one forbidden error (and all the rest of forbidden should match) and all the other responses are `noreply`.
Another thing to check is if the replicator knows how to handle 202 responses from `new_edits:false` _bulk_docs requests. It looks like previously we might have never returned 202 response due to:
https://github.com/apache/couchdb/blob/3.x/src/fabric/src/fabric_doc_update.erl#L149-L150
```erlang
case [Reply || {ok, Reply} <- Replies]
[] ->
```
So there is a high chance replicator would not expect it and consider it an error.
Even if our (CouchDB) replicator would work, there is a chance PouchDB or other implementations might break. So I would lean towards staying with returning a 201 for this mode. What do you think?
EDIT:
It looks like replicator would break on a 202 response to _bulk_docs:
https://github.com/apache/couchdb/blob/3.x/src/couch_replicator/src/couch_replicator_api_wrap.erl#L456-L465
--
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] tonysun83 commented on pull request #3909: Fix new_edits:false and VDU function_clause
Posted by GitBox <gi...@apache.org>.
tonysun83 commented on pull request #3909:
URL: https://github.com/apache/couchdb/pull/3909#issuecomment-1032249362
@nickva finished the final touches
--
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] tonysun83 merged pull request #3909: Fix new_edits:false and VDU function_clause
Posted by GitBox <gi...@apache.org>.
tonysun83 merged pull request #3909:
URL: https://github.com/apache/couchdb/pull/3909
--
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 #3909: Fix new_edits:false and VDU function_clause
Posted by GitBox <gi...@apache.org>.
nickva commented on pull request #3909:
URL: https://github.com/apache/couchdb/pull/3909#issuecomment-1018659332
We should be specific that this affects the `new_edits:true` _and_ VDU pass corner case, and explicitly check that other replies all `noreply` (meaning VDU passed or VDU might have failed but we just didn't extended the revision tree so it returned a pass).
In other words, we'd assert that there is at least one forbidden error (and all the rest of forbidden should match) and all the other responses are `noreply`.
Another thing to check is if the replicator knows how to handle 202 responses from `new_edits:false` _bulk_docs requests. It looks like previously we might have never returned 202 response due to:
https://github.com/apache/couchdb/blob/3.x/src/fabric/src/fabric_doc_update.erl#L149-L150
```erlang
case [Reply || {ok, Reply} <- Replies]
[] ->
```
So there is a high chance replicator would not expect it and consider it an error.
Even if our (CouchDB) replicator would work, there is a chance PouchDB or other implementations might break. So I would lean towards staying with returning a 201 for this mode. What do you think?
--
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 change in pull request #3909: Fix new_edits:false and VDU function_clause
Posted by GitBox <gi...@apache.org>.
nickva commented on a change in pull request #3909:
URL: https://github.com/apache/couchdb/pull/3909#discussion_r795838752
##########
File path: src/fabric/src/fabric_doc_update.erl
##########
@@ -182,6 +193,34 @@ maybe_reply(Doc, Replies, {stop, W, Acc}) ->
continue
end.
+% This is a corner case where
+% 1) revision tree for the document are out of sync across nodes
+% 2) update on one node extends the revision tree
+% 3) VDU forbids the document
+% 4) remaining nodes do not extend revision tree, so noreply is returned
+% If at at least one node forbids the update, and all other replies
+% are noreply, then we reject the update
+check_forbidden_msg(Replies) ->
+ Pred = fun(R) ->
+ case R of
Review comment:
We could simply if it a tiny bit by matching in the function head to avoid an extra `case` statement
It might look like this
```erlang
Pred = fun
({_, {forbidden, _}}) -> true;
(_) -> false
end
```
--
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 edited a comment on pull request #3909: Fix new_edits:false and VDU function_clause
Posted by GitBox <gi...@apache.org>.
nickva edited a comment on pull request #3909:
URL: https://github.com/apache/couchdb/pull/3909#issuecomment-1018659332
We should be specific that this affects the `new_edits:false` _and_ VDU pass corner case, and explicitly check that other replies are all `noreply`, meaning VDU passed or VDU might have failed but we just didn't extended the revision tree so it returned a pass.
In other words, we'd assert that there is at least one forbidden error (and all the rest of forbidden should match) and all the other responses are `noreply`.
Another thing to check is if the replicator knows how to handle 202 responses from `new_edits:false` _bulk_docs requests. It looks like previously we might have never returned 202 response due to:
https://github.com/apache/couchdb/blob/3.x/src/fabric/src/fabric_doc_update.erl#L149-L150
```erlang
case [Reply || {ok, Reply} <- Replies]
[] ->
```
So there is a high chance replicator would not expect it and consider it an error.
Even if our (CouchDB) replicator would work, there is a chance PouchDB or other implementations might break. So I would lean towards staying with returning a 201 for this mode. What do you think?
EDIT:
It looks like replicator would break on a 202 response to _bulk_docs:
https://github.com/apache/couchdb/blob/3.x/src/couch_replicator/src/couch_replicator_api_wrap.erl#L456-L465
--
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] tonysun83 commented on pull request #3909: Fix new_edits:false and VDU function_clause
Posted by GitBox <gi...@apache.org>.
tonysun83 commented on pull request #3909:
URL: https://github.com/apache/couchdb/pull/3909#issuecomment-1023804432
I've updated the code to reflect some of the suggestions.
```
[noreply, forbidden, noreply] -> forbidden, but return 201 for overall bulk_docs
[forbidden, noreply, forbidden] -> forbidden, but return 201 for overall bulk_docs
[{ok, Doc1}, forbidden, {ok, Doc2}] -> {ok, {ok, Doc1}, {ok, Doc2}} - success
[forbidden, {ok, Doc1}, forbidden] -> {error, {mismatched_errors, Replies}}
[forbidden, some_other_error, forbidden] -> {error, {mismatched_errors, Replies}}
[error_a, error_b, forbidden] -> {error, {mismatched_errors, Replies}}
```
`[noreply, noreply, forbidden]` -> This one is tricky because and I don't have an answer for this. Right now it would be a success because quorum rules would it let slide through by the order of the `noreply`. In order to check for the possibility of a forbidden, we would have to wait for all nodes to respond for every request before making a decision. I think that's going to make things really non performant. Hypothetically, let's say we 7 nodes and one node extends the revision tree and is the most up to date and the other 6 for some reason are behind and the revision tree is not extended for all of them. I think we should leave the quorum rules here in place.
--
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] tonysun83 edited a comment on pull request #3909: Fix new_edits:false and VDU function_clause
Posted by GitBox <gi...@apache.org>.
tonysun83 edited a comment on pull request #3909:
URL: https://github.com/apache/couchdb/pull/3909#issuecomment-1020539556
> In other words, we'd assert that there is at least one forbidden error (and all the rest of forbidden should match) and all the other responses are noreply.
@nickva I'm trying to figure out in what situations where we wouldn't want to forbid the document even if the replies were not `noreply`.
```
[{_, {forbidden, _}, Reply2, Reply3]
```
Reply2, and Reply3 can override forbidden only if they are equal to successful `Reply1 = {ok, Doc1}`, `Reply2 = {ok, Doc2}`. That doesn't seem like it can happen because a successful update implies an update to the revision tree, and the VDU would prevent it.
I can't think of any other scenarios where and Reply2 and Reply3 should override `forbidden`. My view is that checking for `noreply` for the other responses is too restrictive.
--
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 #3909: Fix new_edits:false and VDU function_clause
Posted by GitBox <gi...@apache.org>.
nickva commented on pull request #3909:
URL: https://github.com/apache/couchdb/pull/3909#issuecomment-1025933896
> I think we should leave the quorum rules here in place.
Agree. Yeah, the way the quorum works `[noreply, noreply, forbidden]` would return `noreply` unless we wait for all the replies. Ideally we if we got extra information from workers (the shape of the rev tree and whether VDU ran or skipped, the VDU revision perhaps) we could make better decision but that's a larger 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 change in pull request #3909: Fix new_edits:false and VDU function_clause
Posted by GitBox <gi...@apache.org>.
nickva commented on a change in pull request #3909:
URL: https://github.com/apache/couchdb/pull/3909#discussion_r795880151
##########
File path: src/fabric/src/fabric_doc_update.erl
##########
@@ -157,7 +163,12 @@ force_reply(Doc, [FirstReply | _] = Replies, {Health, W, Acc}) ->
false ->
CounterKey = [fabric, doc_update, mismatched_errors],
couch_stats:increment_counter(CounterKey),
- {error, W, [{Doc, FirstReply} | Acc]}
+ case check_forbidden_msg(Replies) of
+ {forbidden, Msg} ->
+ {Health, W, [{Doc, {forbidden, Msg}} | Acc]};
+ false ->
+ {error, W, [{Doc, {mismatched_errors, Replies}} | Acc]}
Review comment:
I wonder if this would break anything since we previously returned a reply but now return the `mismatched_errors` tuple. The `mismatch_errors` is more clear, of course, but it could also break some json serialization of these errors for that doc (if we know how to serialize a reply error shape but not `{mismatched_errors, [_| _] = Replies}` or when we update a single doc with `update_doc(...)`
--
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] tonysun83 commented on a change in pull request #3909: Fix new_edits:false and VDU function_clause
Posted by GitBox <gi...@apache.org>.
tonysun83 commented on a change in pull request #3909:
URL: https://github.com/apache/couchdb/pull/3909#discussion_r801295805
##########
File path: src/fabric/src/fabric_doc_update.erl
##########
@@ -157,7 +163,12 @@ force_reply(Doc, [FirstReply | _] = Replies, {Health, W, Acc}) ->
false ->
CounterKey = [fabric, doc_update, mismatched_errors],
couch_stats:increment_counter(CounterKey),
- {error, W, [{Doc, FirstReply} | Acc]}
+ case check_forbidden_msg(Replies) of
+ {forbidden, Msg} ->
+ {Health, W, [{Doc, {forbidden, Msg}} | Acc]};
+ false ->
+ {error, W, [{Doc, {mismatched_errors, Replies}} | Acc]}
Review comment:
switched back to using FirstReply to make sure things don't
--
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 #3909: Fix new_edits:false and VDU function_clause
Posted by GitBox <gi...@apache.org>.
nickva commented on pull request #3909:
URL: https://github.com/apache/couchdb/pull/3909#issuecomment-1020646304
> I'm trying to figure out in what situations where we wouldn't want to forbid the document even if the replies were not `noreply`.
>
I was thinking of the case if we had some other errors - node crashed, other unspecified errors, etc. and we wouldn't want to ignore those in favor of a `forbidden`. Unless perhaps we're explicit and can say two node crashes and one forbidden should resolve to forbidden, but not if they are arbitrary errors?
> `[{_, {forbidden, _}, Reply2, Reply3]`
> Reply2, and Reply3 can override forbidden only if they are equal to successful Reply1 = {ok, Doc1}, Reply2 = {ok, Doc2}. That doesn't seem like it can happen because a successful update implies an update to the revision tree, and the VDU would prevent it.
For regular doc updates when revision trees don't match, we'd probably get conflicts for some and forbidden for other responses. It could be that the VDU is out of sync on the nodes as well. So I agree it's a bit unusual if we see a `[forbidden, {ok, _}, {ok, _}]` but then why not pick the `{ok, _}` response and ignore the forbidden? To be on the safe side, we should probably crash or highlight that as an odd mismatch. I think the current quorum logic currently would return a success if there are 2 successes at least? (I haven't verified it yet). What about `[forbidden, forbidden, {ok, _}]`, this is where we get the mismatch error and a 500 I think, and I think we should still get it. Ideally we'd probably want to inspect the revision tree to see which node saw a more recent revision tree (if it can even be determined), then we'd trust the node with the most recent revision doc and VDU revision, but that maybe a harder thing to do.
Because it's a bit hard to decide what to do in case of `new_edits:true` when we get those mismatches, I think we should instead of focus on the `new_edits:false` only, and restrict the PR to that. Because there, we have the corner case that `noreply` would be returned when the VDU [is not run because when revision tree is not extended](https://github.com/apache/couchdb/blob/3.x/src/couch/src/couch_db.erl#L1239-L1242). In that case I could see returning `forbidden` for the doc when we have either`[forbidden, noreply, noreply]` or `[forbidden, forbidden, noreply]`.
--
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] tonysun83 edited a comment on pull request #3909: Fix new_edits:false and VDU function_clause
Posted by GitBox <gi...@apache.org>.
tonysun83 edited a comment on pull request #3909:
URL: https://github.com/apache/couchdb/pull/3909#issuecomment-1020539556
--
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 #3909: Fix new_edits:false and VDU function_clause
Posted by GitBox <gi...@apache.org>.
nickva commented on pull request #3909:
URL: https://github.com/apache/couchdb/pull/3909#issuecomment-1020646304
> I'm trying to figure out in what situations where we wouldn't want to forbid the document even if the replies were not `noreply`.
>
I was thinking of the case if we had some other errors - node crashed, other unspecified errors, etc. and we wouldn't want to ignore those in favor of a `forbidden`. Unless perhaps we're explicit and can say two node crashes and one forbidden should resolve to forbidden, but not if they are arbitrary errors?
> `[{_, {forbidden, _}, Reply2, Reply3]`
> Reply2, and Reply3 can override forbidden only if they are equal to successful Reply1 = {ok, Doc1}, Reply2 = {ok, Doc2}. That doesn't seem like it can happen because a successful update implies an update to the revision tree, and the VDU would prevent it.
For regular doc updates when revision trees don't match, we'd probably get conflicts for some and forbidden for other responses. It could be that the VDU is out of sync on the nodes as well. So I agree it's a bit unusual if we see a `[forbidden, {ok, _}, {ok, _}]` but then why not pick the `{ok, _}` response and ignore the forbidden? To be on the safe side, we should probably crash or highlight that as an odd mismatch. I think the current quorum logic currently would return a success if there are 2 successes at least? (I haven't verified it yet). What about `[forbidden, forbidden, {ok, _}]`, this is where we get the mismatch error and a 500 I think, and I think we should still get it. Ideally we'd probably want to inspect the revision tree to see which node saw a more recent revision tree (if it can even be determined), then we'd trust the node with the most recent revision doc and VDU revision, but that maybe a harder thing to do.
Because it's a bit hard to decide what to do in case of `new_edits:true` when we get those mismatches, I think we should instead of focus on the `new_edits:false` only, and restrict the PR to that. Because there, we have the corner case that `noreply` would be returned when the VDU [is not run because when revision tree is not extended](https://github.com/apache/couchdb/blob/3.x/src/couch/src/couch_db.erl#L1239-L1242). In that case I could see returning `forbidden` for the doc when we have either`[forbidden, noreply, noreply]` or `[forbidden, forbidden, noreply]`.
--
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] tonysun83 commented on pull request #3909: Fix new_edits:false and VDU function_clause
Posted by GitBox <gi...@apache.org>.
tonysun83 commented on pull request #3909:
URL: https://github.com/apache/couchdb/pull/3909#issuecomment-1020539556
> In other words, we'd assert that there is at least one forbidden error (and all the rest of forbidden should match) and all the other responses are noreply.
@nickva I'm trying to figure out in what situations where we wouldn't want to forbid the document even if the replies were not `noreply`.
```
[{_, {forbidden, _}, Reply2, Reply3]
```
Reply2, and Reply3 can override forbidden only if they are equal to successful `Reply1 = {ok, Doc1}`, `Reply2 = {ok, Doc2}`. That doesn't seem like it can happen because the VDU would trigger upon a successful update.
I can't think of any other scenarios where and Reply2 and Reply3 should override `forbidden`
--
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] tonysun83 commented on pull request #3909: Fix new_edits:false and VDU function_clause
Posted by GitBox <gi...@apache.org>.
tonysun83 commented on pull request #3909:
URL: https://github.com/apache/couchdb/pull/3909#issuecomment-1020539556
> In other words, we'd assert that there is at least one forbidden error (and all the rest of forbidden should match) and all the other responses are noreply.
@nickva I'm trying to figure out in what situations where we wouldn't want to forbid the document even if the replies were not `noreply`.
```
[{_, {forbidden, _}, Reply2, Reply3]
```
Reply2, and Reply3 can override forbidden only if they are equal to successful `Reply1 = {ok, Doc1}`, `Reply2 = {ok, Doc2}`. That doesn't seem like it can happen because the VDU would trigger upon a successful update.
I can't think of any other scenarios where and Reply2 and Reply3 should override `forbidden`
--
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 #3909: Fix new_edits:false and VDU function_clause
Posted by GitBox <gi...@apache.org>.
nickva commented on pull request #3909:
URL: https://github.com/apache/couchdb/pull/3909#issuecomment-1032895939
@tonysun83 looks pretty good, try erlfmt-format and a rebase, so the CI will run
--
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