Skip to content

Commit

Permalink
hotfix(core): don't hold on Data (#3926)
Browse files Browse the repository at this point in the history
The `Data` field of `core.ResultSignedBlock` was retained even after we constructed the respective header and moved on. This was due to the header consisting of pointers pointing to fields of `core.ResultSignedBlock`, retaining the whole structure, including the `Data` field. A BN, by default, has a header store cache of size 4096; thus, holding on `Data` with 8 MB blocks took around 32GiB of RAM.

Initially, we believed that the issue was with JSON unmarshalling. However, we were wrong about this being the whole story, as while profiles did confirm that the allocations did originate there, the allocated data wasn't cleaned up.  Kudos to @rach-id for helping us figuring this out!

This is a hotfix and is meant to be replaced with a better solution described on TODO. Also, the origin of this bug is yet to be confirmed by @rach-id by testing RAM usage with disabled header cache. 

---

🎱mb certified
  • Loading branch information
Wondertan committed Nov 15, 2024
1 parent 353141f commit 4aea81a
Showing 1 changed file with 6 additions and 1 deletion.
7 changes: 6 additions & 1 deletion core/exchange.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"time"

"github.com/tendermint/tendermint/types"
"golang.org/x/sync/errgroup"

libhead "github.com/celestiaorg/go-header"
Expand Down Expand Up @@ -177,7 +178,11 @@ func (ce *Exchange) getExtendedHeaderByHeight(ctx context.Context, height *int64
if err != nil {
return nil, fmt.Errorf("extending block data for height %d: %w", b.Header.Height, err)
}
// create extended header

// TODO(@Wondertan): This is a hack to deref Data, allowing GC to pick it up.
// The better footgun-less solution is to change core.ResultSignedBlock fields to be pointers instead of values.
b.Data = types.Data{}

eh, err := ce.construct(&b.Header, &b.Commit, &b.ValidatorSet, eds)
if err != nil {
panic(fmt.Errorf("constructing extended header for height %d: %w", b.Header.Height, err))
Expand Down

0 comments on commit 4aea81a

Please sign in to comment.