Conversation
…nment Phase 4 of forward-porting concurrent multi-key support from v8 (#817). A credential payload that parses but cannot produce a usable accepted set is now refused before MessageReceiver.Upsert records its version. The ordering is the point: Upsert deduplicates by version, so a refused payload that had already advanced the version would make LaunchDarkly's replay of it a no-op, leaving the environment serving credentials it should have replaced with no way back short of restarting the process. A refused environment keeps the credentials it had. The stream reconnects, because the service cannot learn that relay refused the payload and would otherwise send nothing further. Well-formed environments in the same put are applied as usual. persistPut substitutes each refused environment's last-good cached entry, so a bad payload does not drop those environments from the auto-config cache, or empty it when every environment in the put was refused. If the prior snapshot cannot be read there is no safe cache to assemble, so it is left alone. The stream also now moves to the extended retry delays after a second consecutive unusable event, rather than reconnecting on the short delays indefinitely. One bad event may be a single corruption and is worth asking again at once; a second means the reconnect brought the same payload back. This closes a finding from the review of #817. Also removes StreamManager.lastKnownEnvs, dead since #401 removed its last read.
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.
Summary
Phase 4 of forward-porting concurrent multi-key support from v8 (#817) to v9. Stacked on #896; GitHub retargets it to
feat/concurrent-keys-v9as the stack merges.A credential payload that parses as JSON but cannot produce a usable accepted set is now refused before
MessageReceiver.Upsertrecords its version.The ordering is the whole point
Upsertdeduplicates by version. A refused payload that had already advanced the version would make LaunchDarkly's replay of that same payload a no-op, leaving the environment serving credentials it should have replaced with no way back short of restarting the process.TestReplayOfTheSameVersionIsAppliedAfterARefusalis the test that matters, and it is control-verified: move the validation back after theUpsertand it fails.What happens to a refused payload
The environment keeps the credentials it already had. The stream reconnects, because the service cannot learn that relay refused the payload and would otherwise send nothing further. Well-formed environments in the same put are applied as usual.
persistPutsubstitutes each refused environment's last-good cached entry before writing, so a bad payload does not drop those environments from the auto-config cache, or empty it when every environment in the put was refused. If the prior snapshot cannot be read there is no safe cache to assemble, so it is left untouched rather than rewritten from partial data.Backoff on repeated unusable events
The stream now moves to the extended retry delays after a second consecutive unusable event. This closes the second Medium from the review of #817: the malformed-payload reconnect goes through
stream.Restart(), which bypasses the retry strategy, so it reconnected on the short delays indefinitely.Escalating on the first event was the obvious implementation and it is wrong — it broke
TestStreamStatusRecordsInvalidDataError, and rightly so, since one corrupt event would have cost minutes of recovery. One bad event may be a single corruption and is worth asking again at once; a second means the reconnect brought the same payload back. The counter resets on any event relay can use.This applies to JSON-malformed events as well as credential-malformed ones, which is slightly beyond the ticket but the same defect class, and treating only one of them specially would be arbitrary.
Testing note
The last-good cache preservation is covered by a recording cache fake at the stream-manager level, rather than the Redis-backed test v8 added for it. That exercises the substitution logic without a Redis dependency, and
internal/autoconfigcachealready has its own store round-trip tests. Happy to add the integration-level version if it is wanted.Also removes
StreamManager.lastKnownEnvs, dead since #401 removed its last read in June 2024.Part of SDK-3202, under epic SDK-3188.
Note
Overview
Auto-config streaming now rejects environment payloads that parse as JSON but cannot build a usable credential set, without dropping the environment or poisoning version tracking.
Validation runs before
MessageReceiver.Upserton PUT and PATCH paths viavalidateCredentialPayload(BuildAcceptedSet). Refused environments keep their prior credentials; the stream restarts so LaunchDarkly can replay a fixed payload at the same version. Other environments in a mixed PUT still apply.persistPutmerges refused entries from the previous cache snapshot on write (or skips the write if the prior snapshot cannot be read).After two consecutive unusable events (bad JSON or bad credentials), reconnects switch to the extended SSE retry profile so short-delay spin stops; the counter resets on any usable event. Removes unused
lastKnownEnvs. New stream-manager tests cover refusal, replay-after-refusal, partial PUT, and cache preservation.Reviewed by Cursor Bugbot for commit a4a5024. Bugbot is set up for automated code reviews on this repo. Configure here.