Repository navigation
Conversation
Docs preview (local build)Handbook preview: https://docs-v3-preview.elastic.dev/elastic/docs-builder/pull/4122/ Sample changed pages: |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Requesting changes due to a high-confidence correctness regression noted inline that can leave product registry state stale after partial failure/retry.
What is this? | From workflow: PR Review
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
d656566 to
74e7c01
Compare
There was a problem hiding this comment.
Requesting changes based on a high-confidence correctness regression noted inline.
What is this? | From workflow: PR Review
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
There was a problem hiding this comment.
Requesting changes due to a correctness issue called out inline.
What is this? | From workflow: PR Review
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
There was a problem hiding this comment.
Requesting changes due to a high-confidence correctness issue in the no-late-notes retry path that can leave stale amend sidecar/registry state.
What is this? | From workflow: PR Review
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
There was a problem hiding this comment.
Requesting changes due to a high-confidence correctness regression noted inline.
What is this? | From workflow: PR Review
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
8415133 to
ebb39af
Compare
ebb39af to
97dbbf2
Compare
There was a problem hiding this comment.
Approved: no new actionable issues found.
What is this? | From workflow: PR Review
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
97dbbf2 to
38bf4d9
Compare
![infra-vault-gh-plugin-prod[bot]](https://avatars.githubusercontent.com/u/6764390?s=60&v=4){kind=small}
| if (write.Notes.Count == 0) | ||
| { | ||
| if (touchedProducts.Contains(write.Product)) | ||
| emptyIndexes.Add(new NoteAmendEmptyIndex(write.Product, indexKey)); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Severity: P2 (Medium)
Vanished indexes are only queued for deletion when their product touched a sidecar. If a note is deleted or retargeted before any bundle ships, ProcessProductAsync returns false and the old product-scoped index remains, even on subsequent reconciles. It still lists the old note path; CDN note fetching treats that list as authoritative and fails bundling when the deleted note cannot be fetched. Allow stale-index cleanup when no published bundle needs sidecar work.
Generated by Libra. React with 👍 or 👎 to give feedback on this comment.
| foreach (var (product, version) in vanished) | ||
| { | ||
| if (!map.TryGetValue(product, out var byVer)) | ||
| map[product] = byVer = [with(StringComparer.Ordinal)]; | ||
| if (!byVer.ContainsKey(version)) | ||
| byVer[version] = []; | ||
| } |
There was a problem hiding this comment.
Severity: P3 (Low)
These empty buckets also represent failed reads, not just vanished notes: ExtractProductVersionsAsync catches a source GET failure and returns no pairs. Passing that result to amend reconciliation now deletes a live note's published .amend-notes sidecar and refreshes the registry without it. The batch can then succeed, so the note stays unpublished until another event triggers reconciliation. Propagate source-read failures instead of treating an incomplete inventory as evidence of deletion.
Generated by Libra. React with 👍 or 👎 to give feedback on this comment.
| AddShallow(shallowWork, bundleScope, messageId); | ||
| foreach (var empty in amend.EmptyProductIndexes) | ||
| { | ||
| if (!string.Equals(empty.Product, product, StringComparison.Ordinal)) | ||
| continue; | ||
| await notesReconciler.DeleteIndexAsync(empty.Key, ctx); |
There was a problem hiding this comment.
Severity: P3 (Low)
Keep the vanished-product index until the bundle shallow-map update also succeeds. On the last-note deletion, this removes the retry breadcrumb before the later shallow pass runs. If writing bundle/registry.json fails, the message is redelivered, but the product is now absent from the notes map and no bundle shallow work is scheduled on retry. The shallow change token therefore remains stale despite a successful retry.
Generated by Libra. React with 👍 or 👎 to give feedback on this comment.
38bf4d9 to
75cddb8
Compare
Mpdreamz
left a comment
Mpdreamz
left a comment
There was a problem hiding this comment.
Reviewed the source and tests. The scoping of amend to the products in the notes map, the reconciler-owned sidecars and the retry-after-registry-failure path all look right. One gap I'd fix before merging.
A vanished product-scoped index is only deleted when its product was "touched". RewriteNotesIndexesAsync adds an empty index to emptyIndexes only if (touchedProducts.Contains(write.Product)). ProcessProductAsync returns false, so the product is not touched, when the product has no bundle registry (404, or no bundles) or no parent bundle matches the version. In both cases the index is not rewritten and not deleted.
Scenario:
- A note for
kibanaat9.0.0is published, sonotes-kibana-9.0.0.jsonlists it. - The note is removed before any
9.0.0bundle exists. ReconcileRepoAsyncreturnskibana → 9.0.0 → []and keeps the key, which is whatReconcileRepo_ProductScopedStale_ReturnedAsEmptyBucketWithoutDeletingasserts.- Amend finds no registry or parent bundle, so
kibanais not touched, and the key stays with its old note list. - When
kibana@9.0.0is bundled later,FetchNotesAsyncfinds the product index, so it does not fall back to the legacy key. The removed note 404s, and the fetch reports "listed in the notes index but could not be fetched".
Every later reconcile finds the same vanished index and skips it the same way. Before this stack, DeleteStaleIndexes removed the stale index in the same pass.
I did not run this. It comes from reading RewriteNotesIndexesAsync, ProcessProductAsync and the tests: NoPublishedBundle_NoAmend and NoBundleForVersion_NoAmend_SeqRemainsZero don't pass a vanished product, and the other tests cover only the case where a bundle exists.
Suggested fix: when a product has no sidecar to drop (no registry, or no matching parent bundle), delete its empty index in the same pass. Then add a test for a vanished product with no published bundle, asserting the key is deleted.
I merged #4120, so this branch needs a rebase onto main.
Summary
NoteAmendis scoped to products in the notes map and those products get a registry plus shallow rebuild.This PR is the third phase of fixes for changelog note registry issues.
Background
Relates to #4090, #4103
Per #4103 (comment), since the related issues touch the same areas of the code and have a significant, overlapping impact, this PR is part of a multi-PR phased approach.
Details
NoteAmend. It no longer lists every
bundle/{product}/prefix. It only processes products that appear in the product×version notes map, writesbundle_seqon product-scoped indexes, and still refreshes the legacynotes-{version}.jsonpath list. Other products’.amend-notes.yamlfiles are not deleted.ReconcileAsyncreturns products that had a sidecar write, skip-unchanged, or delete.Scrubber. After notes work, each returned product runs
BundleRegistryReconciler.ReconcileGroupAsyncand is added to the bundle shallow map, using the same SQS message ids. A registry failure fails those notes messages.Tests and docs. Isolation coverage is in
NoteAmendReconcilerTestsandScrubberProcessorTests.cmd-note.mdandchangelog-bundle-registry.mddescribe the per-product sidecar and registry rebuild.Screenshots
When I revert work-arounds (remove local copy of note amend bundles and change the directive to pull from CDN):
:::{changelog} cloud-enterprise :config: /changelog.yml :link-visibility: keep-links :description-visibility: keep-descriptions :highlights: :::...it's now successfully finding and rendering the
cloud-4.2.0.amend-notes.yaml:I do not know how to test the other S3 file handling changes until the stacked PRs are merged.
Generative AI disclosure
Tool(s) and model(s) used: Cursor Grok 4.6