Skip to content

restore: fix https snapshot peers not selectable #11379

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
ripatel-fd wants to merge 1 commit into
base: main
Choose a base branch
from
Open

restore: fix https snapshot peers not selectable #11379

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
2 changes: 2 additions & 0 deletions src/discof/restore/Local.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,15 @@ $(call make-unit-test,test_ssmanifest_parser,utils/test_ssmanifest_parser,fd_dis
$(call make-unit-test,test_slot_delta_parser,utils/test_slot_delta_parser,fd_discof fd_flamenco fd_ballet fd_util)
$(call make-unit-test,test_sspeer_selector,utils/test_sspeer_selector,fd_discof fd_flamenco fd_ballet fd_util)
$(call make-unit-test,test_ssping,utils/test_ssping,fd_discof fd_flamenco fd_ballet fd_util)
$(call make-unit-test,test_ssresolve,utils/test_ssresolve,fd_discof fd_waltz fd_flamenco fd_ballet fd_util,$(OPENSSL_LIBS))
$(call make-unit-test,test_ssload,utils/test_ssload,fd_discof fd_disco fd_flamenco fd_ballet fd_tango fd_util)
$(call make-unit-test,test_ssarchive,utils/test_ssarchive,fd_discof fdctl_platform fd_ballet fd_util)
$(call make-unit-test,test_ssparse,utils/test_ssparse,fd_discof fd_flamenco fd_ballet fd_util)
$(call run-unit-test,test_slot_delta_parser)
$(call run-unit-test,test_sspeer_selector)
$(call run-unit-test,test_ssarchive)
$(call run-unit-test,test_ssparse)
$(call run-unit-test,test_ssresolve)
$(call run-unit-test,test_ssload)

$(call make-fuzz-test,fuzz_snapshot_parser,utils/fuzz_snapshot_parser,fd_discof fd_flamenco fd_ballet fd_util)
Expand Down
27 changes: 20 additions & 7 deletions src/discof/restore/utils/fd_http_resolver.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,12 @@ poll_resolve( fd_http_resolver_t * resolver,
int res = fd_ssresolve_advance_poll_out( ssresolve );

if( FD_UNLIKELY( res==FD_SSRESOLVE_ADVANCE_ERROR ) ) {
/* Errors past this point are TLS shutdown failures. Keep the
result. */
if( FD_UNLIKELY( fd_ssresolve_is_resolved( ssresolve ) ) ) {
fd_ssresolve_finish( ssresolve );
return 0;
}
unresolve_peer( resolver, peer_pool_ele( resolver->pool, resolver->fds_idx[ idx ] ), now );
return -1;
}
Expand All @@ -415,10 +421,14 @@ poll_resolve( fd_http_resolver_t * resolver,
int res = fd_ssresolve_advance_poll_in( ssresolve, &resolve_result );

if( FD_UNLIKELY( res==FD_SSRESOLVE_ADVANCE_ERROR ) ) {
if( FD_UNLIKELY( fd_ssresolve_is_resolved( ssresolve ) ) ) {
fd_ssresolve_finish( ssresolve );
return 0;
}
unresolve_peer( resolver, peer_pool_ele( resolver->pool, resolver->fds_idx[ idx ] ), now );
return -1;
} else if( FD_UNLIKELY( res==FD_SSRESOLVE_ADVANCE_AGAIN ) ) {
return -1;
return 0; /* let the caller handle POLLERR/POLLHUP below */
} else if( FD_LIKELY( res==FD_SSRESOLVE_ADVANCE_RESULT ) ) {
FD_TEST( peer->deadline_nanos>now );

Expand Down Expand Up @@ -463,12 +473,15 @@ poll_advance( fd_http_resolver_t * resolver,
if( FD_UNLIKELY( res ) ) continue;
}

/* Only react to POLLERR/POLLHUP if the ssresolve hasn't completed
yet. After a redirect is parsed the server often closes the
connection, which is harmless. */
if( FD_UNLIKELY( (pfd->revents & (POLLERR|POLLHUP)) && !fd_ssresolve_is_done( ssresolve ) ) ) {
unresolve_peer( resolver, peer_pool_ele( resolver->pool, resolver->fds_idx[ i ] ), now );
continue;
/* Servers usually close right after the redirect. If the result
was already parsed that is harmless, so give up on the pending
TLS shutdown instead of discarding the peer. */
if( FD_UNLIKELY( pfd->revents & (POLLERR|POLLHUP) ) ) {
if( FD_UNLIKELY( !fd_ssresolve_is_resolved( ssresolve ) ) ) {
unresolve_peer( resolver, peer_pool_ele( resolver->pool, resolver->fds_idx[ i ] ), now );
continue;
}
if( FD_UNLIKELY( !fd_ssresolve_is_done( ssresolve ) ) ) fd_ssresolve_finish( ssresolve );
}

/* Once both the full and incremental snapshots are resolved, we can
Expand Down
16 changes: 16 additions & 0 deletions src/discof/restore/utils/fd_ssresolve.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

struct fd_ssresolve_private {
int state;
int resolved; /* whether a valid resolve result has been parsed */
long deadline;

fd_ip4_port_t addr;
Expand Down Expand Up @@ -76,6 +77,7 @@ fd_ssresolve_new( void * shmem ) {
fd_ssresolve_t * ssresolve = FD_SCRATCH_ALLOC_APPEND( l, FD_SSRESOLVE_ALIGN, sizeof(fd_ssresolve_t) );

ssresolve->state = FD_SSRESOLVE_STATE_REQ;
ssresolve->resolved = 0;
ssresolve->request_sent = 0UL;
ssresolve->request_len = 0UL;
ssresolve->response_len = 0UL;
Expand Down Expand Up @@ -125,6 +127,7 @@ fd_ssresolve_init( fd_ssresolve_t * ssresolve,
ssresolve->full = full;

ssresolve->state = FD_SSRESOLVE_STATE_REQ;
ssresolve->resolved = 0;
ssresolve->request_sent = 0UL;
ssresolve->request_len = 0UL;
ssresolve->response_len = 0UL;
Expand All @@ -145,6 +148,7 @@ fd_ssresolve_init_https( fd_ssresolve_t * ssresolve,
ssresolve->full = full;

ssresolve->state = FD_SSRESOLVE_CONNECT;
ssresolve->resolved = 0;
ssresolve->request_sent = 0UL;
ssresolve->request_len = 0UL;
ssresolve->response_len = 0UL;
Expand Down Expand Up @@ -299,6 +303,7 @@ fd_ssresolve_parse_redirect( fd_ssresolve_t * ssresolve,
result->base_slot = full_entry_slot;
}

ssresolve->resolved = 1;
if( FD_UNLIKELY( ssresolve->is_https ) ) ssresolve->state = FD_SSRESOLVE_STATE_SHUTTING_DOWN;
else ssresolve->state = FD_SSRESOLVE_STATE_DONE;
return FD_SSRESOLVE_ADVANCE_RESULT;
Expand Down Expand Up @@ -496,6 +501,17 @@ fd_ssresolve_is_done( fd_ssresolve_t * ssresolve ) {
return ssresolve->state==FD_SSRESOLVE_STATE_DONE;
}

int
fd_ssresolve_is_resolved( fd_ssresolve_t * ssresolve ) {
return ssresolve->resolved;
}

void
fd_ssresolve_finish( fd_ssresolve_t * ssresolve ) {
FD_TEST( ssresolve->resolved );
ssresolve->state = FD_SSRESOLVE_STATE_DONE;
}

void
fd_ssresolve_cancel( fd_ssresolve_t * ssresolve ) {
if( FD_LIKELY( ssresolve->sockfd!=-1 ) ) {
Expand Down
12 changes: 12 additions & 0 deletions src/discof/restore/utils/fd_ssresolve.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,18 @@ fd_ssresolve_advance_poll_in( fd_ssresolve_t * ssresolve,
int
fd_ssresolve_is_done( fd_ssresolve_t * ssresolve );

/* fd_ssresolve_is_resolved returns whether the snapshot slot and hash
have been parsed. An https ssresolve is not done at that point: it
still has a TLS close_notify to exchange. */
int
fd_ssresolve_is_resolved( fd_ssresolve_t * ssresolve );

/* fd_ssresolve_finish gives up on that close_notify exchange and marks
the state machine done, for when the server hangs up first. Only
valid once fd_ssresolve_is_resolved. */
void
fd_ssresolve_finish( fd_ssresolve_t * ssresolve );

void
fd_ssresolve_cancel( fd_ssresolve_t * ssresolve );

Expand Down
Loading
Loading