Skip to content

fix: leaking cluster clients in EmbeddedDMap.Scan#270

Merged
buraksezer merged 2 commits into
olric-data:masterfrom
phmx:fix/issue263/mem-leak-in-dmap-scan
Oct 28, 2025
Merged

fix: leaking cluster clients in EmbeddedDMap.Scan#270
buraksezer merged 2 commits into
olric-data:masterfrom
phmx:fix/issue263/mem-leak-in-dmap-scan

Conversation

@phmx
Copy link
Copy Markdown
Contributor

@phmx phmx commented Aug 22, 2025

Fix for #263

Problem:
EmbeddedDMap.Scan function creates cluster client which stays alive with fetchRoutingTablePeriodically in a separate go-routine created in NewClusterClient. This causes memory leaks similar to #209.

Solution:

  • make ClusterClient.fetchRoutingTable interruptable by using cluster client's ctx inside;
  • make DMap Close-able and call ClusterClient.Close for cluster client cached by EmbeddedDMap.

NOTE: I couldn't come up with a more clean test and resorted to outputting stacks and mem stats as in the original problem description. Maybe we could check that there are no more than 2 go-routines left?

@buraksezer buraksezer self-requested a review August 25, 2025 10:31
Fix for olric-data#263

Problem:
`EmbeddedDMap.Scan` function creates cluster client which stays alive
with `fetchRoutingTablePeriodically` in a separate go-routine created in
`NewClusterClient`. This causes memory leaks similar to
olric-data#209.

Solution:
* make `ClusterClient.fetchRoutingTable` interruptable by using cluster
  client's `ctx` inside;
* make `DMap` `Close`-able and call `ClusterClient.Close` for cluster
  client cached by `EmbeddedDMap`.
@phmx phmx force-pushed the fix/issue263/mem-leak-in-dmap-scan branch from 3d8eb7b to 1ad980c Compare September 30, 2025 08:48
@marclop
Copy link
Copy Markdown

marclop commented Oct 28, 2025

It'd be nice to get this merged

@buraksezer buraksezer merged commit 6991973 into olric-data:master Oct 28, 2025
4 checks passed
@buraksezer
Copy link
Copy Markdown
Collaborator

It'd be nice to get this merged

Hey @marclop, just a heads up — the PR has been merged and is part of the Olric 0.7.1 release. Thanks for the patience! 🙌

I released the new version recently.

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.

3 participants