Skip to content

fix(metastore): keep the shard table entry of sources holding no shard - #6803

Open
sergiught wants to merge 1 commit into
quickwit-oss:mainfrom
sergiught:fix/shard-api-source-lost-on-metastore-reload
Open

sergiught wants to merge 1 commit into
quickwit-oss:mainfrom
sergiught:fix/shard-api-source-lost-on-metastore-reload

Conversation

@sergiught

Copy link
Copy Markdown

Description

Fixes the root cause behind #5782, where a file-backed (S3) metastore with SQS ingestion starts returning NotFound(Source) after a few hours.

FileBackedIndex.per_source_shards is meant to hold one entry per source. Both add_source and From<IndexMetadata> set it up that way. The serialization round trip breaks it:

  • serializing an index drops any source whose shard list is empty
  • deserializing re-creates entries only for sources where source_type() == SourceType::IngestV2

An SQS source is SourceParams::File(FileSourceParams::Notifications(_)). It keeps its checkpoint in the shard table, so use_shard_api returns true, but it reports SourceType::File. It falls through that restore.

Once its shards are pruned to zero and the index is persisted, every shard API call on a reloaded index goes through get_shards_for_source[_mut] and returns NotFound(EntityKind::Source).

flowchart LR
    A["in memory<br>sqs-source, no shards"]
    B["index.json<br>entry dropped"]
    C["after reload<br>entry still missing"]
    D["prune_shards<br>NotFound(Source)"]
    A -->|serialize| B -->|deserialize| C --> D
Loading

This lines up with the report. The source stays in metadata.sources, so the CLI still lists it. Restarting does not help, because the persisted index is already missing the entry. It surfaces as failed to prune shards error=NotFound(Source ...).

The reporters also found that Kafka and the ingest API kept working on the same index. Neither touches the shard API, and ingest v2 was already covered by the IngestV2 branch, which leaves SQS as the only source type in the gap.

The fix

Both sides now key off use_shard_api rather than SourceType::IngestV2, as @rdettai suggested in the issue.

Serialization keeps the entry for shard API sources and still skips every other source, so the format stays compact. Deserialization restores missing entries, so a fixed node repairs an already-broken index the moment it loads it, without anyone hand-editing the file in S3.

flowchart LR
    A["in memory<br>sqs-source, no shards"]
    B["index.json<br>sqs-source: []"]
    E["index.json from an<br>affected version<br>(no entry)"]
    C["after reload<br>entry present"]
    D["prune_shards works"]
    A -->|"serialize keeps it"| B -->|deserialize| C --> D
    E -->|"deserialize restores it"| C
Loading

Keeping the serialize side lets a node still running an affected version read an index written by a fixed one. That covers the red/black overlap described in the issue.

How was this PR tested?

Three new tests, each checked to fail when, and only when, the half it covers is reverted:

Test Guards
test_serialize_keeps_shardless_shard_api_source serialize side
test_deserialize_restores_dropped_shardless_shard_api_source deserialize side
test_file_backed_metastore_shard_api_sources_survive_reload end to end, through a real persist and reload

The end to end test reproduces the reported failure. It creates an index with an SQS source and an ingest v2 source, then has a second FileBackedMetastore reload the index from the same storage and call prune_shards. Without the fix it panics with NotFound(Source { index_id: "test-index", source_id: "sqs-source" }).

The shared shard suite previously used SourceConfig::ingest_v2() in all 8 of its tests and never exercised a reload, so neither source type had reload coverage.

The tests use RamStorage, which exercises the same code path as S3. metastore_resolver.rs maps Protocol::S3, Azure, Google, File and Ram all to MetastoreBackend::File, and load_index reads bytes through dyn Storage before handing identical JSON to the same deserialization path.

Commands run locally against this branch:

  • cargo test -p quickwit-metastore: 138 passed, 0 failed
  • test_file_backed_index_backward_compatibility passes with no change to the test-data/file-backed-index/*.expected.json goldens
  • cargo check --workspace --all-targets and cargo clippy -p quickwit-metastore --all-targets are clean
  • rustfmt is clean on both changed files, checked with stable

I could not run make test-all or make fmt locally, since nightly and the Docker services were unavailable on this machine. A maintainer may want to kick off /ci-run-all-tests.

Notes for reviewers

  • v0.9.0 carries the same SourceType::IngestV2 check. The released v0.8.x line predates use_shard_api and SQS file sources, so it cannot hit this. The 0.8.0 build in the report (3a070c8) is a main build from 2025-04-23, not the release.
  • The repair happens in the binary, not in the data. A fixed node recovers on load, while an unfixed node reading the same bucket stays broken until it is replaced.
  • This does not make the two-metastore topology safe. The reporters ran one metastore per cluster during red/black deployments, and the file-backed metastore is last writer wins over the whole index.json, so a write race can still discard splits, shards or checkpoints. That hazard is untouched here, which is why the commit says See #5782 rather than Closes.
  • use_shard_api panics on SourceParams::Stdin. Deserialization now calls it for every source on every index load, which is wider exposure than the Postgres metastore's per-publish call sites. It looks unreachable, since SourceConfigForSerialization::validate_and_build rejects Stdin and it cannot reach persisted metadata, but a second opinion would be welcome. Returning false instead would silently accept a state the function exists to reject.
  • Pre-existing and untouched: delete_source drops a source from the metadata but leaves its per_source_shards entry, so a deleted source with non-empty shards round trips its stale shards indefinitely.

A source whose checkpoint is stored in the shard table holds no shard
until its first one is opened. The file-backed metastore dropped such an
empty entry when serializing an index, and restored it on load only for
`SourceType::IngestV2`.

An SQS file source stores its checkpoint in the shard table but reports
`SourceType::File`, so it fell through that restore. Once its shards had
been pruned and the index was persisted, every shard API call on the
reloaded index failed with `NotFound(Source)`, while the source itself
stayed visible in the index metadata. Restarting did not help, since the
persisted index was already missing the entry.

Key both sides off `use_shard_api`, which covers every source whose
checkpoint lives in the shard table. Restoring the entry on
deserialization also repairs indexes already persisted without it.

See quickwit-oss#5782
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.

1 participant