Skip to content

Conversation

@faec
Copy link
Contributor

@faec faec commented Aug 14, 2025

The NewRegistry method in elastic-agent-libs is inherently unsafe, especially in concurrent code: calling NewRegistry on a name that already exists causes a panic, but checking whether the name exists first is not atomic. The recently added GetOrCreateRegistry method is a safe alternative that checks the name's existence and initializes it atomically, and never panics. This PR removes the last remaining NewRegistry call in the apm-server repo.

Related issues

This is a precautionary followup to elastic/fleet-server#5170 and #17872, where a different NewRegistry call site caused a panic on server reload.

@faec faec requested a review from a team as a code owner August 14, 2025 13:26
@faec faec self-assigned this Aug 14, 2025
@github-actions
Copy link
Contributor

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

@mergify
Copy link
Contributor

mergify bot commented Aug 14, 2025

This pull request does not have a backport label. Could you fix it @faec? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-7.17 is the label to automatically backport to the 7.17 branch.
  • backport-8./d is the label to automatically backport to the 8./d branch. /d is the digit.
  • backport-9./d is the label to automatically backport to the 9./d branch. /d is the digit.
  • backport-active-all is the label that automatically backports to all active branches.
  • backport-active-8 is the label that automatically backports to all active minor branches for the 8 major.
  • backport-active-9 is the label that automatically backports to all active minor branches for the 9 major.

@faec faec added backport-9.0 Automated backport to the 9.0 branch backport-8.19 Automated backport to the 8.19 branch backport-9.1 Automated backport to the 9.1 branch labels Aug 14, 2025
@kruskall kruskall removed backport-9.0 Automated backport to the 9.0 branch backport-9.1 Automated backport to the 9.1 branch labels Aug 14, 2025
@kruskall
Copy link
Member

Thanks! 🙇

older 9.x version don't have local registries so it's a bit more complicated to switch. Change LGTM!

Can you sign your commits ?

@kruskall kruskall added the backport-9.1 Automated backport to the 9.1 branch label Aug 14, 2025
@carsonip
Copy link
Member

carsonip commented Aug 14, 2025

@kruskall Why remove backport-9.1? I thought it should go into both 8.19 and 9.1+

I see you have added it now

@faec faec force-pushed the remove-newregistry branch from a268b1d to 2254126 Compare August 14, 2025 17:23
@faec faec force-pushed the remove-newregistry branch from 2254126 to 700e0cb Compare August 14, 2025 17:29
@kruskall kruskall added this pull request to the merge queue Aug 14, 2025
Merged via the queue into elastic:main with commit 013ef6f Aug 14, 2025
19 checks passed
mergify bot pushed a commit that referenced this pull request Aug 14, 2025
mergify bot pushed a commit that referenced this pull request Aug 14, 2025
@faec faec deleted the remove-newregistry branch August 14, 2025 19:20
mergify bot added a commit that referenced this pull request Aug 14, 2025
(cherry picked from commit 013ef6f)

Co-authored-by: Fae Charlton <[email protected]>
mergify bot added a commit that referenced this pull request Aug 14, 2025
(cherry picked from commit 013ef6f)

Co-authored-by: Fae Charlton <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-8.19 Automated backport to the 8.19 branch backport-9.1 Automated backport to the 9.1 branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants