Skip to content

replay: eviction fix #8647

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.

Sign up for GitHub

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

Open
yufeng-jump wants to merge 1 commit into
base: main
Choose a base branch
from
+52 −22
Open

replay: eviction fix #8647

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/discof/replay/fd_replay_tile.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2153,8 +2153,7 @@ after_credit( fd_replay_tile_t * ctx,
return;
}

if( FD_UNLIKELY( fd_banks_prune_dead_banks( ctx->banks ) ) ) {
// FIXME: anything pruned from banks should also be pruned from txncache and accdb
if( FD_UNLIKELY( fd_banks_prune_dead_banks( ctx->banks, ctx->txncache, ctx->accdb_admin, ctx->progcache_admin ) ) ) {
*charge_busy = 1;
*opt_poll_in = 0;
return;
Expand Down
1 change: 0 additions & 1 deletion src/flamenco/accdb/fd_accdb_admin_v1.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,6 @@ fd_accdb_v1_cancel( fd_accdb_admin_t * accdb_,
}
fd_funk_txn_t * txn = fd_funk_txn_map_query_ele( query );

fd_accdb_txn_cancel_next_list( accdb, txn );
fd_accdb_txn_cancel_tree( accdb, txn );
}

Expand Down
7 changes: 5 additions & 2 deletions src/flamenco/accdb/fd_accdb_admin_v2.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,12 @@ fd_accdb_v2_attach_child( fd_accdb_admin_t * admin_,
}

void
fd_accdb_v2_cancel( fd_accdb_admin_t * admin,
fd_accdb_v2_cancel( fd_accdb_admin_t * admin_,
fd_funk_txn_xid_t const * xid ) {
fd_accdb_v1_cancel( admin, xid );
fd_accdb_admin_v2_t * admin = downcast( admin_ );
admin->base.accdb_type = FD_ACCDB_TYPE_V1;
fd_accdb_v1_cancel( admin_, xid );
admin->base.accdb_type = FD_ACCDB_TYPE_V2;
}

static void
Expand Down
1 change: 0 additions & 1 deletion src/flamenco/progcache/fd_progcache_admin.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,6 @@ fd_progcache_txn_cancel( fd_progcache_admin_t * cache,
}
fd_funk_txn_t * txn = fd_funk_txn_map_query_ele( query );

fd_progcache_txn_cancel_next_list( cache, txn );
fd_progcache_txn_cancel_tree( cache, txn );
}

Expand Down
20 changes: 19 additions & 1 deletion src/flamenco/runtime/fd_bank.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
#include "fd_bank.h"
#include "fd_runtime_const.h"
#include "../rewards/fd_stake_rewards.h"
#include "fd_txncache.h"
#include "../accdb/fd_accdb_admin.h"
#include "../progcache/fd_progcache_admin.h"

ulong
fd_bank_align( void ) {
Expand Down Expand Up @@ -1034,7 +1037,10 @@ fd_banks_mark_bank_dead( fd_banks_t * banks,
}

int
fd_banks_prune_dead_banks( fd_banks_t * banks ) {
fd_banks_prune_dead_banks( fd_banks_t * banks,
fd_txncache_t * txncache,
fd_accdb_admin_t * accdb_admin,
fd_progcache_admin_t * progcache_admin ) {
fd_rwlock_write( &banks->locks->banks_lock );

int any_pruned = 0;
Expand All @@ -1054,6 +1060,18 @@ fd_banks_prune_dead_banks( fd_banks_t * banks ) {

FD_LOG_DEBUG(( "pruning dead bank (idx=%lu)", bank->idx ));

if( txncache && bank->flags&FD_BANK_FLAGS_REPLAYABLE ) {
fd_txncache_cancel_fork( txncache, bank->txncache_fork_id );

fd_bank_t bank_l[1];
if( FD_UNLIKELY( !fd_banks_bank_query( bank_l, banks, bank->idx ) ) ) {
FD_LOG_CRIT(( "invariant violation: bank is NULL for bank index %lu", bank->idx ));
}
fd_funk_txn_xid_t xid = { .ul = { fd_bank_slot_get( bank_l ), bank->idx } };
fd_accdb_cancel( accdb_admin, &xid );
fd_progcache_txn_cancel( progcache_admin, &xid );
}

bank->flags = 0UL;

/* There are a few cases to consider:
Expand Down
16 changes: 14 additions & 2 deletions src/flamenco/runtime/fd_bank.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,15 @@
#include "../../ballet/lthash/fd_lthash.h"
#include "fd_txncache_shmem.h"

struct fd_txncache_private;
typedef struct fd_txncache_private fd_txncache_t;

struct fd_accdb_admin;
typedef struct fd_accdb_admin fd_accdb_admin_t;

struct fd_progcache_admin;
typedef struct fd_progcache_admin fd_progcache_admin_t;

Comment on lines 14 to +23
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

The new forward typedefs for fd_txncache_t / fd_accdb_admin_t / fd_progcache_admin_t are likely to cause typedef redefinition errors in any TU that includes this header alongside the canonical headers that already typedef these names (e.g. fd_txncache.h, fd_accdb_base.h/fd_accdb_admin.h, fd_progcache_admin.h). For example, fd_bank.c now includes fd_txncache.h after fd_bank.h, which will redeclare fd_txncache_t. Prefer removing these typedefs from fd_bank.h and either (a) include the canonical header(s) that define the typedefs, or (b) use only struct ...; forward declarations and change the prune API to accept struct fd_* * parameters to avoid needing the typedefs here.

Suggested change
struct fd_txncache_private;
typedef struct fd_txncache_private fd_txncache_t;
struct fd_accdb_admin;
typedef struct fd_accdb_admin fd_accdb_admin_t;
struct fd_progcache_admin;
typedef struct fd_progcache_admin fd_progcache_admin_t;
#include "fd_txncache.h"
#include "fd_accdb_admin.h"
#include "fd_progcache_admin.h"

Copilot uses AI. Check for mistakes.
FD_PROTOTYPES_BEGIN

#define FD_BANKS_MAGIC (0XF17EDA2C7EBA2450) /* FIREDANCER BANKS V0 */
Expand Down Expand Up @@ -197,7 +206,7 @@ FD_PROTOTYPES_BEGIN
fd_banks_mark_bank_dead( banks, dead_bank_idx );

To actually prune away any dead banks, the caller should call:
fd_banks_prune_dead_banks( banks )
fd_banks_prune_dead_banks( banks, txncache, accdb_admin, progcache_admin )

The locks and data used by an fd_bank_t or an fd_banks_t are stored as
separate objects. The locks are stored in an fd_banks_locks_t struct
Expand Down Expand Up @@ -934,7 +943,10 @@ fd_banks_mark_bank_dead( fd_banks_t * banks,
and 0 otherwise. */

int
fd_banks_prune_dead_banks( fd_banks_t * banks );
fd_banks_prune_dead_banks( fd_banks_t * banks,
fd_txncache_t * txncache,
fd_accdb_admin_t * accdb_admin,
fd_progcache_admin_t * progcache_admin );

/* fd_banks_mark_bank_frozen marks the current bank as frozen. This
should be done when the bank is no longer being updated: it should be
Expand Down
26 changes: 13 additions & 13 deletions src/flamenco/runtime/test_bank.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ test_bank_dead_eviction( void * mem ) {
bank_P->data->refcnt = 0UL; /* P(0) */
FD_TEST( fd_banks_pool_used( bank_data_pool )==1UL );

FD_TEST( !fd_banks_prune_dead_banks( banks ) );
FD_TEST( !fd_banks_prune_dead_banks( banks, NULL, NULL, NULL ) );
FD_TEST( fd_banks_pool_used( bank_data_pool )==1UL );

/* Case: isolated dead bank that gets pruned. */
Expand All @@ -301,15 +301,15 @@ test_bank_dead_eviction( void * mem ) {
FD_TEST( fd_banks_clone_from_parent( bank_D, banks, bank_D->data->idx ) );
FD_TEST( fd_banks_pool_used( bank_data_pool )==2UL );

FD_TEST( !fd_banks_prune_dead_banks( banks ) );
FD_TEST( !fd_banks_prune_dead_banks( banks, NULL, NULL, NULL ) );
FD_TEST( fd_banks_pool_used( bank_data_pool )==2UL );
fd_banks_mark_bank_frozen( banks, bank_D );

fd_banks_mark_bank_dead( banks, bank_D->data->idx );
FD_TEST( fd_banks_pool_used( bank_data_pool )==2UL );
FD_TEST( bank_D->data->flags&FD_BANK_FLAGS_DEAD );

FD_TEST( fd_banks_prune_dead_banks( banks ) );
FD_TEST( fd_banks_prune_dead_banks( banks, NULL, NULL, NULL ) );
FD_TEST( fd_banks_pool_used( bank_data_pool )==1UL );

/* Case: multiple isolated dead banks get pruned at once. */
Expand All @@ -318,7 +318,7 @@ test_bank_dead_eviction( void * mem ) {
FD_TEST( fd_banks_pool_used( bank_data_pool )==2UL );
FD_TEST( fd_banks_clone_from_parent( bank_C, banks, bank_C->data->idx ) );
FD_TEST( fd_banks_pool_used( bank_data_pool )==2UL );
FD_TEST( !fd_banks_prune_dead_banks( banks ) );
FD_TEST( !fd_banks_prune_dead_banks( banks, NULL, NULL, NULL ) );
FD_TEST( fd_banks_pool_used( bank_data_pool )==2UL );
fd_banks_mark_bank_frozen( banks, bank_C );

Expand All @@ -327,28 +327,28 @@ test_bank_dead_eviction( void * mem ) {
FD_TEST( fd_banks_pool_used( bank_data_pool )==3UL );
FD_TEST( fd_banks_clone_from_parent( bank_R, banks, bank_R->data->idx ) );
FD_TEST( fd_banks_pool_used( bank_data_pool )==3UL );
FD_TEST( !fd_banks_prune_dead_banks( banks ) );
FD_TEST( !fd_banks_prune_dead_banks( banks, NULL, NULL, NULL ) );
FD_TEST( fd_banks_pool_used( bank_data_pool )==3UL );

fd_bank_t bank_Y[1];
fd_banks_new_bank( bank_Y, banks, bank_P->data->idx, 0L );
FD_TEST( fd_banks_pool_used( bank_data_pool )==4UL );
FD_TEST( fd_banks_clone_from_parent( bank_Y, banks, bank_Y->data->idx ) );
FD_TEST( fd_banks_pool_used( bank_data_pool )==4UL );
FD_TEST( !fd_banks_prune_dead_banks( banks ) );
FD_TEST( !fd_banks_prune_dead_banks( banks, NULL, NULL, NULL ) );
FD_TEST( fd_banks_pool_used( bank_data_pool )==4UL );

fd_bank_t bank_Z[1];
fd_banks_new_bank( bank_Z, banks, bank_C->data->idx, 0L );
FD_TEST( fd_banks_pool_used( bank_data_pool )==5UL );
FD_TEST( fd_banks_clone_from_parent( bank_Z, banks, bank_Z->data->idx ) );
FD_TEST( fd_banks_pool_used( bank_data_pool )==5UL );
FD_TEST( !fd_banks_prune_dead_banks( banks ) );
FD_TEST( !fd_banks_prune_dead_banks( banks, NULL, NULL, NULL ) );
FD_TEST( fd_banks_pool_used( bank_data_pool )==5UL );

fd_banks_mark_bank_dead( banks, bank_Y->data->idx );
fd_banks_mark_bank_dead( banks, bank_Z->data->idx );
FD_TEST( fd_banks_prune_dead_banks( banks ) );
FD_TEST( fd_banks_prune_dead_banks( banks, NULL, NULL, NULL ) );
FD_TEST( fd_banks_pool_used( bank_data_pool )==3UL );

/* Case: dead banks that are siblings of non pruned banks. Make sure
Expand All @@ -370,7 +370,7 @@ test_bank_dead_eviction( void * mem ) {

fd_banks_mark_bank_dead( banks, bank_W->data->idx );
FD_TEST( bank_G->data->sibling_idx==bank_W->data->idx );
FD_TEST( fd_banks_prune_dead_banks( banks ) );
FD_TEST( fd_banks_prune_dead_banks( banks, NULL, NULL, NULL ) );
FD_TEST( fd_banks_pool_used( bank_data_pool )==4UL );
FD_TEST( bank_G->data->sibling_idx==ULONG_MAX );

Expand All @@ -386,7 +386,7 @@ test_bank_dead_eviction( void * mem ) {

fd_banks_mark_bank_dead( banks, bank_W->data->idx );
FD_TEST( bank_G->data->sibling_idx==bank_W->data->idx );
FD_TEST( fd_banks_prune_dead_banks( banks ) );
FD_TEST( fd_banks_prune_dead_banks( banks, NULL, NULL, NULL ) );
FD_TEST( fd_banks_pool_used( bank_data_pool )==5UL );
FD_TEST( bank_G->data->sibling_idx!=ULONG_MAX );
FD_TEST( bank_G->data->sibling_idx==bank_I->data->idx );
Expand All @@ -409,7 +409,7 @@ test_bank_dead_eviction( void * mem ) {
FD_TEST( fd_banks_bank_query( bank_query, banks, bank_I->data->idx ) );
FD_TEST( !fd_banks_bank_query( bank_query, banks, bank_Z->data->idx ) );

FD_TEST( !fd_banks_prune_dead_banks( banks ) );
FD_TEST( !fd_banks_prune_dead_banks( banks, NULL, NULL, NULL ) );

/* Case: don't prune dead banks if there is an outstanding reference
to them. */
Expand All @@ -422,11 +422,11 @@ test_bank_dead_eviction( void * mem ) {

bank_D->data->refcnt = 1UL;
fd_banks_mark_bank_dead( banks, bank_D->data->idx );
FD_TEST( fd_banks_prune_dead_banks( banks ) );
FD_TEST( fd_banks_prune_dead_banks( banks, NULL, NULL, NULL ) );
FD_TEST( fd_banks_pool_used( bank_data_pool )==4UL );

bank_D->data->refcnt = 0UL;
FD_TEST( fd_banks_prune_dead_banks( banks ) );
FD_TEST( fd_banks_prune_dead_banks( banks, NULL, NULL, NULL ) );
FD_TEST( fd_banks_pool_used( bank_data_pool )==3UL );

/* Case: dead bank is the left-most child of the parent. */
Expand Down
Loading