32KB TDS packets for monitored-server connections — the first #2164 lever, measured

[erikdarlingdata]

The measurement that motivated this

The open/drain instrumentation (#2173) shipped and overturned my prior conclusion. Seven samples, three servers, 12MB budget:

server	database	rows	sql	open	drain	open%	effective	per row
multi32	db-L	496	25,910	913	24,732	3.5%	497 KB/s	50 ms
multi-03	db-M	296	12,034	371	11,372	3.1%	1,080 KB/s	38 ms
multi32	db-K	107	35,650	1,027	34,303	2.9%	358 KB/s	321 ms
columbia	db-B	68	51,376	3,229	48,014	6.3%	256 KB/s	706 ms

Two facts fall out:

1. Drain is 94–97%. The server-side aggregate I previously blamed is 3–6%. That hypothesis is dead.
2. Drain is not bandwidth either. Effective throughput tracks LOB size, not total bytes: db-B averages ~180 KB/row and moves 256 KB/s; db-M averages ~41 KB/row and moves 1,080 KB/s — same link, same code, same budget. Four times the throughput for a quarter the row size.

That inverse relationship is per-packet overhead. Nothing in either app set PacketSize, so every connection ran at the driver's 8 KB default — a 180 KB plan costs ~22 packets where 32 KB packets cost ~6.

The change

One shared constant in PerformanceMonitor.Common applied by both builders (MonitoredServerConnection.BuildConnectionString for Darling, ServerConnection.BuildConnectionString for Lite), because the two are otherwise independent code and this is exactly the kind of setting that drifts silently.

32 KB is the protocol maximum; the driver accepts 512–32768 and both ends negotiate down to the smaller, so a server that dislikes it degrades rather than failing. The cost of being wrong is per-connection network buffer memory on both ends — kilobytes at our connection count, which is why it's a constant rather than a knob until someone shows it hurts.

This is a measurement, not a conclusion

The before numbers are above. I'll post the after from the same databases once this is on the dogfood fleet. If it doesn't move the drain, it comes back out — and the next lever is CommandBehavior.SequentialAccess (also unused today), then the known-hash plan skip, which the LOB-transfer finding puts back on the table for a better reason than the one I originally had.

Testing

PacketSizeTuningTests pins that Darling's builder actually emits it (the setting is invisible at runtime — a wrong packet size behaves correctly and merely runs slowly, so only a pin notices it being dropped), that the value stays inside the driver's accepted range (outside it throws at connect time, which on a monitoring fleet means every server going dark at once — that bound matters more than the tuning), and that it isn't silently reverted to the 8 KB default. Common, Darling service, Lite, and Darling.Tests build clean.

Part of #2164.

(Database names are relabelled db-* — this repo is public and the originals are client-identifying. Server names and every measurement are unchanged.)

[claude]

Reviewed the diff (CHANGELOG, PerformanceMonitor.Common/CollectorTdsTuning.cs, MonitoredServerConnection.cs, Lite/Models/ServerConnection.cs, PacketSizeTuningTests.cs). Summary: clean, well-scoped, well-tested — no correctness, parity, security, or perf issues found.

Parity — Both builders set PacketSize = CollectorTdsTuning.MonitoredServerPacketSize from the one shared constant, so Lite and Darling cannot drift on this setting. Confirmed the downstream connection-string reuse sites also inherit the tuned value correctly rather than rebuilding from scratch and dropping it:

• Lite/Analysis/SqlPlanFetcher.cs and Darling/.../PgPlanFetcher.cs (the actual LOB-heavy plan-XML fetch paths this change targets) both do new SqlConnectionStringBuilder(connectionString) { ConnectTimeout = ..., CommandTimeout = ... } — only those two keys are overridden, so PacketSize survives from the resolved base connection string.
• RemoteCollectorService/DarlingCollectorRunner's per-database and Azure-master connections build off server.ConnectionString / CredentialResolver.GetConnectionString(...) the same way.

Correctness — The one thing worth independently verifying was whether 32768 is actually inside SqlConnectionStringBuilder.PacketSize's accepted range (the driver's own validation, not just the test's self-referential InRange assertion) — if it weren't, every monitored-server connection would throw at open, exactly the "every server going dark at once" failure the PR's own comments call out as the real risk. PacketSizeTuningTests.DarlingBuildsMonitoredConnectionsWithTheTunedPacketSize does exercise the real builder with this value, and the "Darling PostgreSQL tests" CI job (which runs the full Darling.Tests suite, no filter) is green on this PR, so that's empirically confirmed rather than just asserted.

Minor, non-blocking nit: the doc comment on CollectorTdsTuning.MonitoredServerPacketSize says "the protocol maximum is 32,767; the driver accepts 512–32,768" — worth a follow-up tweak so the stated protocol max and the accepted/chosen value (32,768) don't read as contradicting each other, but this is documentation text only and doesn't affect behavior.

No missing-index-DMV comments, per instructions.