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

pkg/agentctl: FindCardinality, FindSamples do not invoke WAL.Close() hence leaks resources #506

Closed
odeke-em opened this issue Apr 1, 2021 · 2 comments · Fixed by #507
Closed
Labels
frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed.

Comments

@odeke-em
Copy link

odeke-em commented Apr 1, 2021

There is a missing defer WAL.Close() in FindCardinality and that’ll look like substantial resources will be leaked per

func FindCardinality(walDir string, job string, instance string) ([]Cardinality, error) {
w, err := wal.Open(nil, walDir)
if err != nil {
return nil, err
}
cardinality := map[string]int{}
err = walIterate(w, func(r *wal.Reader) error {
return collectCardinality(r, job, instance, cardinality)
})
if err != nil {
return nil, err
}
res := make([]Cardinality, 0, len(cardinality))
for k, v := range cardinality {
res = append(res, Cardinality{Metric: k, Instances: v})
}
return res, nil
}

ditto for

func FindSamples(walDir string, selectorStr string) ([]*SampleStats, error) {
w, err := wal.Open(nil, walDir)
if err != nil {
return nil, err
}
selector, err := parser.ParseMetricSelector(selectorStr)
if err != nil {
return nil, err

given the code in (*WAL).Close

@odeke-em odeke-em changed the title pkg/agentctl: FindCardinality does not invoke WAL.Close() hence leaks resources pkg/agentctl: FindCardinality, FindSamples do not invoke WAL.Close() hence leaks resources Apr 1, 2021
@rfratto
Copy link
Member

rfratto commented Apr 1, 2021

Thanks for reporting! It's not clear to me how much this actually leaks since they're not being written to, but they should definitely be closed.

@odeke-em
Copy link
Author

odeke-em commented Apr 2, 2021

Thank you for the fix and quick turnaround, @rfratto.

@github-actions github-actions bot added the frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed. label Feb 23, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants