Skip to content

Adds X-Riak-Deleted where missing#520

Merged
broach merged 2 commits intomasterfrom
gh518-http-tombstone-headers
May 2, 2013
Merged

Adds X-Riak-Deleted where missing#520
broach merged 2 commits intomasterfrom
gh518-http-tombstone-headers

Conversation

@broach
Copy link

@broach broach commented Mar 26, 2013

In the case of a single object being returned which is a
tombstone and the case where a single sibling has been
requested via a vtag and it is a tombstone, the
X-Riak-Deleted header will now be added and
returned.

Ideally both these would return 204 rather than 404 and 200
respectively but doing so would have breaking consequences
for clients.

Fixes #518

In the case of a single object being returned which is a
tombstone and the case where a single sibling has been
requested via a vtag and it is a tombstone, the
X-Riak-Deleted header will now be added and
returned.

Ideally both these would return 204 rather than 404 and 200
respectively but doing so would have breaking consequences
for clients.
@ghost ghost assigned broach and seancribbs Apr 2, 2013
@seancribbs
Copy link
Contributor

Quick manual test case and failure:

$ curl -X PUT localhost:8098/buckets/test/props -H "content-type: application/json" -d '{"props":{"allow_mult":true}}'

$ curl -X PUT localhost:8098/buckets/test/keys/1 -H "Content-Type: text/plain"  -d "hello, world"

$ curl localhost:8098/buckets/test/keys/1 -i
HTTP/1.1 200 OK
X-Riak-Vclock: a85hYGBgzGDKBVIcypz/fgbGLL6bwZTImMfK4KFw8RRfFgA=
Vary: Accept-Encoding
Server: MochiWeb/1.1 WebMachine/1.9.2 (someone had painted it blue)
Link: </buckets/test>; rel="up"
Last-Modified: Wed, 03 Apr 2013 21:51:04 GMT
ETag: "3SHUhQSMLhfjR0X7uQCHgf"
Date: Wed, 03 Apr 2013 21:51:17 GMT
Content-Type: text/plain
Content-Length: 11

hello world

$ curl -X PUT localhost:8098/buckets/test/keys/1 -H "Content-Type: text/plain"  -H "X-Riak-Vclock: a85hYGBgzGDKBVIcypz/fgbGLL6bwZTImMfK4KFw8RRfFgA=" -d "hello 2"

$ curl -X DELETE localhost:8098/buckets/test/keys/1 -H "X-Riak-Vclock: a85hYGBgzGDKBVIcypz/fgbGLL6bwZTImMfK4KFw8RRfFgA="

$ curl -i localhost:8098/buckets/test/keys/1
HTTP/1.1 300 Multiple Choices
X-Riak-Vclock: a85hYGBgzGDKBVIcypz/fgbGLL6bwZTInMfK0Ktw8RRfFgA=
Vary: Accept, Accept-Encoding
Server: MochiWeb/1.1 WebMachine/1.9.2 (someone had painted it blue)
Last-Modified: Wed, 03 Apr 2013 21:52:13 GMT
ETag: "ByOt5fgRS2Fba75z0GB1W"
Date: Wed, 03 Apr 2013 21:52:20 GMT
Content-Type: text/plain
Content-Length: 56

Siblings:
6k3PJsbhPOTH4La6QudyXJ
4MqWbAYPO8Y0sQbwfLjkAj

$ curl -i localhost:8098/buckets/test/keys/1 -H "Accept: multipart/mixed"
HTTP/1.1 500 Internal Server Error
Vary: Accept, Accept-Encoding
Server: MochiWeb/1.1 WebMachine/1.9.2 (someone had painted it blue)
Last-Modified: Wed, 03 Apr 2013 21:52:13 GMT
ETag: "ByOt5fgRS2Fba75z0GB1W"
Date: Wed, 03 Apr 2013 21:52:29 GMT
Content-Type: text/html
Content-Length: 1233

<html><head><title>500 Internal Server Error</title></head><body><h1>Internal Server Error</h1>The server encountered an error while processing this request:<br><pre>{error,
    {error,
        {case_clause,{ok,true}},
        [{riak_kv_wm_utils,multipart_encode_body,4,
             [{file,"src/riak_kv_wm_utils.erl"},{line,126}]},
         {riak_kv_wm_object,'-produce_multipart_body/2-lc$^0/1-0-',5,
             [{file,"src/riak_kv_wm_object.erl"},{line,810}]},
         {riak_kv_wm_object,produce_multipart_body,2,
             [{file,"src/riak_kv_wm_object.erl"},{line,809}]},
         {webmachine_resource,resource_call,3,
             [{file,"src/webmachine_resource.erl"},{line,183}]},
         {webmachine_resource,do,3,
             [{file,"src/webmachine_resource.erl"},{line,141}]},
         {webmachine_decision_core,resource_call,1,
             [{file,"src/webmachine_decision_core.erl"},{line,48}]},
         {webmachine_decision_core,decision,1,
             [{file,"src/webmachine_decision_core.erl"},{line,546}]},
         {webmachine_decision_core,handle_request,2,
             [{file,"src/webmachine_decision_core.erl"},{line,33}]}]}}</pre><P><HR><ADDRESS>mochiweb+webmachine web server</ADDRESS></body></html>

@broach
Copy link
Author

broach commented Apr 18, 2013

This is not caused by my PR.

Building riak from master (which pulls riak_kv master), creating a sibling, then fetching via HTTP with accept: multipart/mixed causes this error currently. I just retested to verify.

This does not occur in 1.3.1 which is pulling {tag,"1.3.1"} for riak_kv (also verified just now)

@broach
Copy link
Author

broach commented Apr 18, 2013

The issue is this in src/riak_kv_wm_utils.erl

case dict:find(?MD_DELETED, MD) of {ok, "true"} ->

If you remove the double quotes around true it fixes it which makes sense given the error message. However ... I have no idea what/why that changed. I'm guessing the client puts that value in the dict?

@jrwest
Copy link
Contributor

jrwest commented Apr 18, 2013

@broach is correct, this is caused by [1], the new riak object format inadvertently changed the type of deleted from string to atom. I think the short term fix is to just change the type back (one line change). Long term, we should probably stop leaking riak object in places like kv_wm_utils as @seancribbs has outlined before.

[1] https://github.com/basho/riak_kv/blob/master/src/riak_object.erl#L640

The change in #532 makes this a string again ("true")
@seancribbs
Copy link
Contributor

With #532, this works as expected. 👍

broach pushed a commit that referenced this pull request May 2, 2013
@broach broach merged commit 37bfa2e into master May 2, 2013
@seancribbs seancribbs deleted the gh518-http-tombstone-headers branch April 1, 2015 23:31
@seancribbs seancribbs removed their assignment May 8, 2015
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.

3 participants