I understand the serializer requirement, but why are weforcing binary protocol and no_block=false?

Binary because we must support all chars in keys especially spaces, per psr-6.
No block because we need guarantees when reading/writing to the cache. Dealing with async would make the code complex for no benefit.

Regardingbinary_protocol requirement: I'm on board with this.

But, regardingno_block requirement: I disagree. The intended use of this option doesn't require any additional code, and, in fact, writing such code would invalidate the entire intended usage and benefits ofno_block. The expectation when it is enabled is that the userdoesn't need guarantees when writing. It is acommonly used option and generally doesn't change expectations; due to the design of memcached, one can never assume a value exists as it may have been purged at any time by the LRU algorithm, and will definitely not exist after a restart of the daemon.

fixed for no_block

This needs to beforeach (array_reverse($options) as $name => $value) as seen in my PR, otherwise defaul values that affect multiple options (such aslibketama_compatible) will overwrite use-provided values.

I think reverse or not, it's the responsibility of the user to order correctly. I'd better behave the same as the native setOptions so that expectations don't have to be reversed when using our lib...

It's due to our code, though. The default option oflibketama_compatible in combination with$options += static::$defaultClientOptions; means a user cannot change, for example, the hash algorithm, ever...as our default oflibketama_compatible will be appliedafter the user-provided option (for example['hash' => 'something-else']), resetting the hash tomd5. The options need to be applied in reverse order to allow the user to overwrite our defaults.

To clarify: the user should, rightfully, assume that any options they provide will overwrite our defaults. The issue is,some options actually affect multiple options. Without reversing the array, the user options will not always result in a proper assignment; while reversing the options ensures user options always overwrite defaults.

It's not theorder of options that is important, it's the order ofdefault options touser options that is important.Default options must be applied first, so either the assignment needs to change:

# from this:$options +=static::$defaultClientOptions;# to this (or something similar):foreach (static::$defaultClientOptionsas$name =>$option) {if (!array_key_exists($name,$options) {array_unshift($options, [$name =>$option]);    }}

OR the order of options needs to be reversed:

# from this:foreach ($optionsas$name =>$value) {}# to this (or something similar):foreach (array_reverse($options)as$name =>$value) {}

Otherwise options will behave unexpectedly for the user.

understood, fixed also

We shouldn't add servers after a connection is open (even when the user has added an additional server to their configuration)?

Not sure to get what you mean. Do you see anything wrong with this logic?
Here it is: if the connection is not pristine (meaning it's a 2nd use persistent connection), then we check if there is any change in the list of server. If there is, we reset the list and set the new one.

Ok. Had trouble following the code, but makes sense now.

What about usingAdapterInterface::class to not make it harder for bundles that do static analyses of the container?

Should theAbstractAdapter really be aware of all adapters that require a connection?

This is a stateless static method, no need for yet another class. This has to be somewhere :)
Other comments addressed.

Should we useOPT_SERIALIZER instead ofserializer here to be consistent with the exception message below?

The way this code is setup and documented, you passserializer, notopt_serializer, so I would argue the inverse: theother exception message should be amend to:

binary_protocol must be used

gettype()