Start node announcement task after first peer #288
Conversation
srikanth-iyengar
force-pushed
the
feature/204
branch
from
f2be3ad
to
836e986
Compare
|
@tnull should we include the new internal state in NodeStatus so that we can add tests or it is not required |
srikanth-iyengar
force-pushed
the
feature/204
branch
2 times, most recently
from
4d1b1e4
to
3c1013d
Compare
srikanth-iyengar
force-pushed
the
feature/204
branch
from
3c1013d
to
0573890
Compare
| @@ -38,6 +41,12 @@ where | |||
| return Ok(()); | |||
| } | |||
|
|
|||
| let is_initial_connection_established = | |||
| self.is_initial_connection_established.load(Ordering::Acquire); | |||
| if !is_initial_connection_established { | |||
There was a problem hiding this comment.
I think we can just store here, although only if we successfully connected the peer below. If you really want to check before storing, you should use compare_exchange.
Note however that we'll also have to reset the value in stop after disconnecting all peers, and probably for good measure also at the beginning of start, as otherwise would still broadcast without being connected after restart.
| @@ -144,4 +153,8 @@ where | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| pub fn get_is_initial_connection_established(&self) -> bool { | |||
There was a problem hiding this comment.
nit: Avoid the get_ prefix as per Rust API Guidelines.
| @@ -552,6 +553,11 @@ impl Node { | |||
| // We check every 30 secs whether our last broadcast is NODE_ANN_BCAST_INTERVAL away. | |||
| let mut interval = tokio::time::interval(Duration::from_secs(30)); | |||
| loop { | |||
| let is_initial_connection_established = connect_cm.get_is_initial_connection_established(); | |||
| if !is_initial_connection_established { | |||
| continue; | |||
There was a problem hiding this comment.
This would busy-wait until we actually connect which isn't ideal. Rather, we want to block the the task until the first connection came in. Likely, a tokio::sync::watch channel is preferable to an AtomicBool here, as we could just await until we get a changed() and only then enter this loop.
Fixes #204
Introduced a new atomic internal state to track whether the announcement task is started or not
When we connect to some peer, and the announcement broadcast is not started yet, then we update the state and spawn the
announcement_broadcast_task