dterei / memcache-hs Goto Github PK

Claimed to be far faster than binary: https://www.fpcomplete.com/blog/2016/05/store-package

I.e.,

newMemcacheUnixClient :: FilePath -> IO Connection

@dterei I was wondering if I could pick your brain for a second.

The ElastiCache code I wrote yesterday is not working - The [received <- N.recv socket recvMsgSize](https://github.com/dterei/memcache-hs/blob/13b1c2b703f73d69588dca97338c3ce5cc26ecd9/Database/Memcache/ElastiCacheClient.hs#L188) code never returns.

I can't see anything obvious in resolveViaConfigCmd, but I don't have much networking experience. resolveViaConfigCmd does have a bug that I need to fix where I did not include \r\n in "config get cluster", but even with that modification, I never get any data from the socket.

Looking at the docs for recv, it is not surprising that it is blocking:

Receive data from the socket. The socket must be in a connected state. This function may return fewer bytes than specified. If the message is longer than the specified length, it may be discarded depending on the type of socket. This function may block until a message arrives.

But it should be sending back a message, I thought.

Fail-over with memcache doesn't really make sense. There just aren't clean semantics for implementing it, once you start the thought experiment you end up implementing DynamoDB.

We should support loading the list of servers and SASL auth from the environment. Probably best to have a new cluster creation function that explicitly looks to load form ENV.

GHC Version: 7.10.2
Platform: Mac OS X El Capitan 10.11.3

Hey @dterei !

First of all thank you for this library, it's one of the best out there when it comes to memcached (kudos for implementing authentication, it's the first thing other projects miss!). At work, I have been trying to use this library to talk to memcachier to provide us with a simple key/value cache, but I'm hitting a strange behaviour I will do my best in trying to explain here.

I have some run-of-the-mill code to provide a cache interface, which is not important, but that I'm reporting here for completeness:

memcacheLookup :: (IsKey k, IsValue v) => RemoteCacheProvider Memcache -> k -> IO (Maybe v)
memcacheLookup (MemcacheProvider client) k = do
  res <- Memcache.get client (toS $ toKey k)
  case res of
    Nothing -> return Nothing
    Just (val, _, _) -> return . JSON.decode . toS $ val

memcacheStore :: (IsKey k, IsValue v) => RemoteCacheProvider Memcache -> k -> v -> IO ()
memcacheStore (MemcacheProvider client) k v = do
  let key     = toS $ toKey k
  let payload = toS . JSON.encode $ v
  res <- Memcache.get client key
  case res of
    Nothing -> void $ Memcache.set client key payload 0 0
    Just (_, _, cas) -> void $ Memcache.set' client key payload 0 0 cas

memcacheDelete :: (IsKey k, IsValue v) => RemoteCacheProvider Memcache -> k -> IO (Proxy v)
memcacheDelete (MemcacheProvider client) k = do
  void $ Memcache.delete client (toS $ toKey k) 0
  return Proxy

As you can see, we are serialising/deserialising back and forth some JSON, but this is irrelevant. I have then written a spec which triggers a surprising problem:

newtype TestPayload = TP { value :: T.Text } deriving (Show, Eq)

deriveJSON defaultOptions ''TestPayload

memcacheCacheSpecs :: AppEnv -> Spec
memcacheCacheSpecs e = beforeAll wipeCache $ do
  let cache = e ^. appCache
  describe "Memcache Cache & Caching" $ do
    it "Storing and looking up stuff yield (Just x)" $ do
      let v = TP (T.replicate 2123 "a")
      putStrLn $ "====> SIZE: " <> show (B.length $ toS (JSON.encode v))
      cacheStore cache (CK $ UserID 50) v
      (r :: Maybe TestPayload) <- cacheLookup cache (CK $ UserID 50)
      r `shouldBe` Just v
  where
    wipeCache = do
      (_ :: Proxy TestPayload) <- cacheDelete (e ^. appCache) (CK $ UserID 50)
      return ()

As you can see, I'm simplifying everything at maximum, which is essentially sending over the wire a simple json filled with all a s, something like:

{ "value": "aaaaaaaaaaa" }

The error being thrown by your library, if I try to cacheLookup this value after a cacheStore, is NotEnoughBytes. I have dug deeper into the internals and indeed such error is thrown here, when the library receives less than the expected amount of bytes from the socket:

https://github.com/dterei/memcache-hs/blob/master/Database/Memcache/Wire.hs#L44

I have forked the library and added a bunch of putStrLn statements, like this:

recv :: Socket -> IO Response
recv s = do
    header <- recvAll mEMCACHE_HEADER_SIZE
    let h = dzHeader' (L.fromChunks [header])
    if bodyLen h > 0
        then do
          putStrLn $ "==> BodyLen: " <> show (bodyLen h)
          let bytesToRead = fromIntegral $ bodyLen h
          putStrLn $ "==> BytesToRead: " <> show bytesToRead
          body <- recvAll bytesToRead
          return $ dzBody' h (L.fromChunks [body])
        else return $ dzBody' h L.empty
  where
    recvAll :: Int -> IO B.ByteString
    recvAll n = do
        putStrLn $ "==> Going to read " <> show n <> " bytes..."
        buf <- N.recv s n
        let bufLen = B.length buf
        putStrLn $ "==> I have read " <> show bufLen <> " bytes..."
        if bufLen == n
          then return buf
          else throwIO NotEnoughBytes

And, armed with this tools, I have rerun the specs:

As you can see, there is this magical 1424 bytes value which is the maximum value we get back from the socket. If I try to pass a smaller payload, which size is smaller or equal than 1424, it works correctly. I have found this empirical number which is 1420: if my payload is bigger than 1420 bytes, the library will try to read more than 1424 bytes (Apparently there are 4 extra bytes at the end coming out of nowhere -- padding perhaps?) and everything breaks.

I have tried to perform a get on large payload with a Python client, to try to rule out as many problems as possible, and the latter works correctly:

It's also using the same memcachier instance, which rules out some weird memcachier config issue.

I think that, from my perspective, we are faced with one of these three:

1. There is a bug in my understanding of this API and/or in my store/lookup/delete functions
2. There is a bug in this library
3. There is a very sneaky bug in the network package, specifically in the recv function for ByteStrings.

What do you think? Any input from you would be highly appreciated and value? Please shout if you need any reproduction steps.

Thanks a ton!

Alfredo

Depending on memcache in a cabal project right now causes it to pull in version 3.1.x of the network dependency; however, this version is not API-compatible with the 2.x range, and fails to build. Explicitly adding network <3.0 as a constraint, I can work around the problem, but it would be nice to have proper upper bounds on the dependencies of memcache itself.

@dterei - there has been a broad-based push to rename the default branch from master to main.

More info here: https://github.com/github/renaming

Would you be open to doing so for this repository?

👋 Hi there-

Thank you for this client library, it's the one that best met our needs here at Freckle. Would you be willing to add it in Stackage so we can stop carrying it as an extra-dep? If not, we might bring it in under our section.

Thanks!

Make use of weigh and criterion for benchmarking.