fix(data-store): latch bytes-unsupported instead of re-probing every sync - #32
Open
vaidehi-figma wants to merge 1 commit into
Open
fix(data-store): latch bytes-unsupported instead of re-probing every sync#32vaidehi-figma wants to merge 1 commit into
vaidehi-figma wants to merge 1 commit into
Conversation
…sync `get_bytes` / `set_bytes` have default trait impls that return `BytesNotImplemented`, and whether an implementation overrides them is a static property of that implementation. The C FFI shim (`data_store_c.rs`) exposes no bytes hook at all, so every probe from a Go/C consumer refuses. Both the read and write paths treated that refusal as a fresh event, so a string-only data store logged a WARN on every specs sync: [Statsig::StatsigDataStoreSpecsAdapter] Failed to read specs from data store as bytes. Falling back to string read: Bytes method not implemented [Statsig::SpecStore] Data store bytes write is not implemented. Falling back to string write: Bytes method not implemented At the default 10s sync interval that is 12 warnings per minute per process, forever, plus two doomed FFI round-trips per sync. Figma's labmate runs as a daemonset, so this dominates its log volume in staging and production. Latch the first refusal in both places: - `StatsigDataStoreSpecsAdapter` gains a `bytes_unsupported` flag. Once set, `load_cached_specs` reads the plain-text string cache directly instead of re-probing `get_bytes`. The fallback warning is emitted on the first refusal only. - `SpecStore` gains a `DataStoreBytesWriteSupport` latch threaded into `write_specs_to_data_store`, which skips `set_bytes` once it is known to refuse. The string-fallback and protobuf-skip warnings get separate "already logged" flags so a protobuf response still reports its dropped write even if a plain-text response already reported the string fallback. Behavior is otherwise unchanged: the same keys are read and written, and genuine (non-`BytesNotImplemented`) data store errors still log every time. Covered by a new test asserting a string-only store is probed for bytes exactly once across many background syncs; it sees 4 probes without the fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vaidehi-figma
marked this pull request as ready for review
August 19, 2026 15:51
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.
Problem
DataStoreTrait::get_bytes/set_byteshave default impls that returnBytesNotImplemented, and whether an implementation overrides them is a static property of that implementation. The C FFI shim (statsig-ffi/src/data_store_c.rs) exposes no bytes hook at all, so every probe from a Go/C consumer refuses — forever.Both the read and write paths treated that refusal as a fresh event, so a string-only data store logs a WARN on every specs sync:
At the default 10s
specs_sync_interval_msthat's 12 warnings/minute/process, forever, plus two doomed FFI round-trips per sync. Figma'slabmateruns as a daemonset with an S3-backed data store, so this currently dominates its log volume in staging and production.Nothing is functionally broken — the string fallback works, flags evaluate correctly. It's noise plus a little wasted work.
Fix
Latch the first refusal in both places.
Read path —
StatsigDataStoreSpecsAdaptergains abytes_unsupported: AtomicBool. Once set,load_cached_specsgoes straight to the plain-text string read instead of re-probingget_bytes. The fallback warning is emitted on the first refusal only.Write path —
SpecStoregains aDataStoreBytesWriteSupportlatch, threaded intowrite_specs_to_data_store, which skipsset_bytesonce it's known to refuse. The string-fallback and protobuf-skip warnings get separate "already logged" flags, so a protobuf response still reports its dropped write even if a plain-text response already reported the string fallback — that one means specs aren't cached at all and shouldn't be swallowed.Behavior is otherwise unchanged: same keys read and written, and genuine (non-
BytesNotImplemented) data store errors still log on every occurrence.Testing
test_string_only_data_store_probes_bytes_once_across_syncsasserts a string-only store is probed for bytes exactly once across many background syncs. It reports 4 probes without the fix, 1 with it.cargo test --test data_store_bytes_adapter_tests --test proto_specs_usage_tests --test customized_specs_adapter_tests— all pass.cargo clippy --all-targetsclean; touched files are rustfmt-clean.Follow-ups (not in this PR)
figma3release solabmatecan pick this up (bothstatsig-goandbinaries-linux-gnutags), then bump the tworeplacedirectives in the figma monorepogo.mod.statsig-io/statsig-server-core0.22.0has the same per-synclog_w!and the same bytes-less C FFI.data_store_c.rs+ the Go binding, which would let C/Go consumers use thestatsig-brcache instead of plain text.