Skip to content

Fix space leak in nri-observability - #141

Merged
omnibs merged 9 commits into
trunkfrom
test-memory-leak
Sep 23, 2025
Merged

Fix space leak in nri-observability#141
omnibs merged 9 commits into
trunkfrom
test-memory-leak

Conversation

@omnibs

@omnibs omnibs commented Sep 22, 2025

Copy link
Copy Markdown
Member

Since #134 was deployed to prod, NRI's highest traffic service started slowly increasing memory usage over time, at a rate of around 5MiB/min.

This PR introduces a test app that reproduces the memory leak, and a fix.

The issue was elusive:

  • Profiling with -hm pointed fingers at Data.Conduit.Internal
    • This blogpost made me pretty certain it was the new conduit, but I don't fully follow the blogpost, in all honesty
    • -fno-full-laziness didn't help
  • ndmitchell's recipe for finding space leaks didn't work
    • Using -K1K didn't make my leaky test app blow up
  • Profiling with -hy showed an Int was leaking, and Waj's PR did introduce an Int inside the STM in the handler

I was still suspicious of Conduit, so while googling for conduit stm space leak I landed on the strict-stm library, which claimed to have helped make cardano-node leak-free.

The library was a drop-in replacement for stm, and using it made our leak go away.

before after
image image

@brian-carroll brian-carroll left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent! Great detective work! 🕵️ 🔍

@omnibs
omnibs added this pull request to the merge queue Sep 23, 2025
Merged via the queue into trunk with commit 4e5b8f2 Sep 23, 2025
2 checks passed
@omnibs
omnibs deleted the test-memory-leak branch September 24, 2025 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants