memcache: add pipelining, modernize some of the Go

[bradfitz]

Also:

• add synctest tests, which requires newer Go, so build tag those
  out. We need to bump go.mod to get working synctest, so:
• ... also add a CI check that we keep building with Go 1.18 for now,
  not using Go language/API changes newer than Go 1.18.
• few more config knobs

Fixes #160

[dormando]

Looks pretty swanky on a first skim, neat!

I'll review it more carefully and try using it locally to kick the API tires. hopefully won't take that long...

[dormando]

(if there's something more useful I should be doing in a review, lmk. I'm relatively informal).

Writing out my understanding of the high level logic...

User

• Create a request via client.Get/Set/whatever.
• These API calls block until a response is received.
• Behind the scenes the requests are pipelined.

Internal

• Collect pipeReqs into per backend queues.
• On dispatch, find backend with smallest queue and write
• If all backends are at MaxPipelineDepth dial a new connection
• If all dials are in use, internally queue the req

Then there's a bunch of logic around when to dial, how much to enqueue before adding a new dial, reaping idle connections, etc. Some requests can be retried and some fail if a connection is lost. Will look more into that.

Starter questions?

• Any way for a user to submit N requests at once and block? IE: batch sets during a bulk load.
• There no noreply support correct? (that breaks the pipelining)
• Seems like errors are handled by being detected as any unexpected response? The protocol has some standards around this: CLIENT_ERROR (and ERROR, sigh) means the server thinks you need to close the connection and blow the queue. SERVER_ERROR means that particular request in the stack can be potentially be retried; no reason to stop processing.
• Thoughts on the initial defaults here? I suspect a much lower maxconns can be good, but might require a different enqueuing approach.

Queue handling differences from my own work

In my own code I try to minimize the number of connections, but this requires changes around guarantees...

• Maxconns defaults to 2 (and preconnects, but not necessary for this client). This is per worker thread so a user may have like 8 conns typically.
• [same] New requests enqueue onto the shortest pipeline, but this check should be less overhead since we encourage fewer conns on busy systems
• [different] if maxdepth is reached on all conns, further requests fast-fail. It uses much higher numbers for maxdepth, and instead writes to the socket (nonblock) until EWOULDBLOCK. The idea is to let the OS batch as much as it can.
• Once EWOULDBLOCK requests enqueue until the maxdepth is reached.
• The depth fast fail is an OOM backstop if a client is just submitting way too fast.

Concern is with a small pipeline (8) someone doing a bulk load or fetch (say 1,000 small writes) ends up waiting for 120+ roundtrips. User has to recognize this situation and change the pipeline value to be higher. Typically this means they do a benchmark and decide it's slow.

@bradfitz Here's some high level stuff then I'll look into more detail? Code looks good enough that I probably have no business reviewing it. Maybe some concern over the number of channel trips becoming a performance bottleneck?

edit: fwiw I don't use AI when reviewing code. hopefully that makes my words worth reading :P

[SuperQ]

I typically like to support at least the last two versions of Go to follow the upstream support lifecycle.

Also, since Go 1.21, this is now a semver tag.

Suggested change

	go 1.26
	go 1.25.0

[bradfitz]

Also, since Go 1.21, this is now a semver tag.

Not necessarily. It can be, but you can also just say 1.X if you don't care about specifics.

[SuperQ]

True. But I still prefer the semver convention.

So, if we want to still support compile with 1.18, why not just leave this at 1.18 for now?

[bradfitz]

See the comment in the test: I want to use synctest in tests so I can't use 1.18 in go.mod

[bradfitz]

@dormando, I pushed an update to handle SERVER_ERROR.

Let's do batched set calls in a future PR.

And yeah, no noreply support.

PTAL?

[dormando]

@bradfitz awesome! I think most of what I yapped about can be iterated on in future PR's, except: need limit on b.queue length? Else user with horked backend ends up OOM'ing with no option for circuit breaking?

Don't think I see a limit on the queue:

• maxdepth exceeded, new dial
• maxconns exceeded, goes into b.queue
• dequeues as slots become available.