Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Patch for memory leak on event buffer #787

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

einarmo
Copy link
Collaborator

@einarmo einarmo commented Feb 21, 2025

This is the same issue as the other memory leak, except caused by the buffer for events instead of the one for datapoints...

The patch is even a little hackier, but I also realized that this feature only actually does anything if influxdb is enabled (since that is the only target where we use the state of the target as state for events).

Keep in mind that this entire feature is being removed completely in v3. To avoid breaking existing deployments I'm keeping it, ref discussion when we did this for the data point buffer, but I'm disabling it for deployments without influx, since it never worked for those.

This is the same issue as the other memory leak, except caused by the
buffer for events instead of the one for datapoints...

The patch is even a little hackier, but I also realized that this
feature only actually does _anything_ if influxdb is enabled (since that
is the only target where we use the state of the target as state for
events).

Keep in mind that this entire feature is being removed completely in v3.
To avoid breaking existing deployments I'm keeping it, ref discussion
when we did this for the data point buffer, but I'm disabling it for
deployments without influx, since it never worked for those.
Copy link

codecov bot commented Feb 21, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.95%. Comparing base (684353c) to head (c7cf8bd).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #787      +/-   ##
==========================================
- Coverage   80.05%   79.95%   -0.11%     
==========================================
  Files         134      134              
  Lines       18836    18839       +3     
  Branches     2801     2800       -1     
==========================================
- Hits        15079    15062      -17     
- Misses       2911     2930      +19     
- Partials      846      847       +1     
Files with missing lines Coverage Δ
Extractor/History/EventExtractionState.cs 98.03% <100.00%> (+2.12%) ⬆️
Extractor/Streamer.cs 88.45% <100.00%> (+0.03%) ⬆️

... and 5 files with indirect coverage changes

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.

1 participant