Add skip_record_validation_for indexer config for sampled backfill validation#1315
Open
vermatron wants to merge 1 commit into
Open
Add skip_record_validation_for indexer config for sampled backfill validation#1315vermatron wants to merge 1 commit into
skip_record_validation_for indexer config for sampled backfill validation#1315vermatron wants to merge 1 commit into
Conversation
…validation Adds an indexer config option that skips per-record JSON schema validation for a configurable fraction of records, keyed by GraphQL type. It exists for backfills of trusted, pre-validated data, where the per-record schema walk is a meaningful ingest cost and the datastore mappings provide a coarse backstop. `skip_record_validation_for` maps a type name to a fraction in `[0.0, 1.0]`: `0.0` (or an absent key) validates every record, `1.0` skips every record, and values in between sample. The skip decision is deterministic per event id -- a stable `Zlib.crc32` of `EventID#to_s` buckets each event -- so the same event makes the same decision on retry across indexer pods (`String#hash` is unsuitable: `RUBY_HASH_SEED` is per-process). The event envelope is always validated regardless of the sampling rate. Two supporting pieces: - `RecordPreparer::UnknownTypeError` (a `KeyError` subtype) is raised when a skipped record reaches the preparer with a missing/unknown abstract-type `__typename`. `Factory#build` rescues it and returns a structured `FailedEventError` rather than letting an exception escape, with a message that omits the offending value. - Skipping a safety check is never silent: `Processor` tallies skipped records per batch and logs a single aggregate `RecordValidationSkipped` entry (with per-type counts), mirroring the batch-level `ElasticGraphIndexingLatencies` log. Per-record logging would be untenable at backfill scale.
vermatron
requested review from
BrianSigafoos-SQ,
bsorbo,
ellisandrews-toast,
jwils,
jwondrusch,
marcdaniels-toast,
myronmarston and
rossroberts-toast
as code owners
July 22, 2026 09:21
|
|
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.
Why
During large backfills of already-validated data, per-record JSON schema validation is wasted work. Every record walks the full schema (regex, enum, min/max, format, abstract-type discriminators) even though the source has already been validated upstream. Today there's no way to trade that cost for throughput.
This adds a config option that skips per-record validation for a chosen fraction of records, per GraphQL type, while keeping a sampled slice validated as a canary so schema drift still surfaces. It's a sibling of the existing
skip_derived_indexing_type_updatesbackfill knob and follows the same shape.Design notes:
skip_record_validation_formaps a type name to a fraction in[0.0, 1.0].0.0(or an absent key) validates everything,1.0skips everything, and values in between sample. The value is the fraction skipped, so0.9skips 90% and validates 10%.The event envelope is always validated. Only the per-record schema walk gets sampled.
Skipping isn't silent.
Processorcounts skipped records per batch and logs oneRecordValidationSkippedline with per-type counts, the same way it logsElasticGraphIndexingLatencies. Logging per record would drown a1.0backfill in log lines, so the count is aggregated per batch.Safety net for skipped abstract-type records: once validation is off, a record with a missing or unknown
__typenamecan reachRecordPreparer. It now raises a typedRecordPreparer::UnknownTypeError, andFactory#buildrescues it into aFailedEventErrorso one bad record fails on its own instead of crashing the batch. The rescue is narrow (a typed error, not a barerescue KeyError) so it can't hide schema-artifact bugs, and its message drops the offending value to avoid leaking record data. A pre-walk check was considered and dropped: it would re-implementprepare_for_index's recursion over nested fields and drift out of sync with it.The field defaults to
{}, so nothing changes unless you set it. Additive and minor-release-safe.What
Config:
config.rb: newskip_record_validation_forJSON schema property (object, per-type number in[0, 1]additionalProperties: false, default{});convert_valuescoerces rates toFloat.operation/factory.rb: newskip_validation?(type, event)helper;buildskips record validation when sampled, records the skipped type on the result, and rescuesUnknownTypeError.BuildResultgainsvalidation_skipped_for.processor.rb: aggregateRecordValidationSkippedlog per batch when any record was skipped.record_preparer.rb: newRecordPreparer::UnknownTypeError; the abstract-typefetchraises it with a value-free message.indexer.rb: wireconfig.skip_record_validation_forinto the factory.elasticgraph-localconfig_schema.yaml: the new property added to the shared config validation schema.Verification
script/run_specs(COVERAGE=1, real Elasticsearch): 5222 examples, 0 failures. Coverage holds at the project's expected level (the one non-100% file,gem_spec.rb, is pre-existing and untouched here).script/type_check(Steep): no type errors.script/lint(Standard Ruby): 890 files, no offenses.script/spellcheck(codespell): clean.bundle exec rake schema_artifacts:check: up to date (runtime config only, no artifact changes).bundle exec rake site:validate: 148 runs, 0 failures.New tests:
config_spec.rb: integer YAML rates coerce toFloat(1to1.0), and out-of-range rates (1.5,-0.1) are rejected at config load.operation/factory_spec.rb: a skipped type builds operations without record validation; non-skipped types still fail on bad records; envelope validation still runs for skipped types; fractional sampling (stubbedZlib.crc32for both branches); retry stability; the derived-index path under skip; and the unknown-__typenamecase returning aFailedEventErrorinstead of raising.processor_spec.rb: a batch with skips logs oneRecordValidationSkippedwith the rightcount/counts_by_type; a batch with no skips logs none.