Repository navigation
Conversation
|
I'm not clear if this is a draft you're still working on or if it's ready for review? It definitely needs to be tested on illumos since the implementation tends to be platform-specific. What's the full keepalive policy after this change? I'm also a little worried that it would be easy to regress this without realizing it (particularly if the env var behavior could change in CockroachDB). Is there some way to regression-test that the setting is correct? |
|
Hey! The change is in place, but I haven’t tested it on illumos yet. I should have marked this PR as a draft. I’ll do that while validation is still pending. Based on source inspection, I expect the policy to be:
If the upstream illumos defaults apply, the abort threshold would still be 480s, giving roughly 510s for idle plus abort time. Definitely not Nexus’s For regression coverage, I’ll look into an illumos test that checks the accepted SQL socket’s keepalive settings. I’ll update the PR with the illumos results and test approach before marking it ready for review. |
Summary
Set
COCKROACH_SQL_TCP_KEEP_ALIVE=30sin CockroachDB’s SMF start-method environment, aligning its SQL keepalive idle threshold with Nexus’s 30-second threshold. Add regression tests for the manifest setting and the accepted SQL socket’s keepalive configuration on illumos.Related to #11084.
Resulting policy
Accepted SQL socket measurements on Helios:
Only the idle threshold changes. The remaining settings retain their existing behavior; this does not match Nexus’s
30s / 10s / 12policy. Actual disconnect timing was not measured.Validation
Tested commit
6eb07b3d41904c193c9db7c40f9edb0ef43d5633on Helios3.0.23976with CockroachDBv22.1.22-64-g86fdbfca06and Go1.22.11.Passed
svccfg validate, XML checks, formatting checks, andgit diff --check.DTrace A/B testing confirmed idle thresholds of 60 seconds with the variable unset and 30 seconds with
30s.An isolated SMF service delivered the environment setting to CockroachDB; direct socket inspection confirmed the policy above.
Both regression tests passed on Helios:
cargo nextest run --locked -p omicron-test-utils \ -E 'test(sql_keepalive)' --test-threads 1Result: 2 passed, 37 excluded by the filter.