Skip to content

Add skip_record_validation_for indexer config for sampled backfill validation#1315

Open
vermatron wants to merge 1 commit into
block:mainfrom
vermatron:add-skip-record-validation-for-config
Open

Add skip_record_validation_for indexer config for sampled backfill validation#1315
vermatron wants to merge 1 commit into
block:mainfrom
vermatron:add-skip-record-validation-for-config

Conversation

@vermatron

Copy link
Copy Markdown

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_updates backfill knob and follows the same shape.

Design notes:

  • skip_record_validation_for maps a type name to a fraction in [0.0, 1.0]. 0.0 (or an absent key) validates everything, 1.0 skips everything, and values in between sample. The value is the fraction skipped, so 0.9 skips 90% and validates 10%.

  • The event envelope is always validated. Only the per-record schema walk gets sampled.

  • Skipping isn't silent. Processor counts skipped records per batch and logs one RecordValidationSkipped line with per-type counts, the same way it logs ElasticGraphIndexingLatencies. Logging per record would drown a 1.0 backfill 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 __typename can reach RecordPreparer. It now raises a typed RecordPreparer::UnknownTypeError, and Factory#build rescues it into a FailedEventError so one bad record fails on its own instead of crashing the batch. The rescue is narrow (a typed error, not a bare rescue 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-implement prepare_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:

  indexer:
    skip_record_validation_for:
      Widget: 0.9      # skip validation for 90% of Widget records, validate 10%
      Component: 1.0   # skip validation for all Component records
  • config.rb: new skip_record_validation_for JSON schema property (object, per-type number in [0, 1] additionalProperties: false, default {}); convert_values coerces rates to Float.
  • operation/factory.rb: new skip_validation?(type, event) helper; build skips record validation when sampled, records the skipped type on the result, and rescues UnknownTypeError. BuildResult gains validation_skipped_for.
  • processor.rb: aggregate RecordValidationSkipped log per batch when any record was skipped.
  • record_preparer.rb: new RecordPreparer::UnknownTypeError; the abstract-type fetch raises it with a value-free message.
  • indexer.rb: wire config.skip_record_validation_for into the factory.
  • RBS signatures updated for all of the above.
  • elasticgraph-local config_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 to Float (1 to 1.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 (stubbed Zlib.crc32 for both branches); retry stability; the derived-index path under skip; and the unknown-__typename case returning a FailedEventError instead of raising.
  • processor_spec.rb: a batch with skips logs one RecordValidationSkipped with the right count/counts_by_type; a batch with no skips logs none.

…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.
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants