Skip to content
This repository was archived by the owner on Jan 20, 2026. It is now read-only.

Fix memory leak due to emitting infinite events during snapshot#391

Merged
yzang2019 merged 3 commits intomainfrom
yzang/SEI-6277
Jan 9, 2024
Merged

Fix memory leak due to emitting infinite events during snapshot#391
yzang2019 merged 3 commits intomainfrom
yzang/SEI-6277

Conversation

@yzang2019
Copy link
Collaborator

Describe your changes and provide context

Problem:
Currently when the wasmd snapshotter export snapshot, it will create a CacheMultiStore and use cachekv to access the historical state, which will then create a mergeIterator to iterate through key/values. In MergeIterator, everytime we access a key or value, it will emit a resource access events for "iterate" operation, but there's no limit on how many events to be emitted, so as we append more and more events to event manager during wasm snapshot, it starts to fill up the memory.

Solution:
We can completely get rid of this iterator events, that event is not useful at all.

Testing performed to validate your change

Tested and validated on atlantic-2

// If parent is invalid, get the cache value.
if !iter.parent.Valid() {
value := iter.cache.Value()
iter.eventManager.EmitResourceAccessReadEvent("iterator", iter.storeKey, iter.cache.Key(), value)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These events are causing memory leak during snapshot creation, it should be safe to delete since those events are not being used in critical path at all

@yzang2019 yzang2019 requested a review from Kbhat1 January 3, 2024 18:12
@codecov
Copy link

codecov bot commented Jan 3, 2024

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (31cfbd9) 54.80% compared to head (5c38764) 54.79%.
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #391      +/-   ##
==========================================
- Coverage   54.80%   54.79%   -0.01%     
==========================================
  Files         622      622              
  Lines       52168    52166       -2     
==========================================
- Hits        28590    28585       -5     
- Misses      21495    21498       +3     
  Partials     2083     2083              
Files Coverage Δ
store/cachekv/mergeiterator.go 79.22% <100.00%> (-0.66%) ⬇️
store/cachekv/store.go 80.82% <100.00%> (ø)
storev2/rootmulti/store.go 3.09% <0.00%> (-0.02%) ⬇️

@yzang2019 yzang2019 merged commit c99258c into main Jan 9, 2024
@yzang2019 yzang2019 deleted the yzang/SEI-6277 branch January 9, 2024 18:42
yzang2019 added a commit that referenced this pull request Jan 17, 2024
## Describe your changes and provide context
This PR reverts the change in
#353 and
#391 until we have OCC
fully enabled.

## Testing performed to validate your change
Unit test coverage
codchen pushed a commit that referenced this pull request Feb 6, 2024
## Describe your changes and provide context
This PR reverts the change in
#353 and
#391 until we have OCC
fully enabled.

## Testing performed to validate your change
Unit test coverage
masih pushed a commit to sei-protocol/sei-chain that referenced this pull request Sep 29, 2025
## Describe your changes and provide context
This PR reverts the change in
sei-protocol/sei-cosmos#353 and
sei-protocol/sei-cosmos#391 until we have OCC
fully enabled.

## Testing performed to validate your change
Unit test coverage
masih pushed a commit to sei-protocol/sei-chain that referenced this pull request Sep 30, 2025
## Describe your changes and provide context
This PR reverts the change in
sei-protocol/sei-cosmos#353 and
sei-protocol/sei-cosmos#391 until we have OCC
fully enabled.

## Testing performed to validate your change
Unit test coverage
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants