framerPool.get: a framer obtained while the pool is disabled loses the connection's negotiated config

[nikagra]

Summary

framerPool.get (framer.go) falls back to a freshly constructed framer whenever the pool is disabled:

func (fp *framerPool) get(c *Conn) *framer {
	if !fp.enabled.Load() {
		return newFramer(c.compressor, c.version)
	}
	return fp.pool.Get().(*framer)
}

newFramer (frame.go) carries none of the connection's negotiated protocol-extension state: it never assigns flagLWT or rateLimitingErrorCode, and it explicitly zeroes tabletsRoutingV1 and scyllaUseMetadataID. A framer obtained through that path therefore disagrees with connFramers.defaults, which is the single source of truth the request path reads through Conn.usesMetadataID() and Conn.tracksResultMetadataID().

The pool is disabled in exactly two windows:

1. Before connFramers.initCache runs — the handshake. Correct and intentional: nothing has been negotiated yet, and defaultFramerFlags deliberately withholds FlagCompress until the compressor is confirmed.
2. After connFramers.close() — i.e. after Conn.closeWithError has already drained every in-flight call. This is the problematic one.

Impact

Currently latent. Spelling out why, so it does not have to be re-derived:

• Write path is safe by call ordering. execInternal (conn.go) calls getWriteFramer() before addCall(), and addCall is what rejects a closed connection. So the only way to obtain a mis-configured write framer is on a path where addCall then fails, the framer is released, and no frame is ever written. There is no wire effect.
• Read path is safe by its call sites. recv acquires a read framer only after the call == nil branch has already diverted responses to discardFrame, so responses to drained calls never reach it. The other site, readFrameIntoFramer, is reached only for head.Stream <= 0 — reserved streams, i.e. EVENT frames and protocol errors — which carry no result metadata.
• Pooled reuse is clean. framerPool.resetAndPut only resizes buffers; it does not call newFramer, so a recycled framer keeps its configuration.

What makes it worth fixing is that the invariant is upheld by call ordering across three functions rather than by construction. Moving getWriteFramer() to after addCall() — an otherwise clean simplification, since it would stop acquiring a framer the call may not need — silently turns this into a live bug that mis-encodes EXECUTE frames.

The worse latent case: flagLWT

scyllaUseMetadataID is the least consequential of the four fields. parsePreparedMetadata (frame.go) computes:

meta.lwt = meta.flags&f.flagLWT == f.flagLWT

With flagLWT == 0 this is flags & 0 == 0, i.e. unconditionally true — every prepared statement parsed by such a framer would be reported as LWT. That predates the metadata-id work; #590 makes it four affected fields rather than three.

Proper fix

Give the disabled path the same construction the pool itself uses, so there is one place a framer is built from a framerConfig:

• store defaults framerConfig on framerPool in framerPool.init (it already receives it, but only closes over it inside sync.Pool.New);
• extract the struct literal from that New closure into a newFramerFromConfig(cfg framerConfig) *framer;
• call it from both New and the !enabled branch of get.

Roughly ten lines. Note the fallback framer also has a nil release, which framer.Release() already handles as a no-op, so it does not need to be pooled.

Acceptance

• A framer obtained while the pool is disabled carries the same flagLWT, rateLimitingErrorCode, tabletsRoutingV1 and scyllaUseMetadataID as one obtained from the pool.
• A unit test pins that, so the guarantee stops depending on the ordering of getWriteFramer() and addCall().
• The caveat added to Conn.usesMetadataID's doc comment in Implement SCYLLA_USE_METADATA_ID protocol extention negotiat… #590 can be dropped.

Follow-up to #590.