Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error reason for put is not propagated back to client #678

Merged
merged 1 commit into from
Oct 2, 2013

Conversation

engelsanchez
Copy link
Contributor

On a put error, the code in riak_kv_vnode will ignore the error and reply with a request id. We don't think this request id is necessary at all. We need to send back the error reason. The overload code is already replacing that request id with the overload token when it triggers.

@ghost ghost assigned engelsanchez Oct 2, 2013
@@ -1055,8 +1055,8 @@ prepare_put(#state{vnodeid=VId,
BProps))
end,
case handle_crdt(Coord, CRDTOp, VId, ObjToStore) of
{error, _}=E ->
{{fail, Idx, E}, PutArgs};%% NOTE subverts ReqId to Error
{error, E}=E ->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What gets sent to the fsm, can you overload E like this in one statement?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is not what I saw in my brain when I edited this. I forgot to delete the final =E

@russelldb
Copy link
Member

👍 works and everything :D

A couple of feature writers independently discovered that the vnode
reply included an unused request id (the sender already has the request
id and ignored this one) and used that to return error conditions. That
happens for overload and CRDT operations.

This change makes the reply officially contain an error reason instead
of the unuse request id. Notice that delete replies were changed, but
they are not used right now. That should be harmless and makes it all
consistent.
engelsanchez added a commit that referenced this pull request Oct 2, 2013
…reason

Error reason for put is not propagated back to client
@engelsanchez engelsanchez merged commit 15116ca into develop Oct 2, 2013
@engelsanchez engelsanchez deleted the bugfix/vnode-reply-reqid-to-error-reason branch October 2, 2013 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants