taskman up iot config #615
Conversation
andymck
force-pushed
the
andymck/taskman/iot-config
branch
from
0d7ab29
to
ccd7e63
Compare
Co-authored-by: Jeff Grunewald <[email protected]>
|
@jeffgrunewald if you can pay particular attention to the removal of the shutdown listeners embedded within some of the services.... |
There was a problem hiding this comment.
there are several instances where i was passing a clone of the shutdown listener into a short-lived stream where i was assured it wasn't necessary and those didn't prove to solve the shutdown issues we have with the config service so i'm totally fine with those going. i'm still unclear about the long-lived streams spawned in the route_service.rs module that represent the grpc route updates subscription an HPR has to the config service but i believe @maplant was pretty sure those are also safe to stop monitoring and the tokio runtime would take care of their shutdown signals so if he can confirm that i'm okay simplifying that as well.
|
@jeffgrunewald I addressed your comments but if you can give another pass over those changes. I dont have enough context on that route service to feel comfortable with what is there right now |
There was a problem hiding this comment.
unfortunately @bbalser 's testing with the new streaming ingestor rpcs proved we do need shutdown listeners inside the spawned tasks that handle indefinite streaming RPCs so we'll need to keep the ones in each of the router_service.rs methods that take a GrpcStreamRequest or return a GrpcStreamResult type in order to shut down when OS sends the signal
iot_config/src/route_service.rs
Outdated
| .and_then(|_| stream_existing_devaddrs(&pool, &signing_key, tx.clone())) | ||
| .and_then(|_| stream_existing_skfs(&pool, &signing_key, tx.clone())) | ||
| .await | ||
| .is_err() |
There was a problem hiding this comment.
why not just call this combinator chain, map_err at the end where we keep the error trace and then await??
Applies taskmanager control the the mobile config oracle
Removed some internal shutdown listeners on some of the services. Impact of these removals needs considered further and if required then will need to implement and alternative pattern