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

fix(CommitteeObserver): post consensus mem leak #1859

Open
wants to merge 4 commits into
base: stage
Choose a base branch
from

Conversation

y0sher
Copy link
Contributor

@y0sher y0sher commented Nov 19, 2024

Description

Closes #1857

Remove 12 old slots after each message processing.

NOTE: maybe we need tracking of latest actual slot in order to ignore old messages? what if we already removed but got new msg?

@y0sher y0sher changed the base branch from main to stage November 19, 2024 13:58
Copy link
Contributor

@nkryuchkov nkryuchkov left a comment

Choose a reason for hiding this comment

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

Well spotted! I'd just add more comments. Approving but please fix linter

protocol/v2/ssv/validator/non_committee_validator.go Outdated Show resolved Hide resolved
protocol/v2/ssv/validator/non_committee_validator.go Outdated Show resolved Hide resolved
protocol/v2/ssv/validator/non_committee_validator.go Outdated Show resolved Hide resolved
@nkryuchkov nkryuchkov changed the title fix: CommitteeObserver - post conesnsus mem leak fix(CommitteeObserver): post consensus mem leak Nov 19, 2024
@nkryuchkov nkryuchkov self-requested a review November 19, 2024 15:17
Copy link
Contributor

@iurii-ssv iurii-ssv left a comment

Choose a reason for hiding this comment

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

Could alternatively maybe use ttlcache.Cache (seems like we use it a lot whenever we need to keep map from growing).

@y0sher y0sher requested a review from moshe-blox November 21, 2024 13:46
@y0sher y0sher force-pushed the fix/ncv-postcons-memleak branch from 2e17fa6 to e4ed505 Compare November 22, 2024 13:32
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.

exporter: memory leak in CommitteeObserver.postConsensusContainer
4 participants