Skip to content

repair: fix forest to handle forks from non-frontier slots #4873

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
45 changes: 34 additions & 11 deletions src/discof/forest/fd_forest.c
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,33 @@ advance_frontier( fd_forest_t * forest, ulong slot, ushort parent_off ) {
ulong parent_slot = slot - parent_off;
ele = fd_ptr_if( !ele, fd_forest_frontier_ele_query( fd_forest_frontier( forest ), &parent_slot, NULL, pool ), ele );

/* There's an edge case where a fork begins from from somewhere behind
Copy link
Member

Choose a reason for hiding this comment

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

hm... should this logic live in the insert helper function in forest instead?

the frontier */
if( FD_UNLIKELY( !ele ) ) {
// traverse up the tree to see if we can find a parent
ele = fd_forest_ancestry_ele_query( ancestry, &slot, NULL, pool );
ulong parent_idx = ele->parent;
Copy link
Member

Choose a reason for hiding this comment

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

ancestor

int found_parent_on_frontier = 0;
while( parent_idx != null ) {
fd_forest_ele_t * parent = fd_forest_pool_ele( pool, parent_idx );
if( fd_forest_frontier_ele_query( fd_forest_frontier( forest ), &parent->slot, NULL, pool ) ){
found_parent_on_frontier = 1;
break;
}
parent_idx = parent->parent;
}

if( FD_UNLIKELY( !found_parent_on_frontier ) ) {
ele = fd_forest_ancestry_ele_remove( ancestry, &slot, NULL, pool );
fd_forest_frontier_ele_insert( fd_forest_frontier( forest ), ele, pool );
} else {
/* We hit this case when we are not on frontier, parent is not on
frontier, but grandparent is on frontier. Which means this data
shred will not be advancing the frontier. */
Comment on lines +307 to +309
Copy link
Member

Choose a reason for hiding this comment

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

ancestor

return;
}
}

fd_forest_ele_t * head = ele;
fd_forest_ele_t * tail = head;
fd_forest_ele_t * prev = NULL;
Expand All @@ -310,10 +337,10 @@ advance_frontier( fd_forest_t * forest, ulong slot, ushort parent_off ) {

static fd_forest_ele_t *
query( fd_forest_t * forest, ulong slot ) {
fd_forest_ele_t * pool = fd_forest_pool( forest );
fd_forest_ancestry_t * ancestry = fd_forest_ancestry( forest );
fd_forest_frontier_t * frontier = fd_forest_frontier( forest );
fd_forest_orphaned_t * orphaned = fd_forest_orphaned( forest );
fd_forest_ele_t * pool = fd_forest_pool( forest );
fd_forest_ancestry_t * ancestry = fd_forest_ancestry( forest );
fd_forest_frontier_t * frontier = fd_forest_frontier( forest );
fd_forest_orphaned_t * orphaned = fd_forest_orphaned( forest );

fd_forest_ele_t * ele;
ele = fd_forest_ancestry_ele_query( ancestry, &slot, NULL, pool );
Expand Down Expand Up @@ -424,7 +451,7 @@ fd_forest_publish( fd_forest_t * forest, ulong new_root_slot ) {
head points to the queue head (initialized with old_root_ele). */

fd_forest_ele_t * head = ancestry_frontier_remove( forest, old_root_ele->slot );
head->next = null;
head->next = null;
fd_forest_ele_t * tail = head;

/* Second, BFS down the tree, inserting each ele into the prune queue
Expand Down Expand Up @@ -607,11 +634,6 @@ fd_forest_ancestry_print( fd_forest_t const * forest ) {
FD_LOG_NOTICE( ( "\n\n[Ancestry]\n%lu", fd_forest_pool_ele_const( fd_forest_pool_const( forest ), forest->root )->slot ) );

ancestry_print2( forest, fd_forest_pool_ele_const( fd_forest_pool_const( forest ), forest->root ), NULL, 0, 0, "" );

//FD_LOG_NOTICE(("\n\n[Ancestry]\n%lu", fd_forest_pool_ele_const( fd_forest_pool_const( forest ), forest->root )->slot ) );

//ancestry_print( forest, fd_forest_pool_ele_const( fd_forest_pool_const( forest ), forest->root ), 0, "" );

}

void
Expand All @@ -623,7 +645,8 @@ fd_forest_frontier_print( fd_forest_t const * forest ) {
!fd_forest_frontier_iter_done( iter, frontier, pool );
iter = fd_forest_frontier_iter_next( iter, frontier, pool ) ) {
fd_forest_ele_t const * ele = fd_forest_frontier_iter_ele_const( iter, frontier, pool );
ancestry_print2( forest, fd_forest_pool_ele_const( fd_forest_pool_const( forest ), fd_forest_pool_idx( pool, ele ) ), NULL, 0, 0, "" );
printf( "%lu (%u/%u)\n", ele->slot, ele->buffered_idx + 1, ele->complete_idx + 1 );
//ancestry_print2( forest, fd_forest_pool_ele_const( fd_forest_pool_const( forest ), fd_forest_pool_idx( pool, ele ) ), NULL, 0, 0, "" );

}
}
Expand Down
55 changes: 53 additions & 2 deletions src/discof/forest/test_forest.c
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,55 @@ void test_out_of_order( fd_wksp_t * wksp ) {
fd_wksp_free_laddr( fd_forest_delete( fd_forest_leave( fd_forest_fini( forest ) ) ) );
}

void
test_repair_iter_frontier( fd_wksp_t * wksp ){
/* map_chain says we can't iterate while insertion is happening .... I say we can! */

ulong ele_max = 32UL;
void * mem = fd_wksp_alloc_laddr( wksp, fd_forest_align(), fd_forest_footprint( ele_max ), 1UL );
FD_TEST( mem );
fd_forest_t * forest = fd_forest_join( fd_forest_new( mem, ele_max, 42UL /* seed */ ) );

fd_forest_init( forest, 0 );
fd_forest_data_shred_insert( forest, 1, 1, 10, 0, 1 );
fd_forest_data_shred_insert( forest, 2, 1, 10, 0, 1 );
fd_forest_data_shred_insert( forest, 3, 1, 10, 0, 1 );
fd_forest_data_shred_insert( forest, 4, 1, 10, 0, 1 );
fd_forest_data_shred_insert( forest, 5, 1, 10, 0, 1 );

for( ulong i = 6; i < 31; i++ ) {
FD_TEST( fd_forest_data_shred_insert( forest, i, (ushort)i, 10, 0, 1 ) );
}

fd_forest_print( forest );

/* Frontier should be slot 1. */
ulong key = 1 ;
FD_TEST( fd_forest_frontier_ele_query( fd_forest_frontier( forest ), &key, NULL, fd_forest_pool( forest ) ) );

fd_forest_ele_t * pool = fd_forest_pool( forest );
fd_forest_frontier_t * frontier = fd_forest_frontier( forest );

for( fd_forest_frontier_iter_t iter = fd_forest_frontier_iter_init( frontier, pool );
!fd_forest_frontier_iter_done( iter, frontier, pool );
iter = fd_forest_frontier_iter_next( iter, frontier, pool ) ) {
fd_forest_ele_t * ele = fd_forest_frontier_iter_ele( iter, frontier, pool );
FD_TEST( ele );
}

ulong key2 = 6;
FD_TEST( fd_forest_frontier_ele_query( fd_forest_frontier( forest ), &key2, NULL, fd_forest_pool( forest ) ) );

int cnt = 0;
for( fd_forest_frontier_iter_t iter = fd_forest_frontier_iter_init( frontier, pool );
!fd_forest_frontier_iter_done( iter, frontier, pool );
iter = fd_forest_frontier_iter_next( iter, frontier, pool ) ) {
cnt++;
}
FD_LOG_WARNING(( "frontier sz: %d", cnt ));

}

void
test_print_tree( fd_wksp_t *wksp ){
ulong ele_max = 512UL;
Expand Down Expand Up @@ -234,8 +283,10 @@ main( int argc, char ** argv ) {
fd_wksp_t * wksp = fd_wksp_new_anonymous( fd_cstr_to_shmem_page_sz( page_sz ), page_cnt, fd_shmem_cpu_idx( numa_idx ), "wksp", 0UL );
FD_TEST( wksp );

test_publish( wksp );
test_out_of_order( wksp );
//test_publish( wksp );
//test_out_of_order( wksp );

test_repair_iter_frontier( wksp );
// test_print_tree( wksp );

fd_halt();
Expand Down
Loading