ensure we reconnect on failure #173
Conversation
| }; | ||
|
|
||
| let mut selected_endpoint = healthiest_endpoint(None).await; |
There was a problem hiding this comment.
this is important. it ensures at least one endpoint is connected. selecting just the first one may result on endpoint connection failure and never connects so selected_endpoint.connected().await will never resolve
There was a problem hiding this comment.
in that case we need a test. the current behaviour makes unit test non-deterministic as it may connect any of the dummy server so it is best to fix the waiting for connect behaviour anyway
| @@ -38,19 +40,23 @@ impl Endpoint { | |||
| health_config: HealthCheckConfig, | |||
| ) -> Self { | |||
| let (client_tx, client_rx) = tokio::sync::watch::channel(None); | |||
| let (reconnect_tx, mut reconnect_rx) = tokio::sync::mpsc::channel(1); | |||
There was a problem hiding this comment.
tokio::sync::Notify may be a better option
There was a problem hiding this comment.
notify is one off thing but we may need to reconnect multiple times
| @@ -422,6 +426,10 @@ impl Client { | |||
| _ = selected_endpoint.health().unhealthy() => { | |||
| // Current selected endpoint is unhealthy, try to rotate to another one. | |||
| // In case of all endpoints are unhealthy, we don't want to keep rotating but stick with the healthiest one. | |||
|
|
|||
| // The ws client maybe in a state that requires a reconnect | |||
| selected_endpoint.reconnect().await; | |||
There was a problem hiding this comment.
when will execute the moment endpoint becomes unhealthy and when that happens it will try to reconnect. I don't think this extra reconnect will help
There was a problem hiding this comment.
there is not reconnect currently. we have to drop and re-create the client to actually reconnect. currently it will always fail if the remote drops connection and can never be able to connect to it anymore
|
|
||
| let h1 = tokio::spawn(async move { | ||
| let _req = rx1.recv().await.unwrap(); | ||
| // no response, let it timeout |
There was a problem hiding this comment.
a request timeout will make endpoint unhealthy therefor it will try to reconnect itself
* ensure we reconnect on failure * refactor * fix test
* Revert "Refactor endpoint (#178)" This reverts commit 7fa3132. * Revert "ensure we reconnect on failure (#173)" This reverts commit 5039cfa. * Revert "improve reconnect wait time (#168)" This reverts commit 7cb7c73. * Revert "Await healthy endpoint (#158)" This reverts commit ef1c524. * Revert "endpoint health (#152)" This reverts commit cdbdd9b. * redo validate middleware * fix
No description provided.