Multi get and consistend hashing #1
Conversation
Source? What is this referencing? |
|
sorry, not official - defacto? https://github.com/3rd-Eden/memcached
…On Tue, Mar 23, 2021 at 11:12 PM Jake Teton-Landis ***@***.***> wrote:
as the official client
Source? What is this referencing?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AADMZMFHXR7BT56PNEXBPYTTFFKDBANCNFSM4ZWL735A>
.
|
There was a problem hiding this comment.
Overall looks really, really good. Your buffer handling changes took a bit to sink in but make things more understandable overall. The getMulti internals look good too. What do you think about the tradeoff of using the same seq number for each key, VS using a different seq number for each pipelined request, and doing all the merging and result accumulation in latchCallback?
|
|
||
| if (err) { | ||
| hadError = true; | ||
| callback(err, null, null); |
There was a problem hiding this comment.
what cases does err cover? Is it connection errors, parsing errors, or does it include the server replying "hey we don't have this key"?
a) comment here might be good about this
b) could we ever want to return a partially filled recordMap?
There was a problem hiding this comment.
will add comment
err is just thrown errors - missing key is not an error (also, we are using "getkq - get key quiet" which doesn't even reply on missing key)
re: returning a partially filled recordMap - thought about this one some and ... I think no? it seems really unpredictable if you're calling five memcache backends and one goes bad, maybe the bad one replies first and this is empty, or the bad one replies last and it's mostly full, would the client really want to deal with that ... hmmm ... I mean, there's no harm in passing the recordMap back here and the client can choose if they want to use it? okay fine, yes
There was a problem hiding this comment.
Well I think you’re right that it’s weird to return inconsistently complete stuff.
|
re: "What do you think about the tradeoff of using the same seq number for each key, VS using a different seq number for each pipelined request, and doing all the merging and result accumulation in latchCallback?" - thought about this last night and it seems harder in that case to clear the handlers after the case if there's an error in the middle? would require a bunch more accounting and I think it would make server <-> client a leakier abstraction |
| } | ||
| break; | ||
| case 1: | ||
| if (callback) { callback(null, null, null); } |
There was a problem hiding this comment.
Does this get called only once for the NO_OP, or can it get called multiple times for getkq responses after the server gets the no-op?
blackmad
force-pushed
the
multiGetAndConsistendHashing
branch
from
ec12da0
to
68c504a
Compare
Implement multi/pipelined get in memjs in what I think is the least intrusive way possible, using almost all of their existing abstractions. Didn't want to do a big TS rewrite, or es6 upgrade, etc etc. If we keep doing surgery on this library I think we try to get this merged as in, and then separately try to bring the project up to vanilla es6, and then maybe typescript.
Based on: https://github.com/couchbase/memcached/blob/master/docs/BinaryProtocol.md#0x0d-getkq-get-with-key-quietly
Clients should implement multi-get (still important for reducing network roundtrips!) as n pipelined requests, the first n-1 being getq/getkq, the last being a regular get/getk. That way you're guaranteed to get a response, and you know when the server's done. You can also do the naive thing and send n pipelined get/getks, but then you could potentially get back a lot of "NOT_FOUND" error code packets. Alternatively, you can send 'n' getq/getkqs, followed by a 'noop' command.This takes the last option because I thought it was the clearest. Uses the same hashing library (hashring) as the official client.
Minor things to note