Resume mid-delta on the package metadata label path

Orin Naamanrequested to merge
628038-label-path-resumes-inside-a-delta into master

What does this MR do and why?

  • PackageMetadata v2 sync reads delta directories from a GCS bucket; each delta directory can hold several chunk files.
  • When a bucket label is present (the "label path"), the connector uses it instead of a listObjects call, listing recent deltas as <timestamp>-<chunkcount> entries.
  • caught_up? only compared the checkpoint's sequence against the newest label entry's timestamp, so an instance that stopped mid-delta was reported as caught up — the chunks it hadn't read yet were never read, silently.
  • filtered_entries only kept entries strictly newer than the checkpoint's sequence, so even after fixing caught_up?, the partially-read delta would still get dropped from the list.
  • The listObjects path never had this bug — it already resumes at the file right after the checkpoint.

Fix, all in ee/lib/gitlab/package_metadata/connector/gcp.rb:

  • caught_up? now requires the checkpoint to be at the last chunk of the newest delta, not just at its timestamp.
  • filtered_entries keeps the checkpoint's own delta in the list, since it may only be partially read.
  • files_from_entries resumes at checkpoint.chunk + 1 inside that one delta; every other delta still starts at chunk 0.
  • Net effect: the label path now returns the same files as the listObjects path for the same checkpoint.

Tests:

  • The existing spec encoded the bug. Two contexts used a chunk: 0 checkpoint against a label entry declaring 2 chunks, and expected the result to start at the next delta — asserting that the skipped chunk was fine. They now use a fully-read checkpoint, which keeps what their names say they test and leaves their expectations unchanged.
  • Three new contexts pin the fix: a checkpoint inside the newest delta, a checkpoint inside an older delta, and a checkpoint on the last chunk of a multi-chunk newest delta. Each was confirmed to fail against a mutated version of the method it covers.

References

Note

The exporter change is complementary, not a prerequisite — this fix stands on its own, and is what instances receiving the backport actually need.

Edited by Orin Naaman

Merge request reports