basho / riak-erlang-client

The Riak client for Erlang.
Apache License 2.0
311 stars 188 forks source link

incorrect response on put request when using autogenerated keys [JIRA: CLIENTS-874] #287

Closed sata closed 8 years ago

sata commented 8 years ago

Hi

I'm running a Riak 2.0.6-1 cluster with riak erlang client lib version 2.1.1, riak_pb 2.1.0.2.

According to the README.md

If the return value of the last command was anything but the atom ok (or {ok, Key} when you instruct the server to generate the key), then the store failed. The return value may give you a clue as to why the store failed...

Which is expressed in the code[1] as well. This is the same function header as in 2.1.1.

While the response I get is:

{ok,{riakc_obj,{<<"bucket_type">>,<<"bucket">>},
               <<"DYih3nkXtFi6ytGm7T2q7pjG2DG">>,undefined,[],undefined,
               undefined}}

The object itself is created with undefined as key: Obj = riakc_obj:new(?REQUEST_BUCKET, undefined, to_binary(Req)),

When I traced the process_response/3 function I saw this, I'll only include the interesting bits:

(<0.231.0>) call riakc_pb_socket:process_response(
                   {request,#Ref<0.0.2.19281>,
                      {rpbputreq, ... },
                   {rpbputresp,[],undefined,<<"1lZ4hsH4Y6RB5aYtkVj8JTqNCAh">>},
                   {state, ...}
                  )

rpbputresp is defined as:

-record(rpbputresp, {
    content = [],
    vclock,
    key
}).

So if we review the code call, we see that content is set to [], while in [1] it's expected to be undefined thus the pattern match doesn't work.

I changed the code to pattern match against content [] and I did receive {ok, <<"key">>}.

In [2] for #rpbgetresp [] is being pattern matched not undefined.

I can create a PR to correct this if this is indeed a bug. If it isn't I'm curious to know what's wrong with my setup. Cheers,

[1] https://github.com/basho/riak-erlang-client/blob/eed70717ece9c6796e29b5a326fa6af98b2ec43a/src/riakc_pb_socket.erl#L1661-L1666 [2] https://github.com/basho/riak-erlang-client/blob/eed70717ece9c6796e29b5a326fa6af98b2ec43a/src/riakc_pb_socket.erl#L1644-L1645

lukebakken commented 8 years ago

OK, I did some digging and tests do exist for Riak-generated keys:

https://github.com/basho/riak-erlang-client/blob/master/src/riakc_pb_socket.erl#L3270-L3292

I ran the test suite against a 2.1.4 devrel using this command:

make -C buildbot RIAK_TEST_PBC_1=10017 RIAK_TEST_NODE_1='dev1@127.0.0.1' RIAK_DIR=/home/lbakken/Projects/basho/riak/dev/dev1 test

Those two tests passed OK using the master branch of this client. I'm testing with Riak 2.0.6 next.

sata commented 8 years ago

@lukebakken: the assertMatch on [1] is pretty useless. Since it doesn't check what "Key" is, it can be a riakc_obj, term or a binary.

[1] https://github.com/basho/riak-erlang-client/blob/eed70717ece9c6796e29b5a326fa6af98b2ec43a/src/riakc_pb_socket.erl#L3277

sata commented 8 years ago

Thanks for that command, I was thinking how I could run the live_node_tests

lukebakken commented 8 years ago

Ah, thanks for pointing out the deficiency in the test.

sata commented 8 years ago

the test cases in live_node_tests() won't get executed since they're not eunit test functions, i.e ending with 'foo_test' or 'footest'. the test target in tools.mk is just a wrapper over rebar2 eunit.

lukebakken commented 8 years ago

@sata - the command I provided does run all the tests, including live_node_tests. I haven't run them against a Riak installed from official packages, however, just from local builds for development. I'm assuming you're using official packages?