Add changes to fix issues running the Spacewalk client in Kubernetes

[ebma]

Removes the backoff crate. Assuming that the retry() function introduces some side effects, we manually implement the retry logic now. The existing constants for RETRY_TIMEOUT and RETRY_INTERVAL are kept the same way, except now they are only used to derive the number of retries.

Retrying for Subxt RPC error due to closed connection

Sometimes, the runner encounters the following error:

[2024-05-07T13:56:14Z INFO  runner::runner] Error fetching executable: SubxtError: Rpc error: RPC error: The background task been terminated because: Networking or low-level protocol error: WebSocket connection error: connection closed; restart required. Retrying...
[2024-05-07T13:56:15Z INFO  runner::runner] Error fetching executable: SubxtError: Rpc error: RPC error: The background task been terminated because: Networking or low-level protocol error: WebSocket connection error: connection closed; restart required. Retrying...

I found this PR which adds a new experimental implementation of an RPC client that automatically reconnects. This implementation is only available in subxt v0.35 or later. I tried bumping the subxt dependencies we use in Spacewalk to that version but I encountered conflicts because our Polkadot dependencies are too outdated.
-> I created #521 as a follow-up and we'll ignore this issue for now.

Related to https://github.com/pendulum-chain/tasks/issues/207.

[ebma]

@pendulum-chain/devs this is now ready for review

[gianfra-t]

Looks good to me! Even a simpler and more transparent solution than the previous retry.

Regarding the error due to close connection, could we retry a new one after a failure here by simply creating a new client instance with Ok(OnlineClient::from_url(url).await?)? Similar to what we do in the testing service in node.

Of course this would be a less robust solution than the native subxt solution.

[ebma]

Regarding the error due to close connection, could we retry a new one after a failure here by simply creating a new client instance with Ok(OnlineClient::from_url(url).await?)?

Good point. I looked into this again and realized that this logic was kind of in place before. In this loop, the runner is restarted if try_get_release() returns an error. The problem was that the changes I made were missing the maximum retry timeout that exists in the backoff crate (max_elapsed_time). Instead of defining a maximum duration for which the logic is retried, we can only specify the number of retries, but I now derive that here so the result is similar. With this, the retry_with_log() functions will now return an error in time that is either caught inside the loop I linked to previously, or if it happens elsewhere will make the runner stop entirely. Once stopped, they can be automatically restarted in the infrastructure.

[gianfra-t]

Once stopped, they can be automatically restarted in the infrastructure.

Okay understood, I didn't consider this behavior, makes sense!

[bogdanS98]

Great changes! 👍🏼

[ebma]

Over the weekend, the runner client again encountered the error.

[2024-05-13T08:43:43Z INFO  runner::runner] Error reading chain storage for release: SubxtError: Rpc error: RPC error: The background task been terminated because: Networking or low-level protocol error: WebSocket connection error: connection closed; restart required. Retrying...

Because I slightly changed the phrasing in the log messages, I was able to pin it down to this line which means that the runner was only 'hanging' in this loop statement. Now I noticed that the call to maybe_restart_client doesn't do anything in this case because the child process (the actual vault client binary) is still running and working fine. I tested it and it's still able to process issue and redeem requests, and the RPC connection of the vault client is also working. This is interesting but my assumption is that the RPC connection of client is fine because of its periodic restart every 3 hours, thus the connection is always kind of fresh, whereas the runner never restarts or refreshes its RPC connection. That's why I added some logic to just try and create a new RPC client in the runner.

[ebma]

Something was off with the iterator returned by the exponential-backoff crate and it kept on returning values. I removed that crate completely and now we do everything manually. I also added some small tests for the retry_with_log_... functions so that we can now be sure that they don't retry more often than they should.

[b-yap]

I also experienced this, that's why I moved up to nightly-2024-04-18. 😔

Pendulum has 2 ahash dependencies:
https://github.com/pendulum-chain/pendulum/pull/463/files#diff-13ee4b2252c9e516a0547f2891aa2105c3ca71c6d7a1e682c69be97998dfc87eR150-R172

[ebma]

Do you think nightly-2024-04-18 would work in Spacewalk too? Or was there another issue? I don't remember.

[b-yap]

I have tried to test with +nightly and it was fine.

[ebma]

Should we define nightly-2024-04-18 in the rust-toolchain file in all references (README/github actions) then and remove this patch for ahash from the Cargo.toml file? Or what do you think @b-yap?

[ebma]

@b-yap any thoughts on this?

[b-yap]

mmm, I think the patch isn't needed?
I did a cargo update in my previous PR; the ahash in cargo.lock should be ok for now.
I mentioned the minimum nightly version in the readme; but I did not explain why.
https://github.com/pendulum-chain/pendulum?tab=readme-ov-file#how-to-run-tests
We could add the reason over there.

[ebma]

I removed the patch statement again and updated all references (also in the CI file) to point to the new nightly version. Let's see if the CI passes and then I merge.

[b-yap]

@ebma you can squash and merge this now. I reverted to older version, as 2024-04-18 has some problems.

Pendulum's CI is using 2024-04-18, but it's using the stable spacewalk when it was still 2024-02-09; and there were no problems so far.