binary get-set behavior bug

[seamusabshere]

I think that's what I'm seeing too...

Trying with binary_protocol => true
... failed - #<Memcached::ServerEnd: Key {"hi"=>"127.0.0.1:11211:8"}>
Trying with binary_protocol => false
... succeeded

When I run this...

#!/usr/bin/env ruby

require 'memcached'

[ true, false ].each do |binary_protocol|
  puts "Trying with binary_protocol => #{binary_protocol}"
  m = ::Memcached.new(['127.0.0.1:11211'], :binary_protocol => binary_protocol)
  m.flush

  # Raises a NotFound but we ignore it
  m.get 'hi' rescue nil

  begin
    m.set 'hi', 'there'
    puts "... succeeded"
  rescue
    puts "... failed - #{$!.inspect}"
  end
end

My operating system

$ memcached -h
  memcached 1.4.5
$ ruby -v
  ruby 1.9.2p180 (2011-02-18 revision 30909) [x86_64-darwin10.7.0]
$ uname -a
  Darwin vidalia 10.7.0 Darwin Kernel Version 10.7.0: Sat Jan 29 15:17:16 PST 2011; root:xnu-1504.9.37~1/RELEASE_I386 i386

[kostya]

revert this commit "Always send the NOP on binary MGETs" fix it, but multigets with one key still not work

[seamusabshere]

Is there any interest in fixing this?

[bitbckt]

Not from us. We don't use the binary protocol at all and don't spend any time guaranteeing its correctness at this point. By way of an exception, I fixed the other failures because they were merged into master prior to a fix being submitted.

[miv]

Is there a reson why not to use binary protocol or it just historically?

[seamusabshere]

For what it's worth, this is still a problem in 1.2.7.1.

(To summarize: If you are using binary protocol and you try to call #set after a #get that raises NotFound, you will get a ServerEnd. This only happens in binary mode.)

[kostya]

all tests passed, in my project too.

[seamusabshere]

My test (which is that script above) passes with kostya's branch.

My test fails with 1.2.7.1 (as I said before).

[seamusabshere]

@bitbckt and @fauna: please do accept @kostya's pull request.

It fixes the issue, doesn't touch the libmemcached C code, and only adds the equivalent of Array#length to the normal overhead of #mget

If you would like it to be solved in a different way, please say so.

[evan]

I'm not opposed to this change. Can you rebase it on current master? I'll check it out shortly.

We don't use the binary protocol because it is slower (!).

[kostya]

rebased

little bug when install memcached rake gem gem install .. ... ../libmemcached/memcached.h:26: fatal error: libmemcached/memcached_touch.h: No such file or directory compilation terminated.

[evan]

Fixed that. Looking at your change now.

[kostya]

rake gem rake aborted! Don't know how to build task 'ext/libmemcached-0.32/autom4te.cache/output.0'

[evan]

Fixed again; sorry.

[kostya]

in my other tests this branch is ok.

[evan]

So, the test you including doesn't actually exercise the fault; it just makes sure that your changeset doesn't break something else.

Can you add a test that specifically exercises the fault? I might be able to fix it another way.

[kostya]

test for my changesets already added in revision 65c6da53e2e3e8c661b8e054ac30bbe25e8a1428 and Brandon Mitchel fix it by revision a6569c9e627ad93d9c9fd70b4741631a9fc1a477. but his fix creates "binary get-set behavior bug". In this branch i fix it all.

[evan]

I need a failing test for "binary get-set behavior bug" in order to qualify your change, though.

[kostya]

ec52333 ?

[evan]

~/p/fauna/memcached master $ git cherry-pick ec52333 [master 17c8aca] binary get-set behavior bug Author: Konstantin Makarchev kostya27@gmail.com 1 files changed, 12 insertions(+), 0 deletions(-) ~/p/fauna/memcached master $ ruby test/unit/memcached_test.rb -n test_get_set_binary_behavior Loaded suite test/unit/memcached_test Started . Finished in 0.004273 seconds.

[kostya]

hm, this seems was fixed by 1b3912eb0d8ce0da22f3172fc6e086666f48936c sorry :)

[ChristopherThorpe]

I solved this by adding ServerEnd to the exceptions to retry. See https://github.com/evan/memcached/pull/79

$ diff /tmp/x memcached-1.0.6/lib/memcached/memcached.rb 46a47

    Memcached::ServerEnd,