[DO NOT MERGE] adapter: prove LD reconnect test goes red on pre-fix SDK 3.0.1#37460
Draft
jasonhernandez wants to merge 3 commits into
Draft
[DO NOT MERGE] adapter: prove LD reconnect test goes red on pre-fix SDK 3.0.1#37460jasonhernandez wants to merge 3 commits into
jasonhernandez wants to merge 3 commits into
Conversation
3328e82 to
dd4b798
Compare
Move launchdarkly-server-sdk from the MaterializeInc/rust-server-sdk fork back to upstream crates.io 3.1.1, restoring the launchdarkly-sdk-transport + MetricsTransport setup and dropping the [patch.crates-io] override. The fork existed for launchdarkly/rust-server-sdk#116: a StreamingDataSource /eventsource StreamClosed bug where a non-Eof stream error left the data source stuck with no reconnect, silently breaking LD sync. A prior upgrade to upstream 3.0.1 had to be reverted (incident-984) because that bug was still unfixed upstream. The fixes have since landed, rust-server-sdk#168 and rust-eventsource-client#134/#135, and 3.1.1 resolves eventsource-client to 0.17.5, which carries them. Use the rustls + aws-lc-rs features (hyper-rustls-native-roots, crypto-aws-lc-rs), now the upstream defaults, instead of the prior attempt's native-tls/crypto-openssl, avoiding the OpenSSL path. The transport build_https() call is identical either way. Because the SDK now builds a rustls client, the workspace links both rustls provider features (aws_lc_rs via our features, ring transitively via the hyper-rustls chain). With both enabled rustls cannot select a process-default provider on its own and panics on first client build. Install aws-lc-rs explicitly (idempotently) at the top of the environmentd, clusterd, balancerd, sqllogictest, and testdrive entrypoints. orchestratord already installs it. deny.toml gains skips for the duplicate versions the transport stack pulls (older tower/rustls-native-certs; newer rand/rand_core/getrandom/cpufeatures) and re-adds the launchdarkly-sdk-transport wrapper. Adds MetricsTransport unit tests, including test_metric_frozen_on_midstream_ error, modeling the exact incident-984 failure mode (200 OK then a mid-stream timeout): they assert the last_sse_time_seconds gauge freezes so the staleness alert can detect a stuck data source. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
dd4b798 to
93e1ea9
Compare
This was referenced Jul 6, 2026
7b6801e to
5780d0f
Compare
incident-984 was a runtime failure: the LaunchDarkly data source stopped reconnecting after its streaming connection died, silently wedging flag sync. The existing test/launchdarkly nightly covers value sync, persistence, targeting, and the kill switch, but nothing exercises reconnect after a mid-stream failure. This adds test/launchdarkly-reconnect, which reproduces the incident deterministically against a mock and needs no real LaunchDarkly credentials. The failure mode matters. The incident signature was a mid-body read timeout on a silently-dead connection, hyper::Error(Body, Kind(TimedOut)), which the eventsource client surfaces to the data source as a stream error. A TCP RST or a clean FIN does NOT reproduce it: those are retried inside the eventsource client on every SDK version and never reach the data source (verified empirically, nightly MaterializeInc#17018 stayed green with an RST-based mock even on the exact incident-era dependency stack). The mock serves an initial flag value (2 GiB) on the first TWO streaming connections and then goes silent holding each open, so the transport read timeout ends the stream. Two, because environmentd creates a short-lived bootstrap LD client at boot (load_remote_system_parameters) before the long-lived sync client, and the stall must hit the sync client (verified empirically, nightly MaterializeInc#17020: with only the first connection stalled, the sync client landed on connection two and got the updated value without any reconnect being exercised). Every later (reconnecting) client gets the updated value (3 GiB) plus heartbeats. Production changes, both hidden test knobs: - --launchdarkly-base-uri (env MZ_LAUNCHDARKLY_BASE_URI) overrides the SDK's streaming/polling/events endpoints with a single base URL via the SDK's ServiceEndpointsBuilder::relay_proxy, letting tests point the SDK at the mock. Also generally useful for relay-proxy setups. - MZ_LAUNCHDARKLY_READ_TIMEOUT overrides the transport's streaming read timeout (default 300s) so the test can trigger the timeout path in seconds. mzcompose.py boots environmentd against the mock with a 5s read timeout and asserts SHOW max_result_size reaches 3GB, which can only happen if the sync client's data source reconnected after the timeout. A regressed SDK stays stuck at 2GB and the assertion times out. Wired into the nightly pipeline. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…onnect test fails DO NOT MERGE. This is a throwaway verification layer on top of the upstream migration and its reconnect test. It downgrades the LaunchDarkly dependency stack to the exact pre-fix versions from incident-984 to prove the reconnect test catches the regression by going red. Pinned (matching the lockfile of the reverted MaterializeInc#35903, the incident build): - launchdarkly-server-sdk = 3.0.1 (predates rust-server-sdk#168, in 3.0.3) - eventsource-client = 0.17.1 (predates the reconnect fixes #134/#135, which shipped in 0.17.4) - launchdarkly-sdk-transport = 0.1.1 Pinning only the SDK to 3.0.1 is NOT sufficient: a nightly run of that configuration came back green, because the lock still resolved eventsource-client to 0.17.5, whose reconnect fix recovers the stream beneath the pre-fix SDK. The regression only reproduces with the pre-fix eventsource-client as well. The reconnect test cuts the first streaming connection with a TCP RST, the non-Eof error class those fixes address. On the incident-era stack the data source gives up and never reconnects, so the mock's updated value never arrives and `SHOW max_result_size` stays at 2GB until the assertion times out. A red `LaunchDarkly reconnect` nightly is the expected, desired outcome. The SDK is pinned as exactly "=3.0.1" in Cargo.toml. eventsource-client and launchdarkly-sdk-transport are transitive, so their pins live only in Cargo.lock (via cargo update --precise). Do not regenerate the lockfile on this branch, that would silently float them back to the fixed versions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
5780d0f to
8156f3f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part 3 of 3 in the LaunchDarkly upstream-SDK stack. Stacked on #37026.
What this does
One commit pinning the LD dependency stack to the exact versions from the incident-984 build (the lockfile of the reverted #35903):
launchdarkly-server-sdk=3.0.1(Cargo.toml + lock)eventsource-clientlaunchdarkly-sdk-transportWhat earlier rounds taught us
eventsource-client0.17.5, whose reconnect fix recovers the stream beneath the pre-fix SDK.#37026's mock now stalls the connection (held open, silent) so the SDK's read timeout fires — the exact incident signature,
hyper::Error(Body, Kind(TimedOut)).Expected result
The
LaunchDarkly reconnectnightly should go RED: on the incident-era stack the data source gets the timeout as an unhandled stream error and never reconnects, soSHOW max_result_sizestays at2GBuntil the assertion times out. That red is the proof the test guards the regression. Everything else stays green.Cargo.lockon this branch — the transitive pins would silently float back to the fixed versions.Stacking note: GitHub can't base a fork PR on another fork branch, so this targets
mainand its diff also contains the migration (#37025) and reconnect-test (#37026) commits. Review the top commit.