fix: use batch subscribe and remove 4050 #388
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chris13524
force-pushed
the
fix/remove-extend-subscription-ttl
branch
from
54f0342 to
4abd2ba
Compare
nopestack
reviewed
Mar 5, 2024
| metrics.relay_batch_subscribe_request(start); | ||
| } | ||
| // TODO process each error individually | ||
| // TODO retry relay internal failures? |
nopestack
reviewed
Mar 5, 2024
src/publish_relay_message.rs
Outdated
| Error::Response(rpc::Error::Handler( | ||
| SubscriptionError::SubscriberLimitExceeded, | ||
| )) => { | ||
| // FIXME figure out how to handle this properly; being unable to subscribe means a broken state |
There was a problem hiding this comment.
Consider creating an issue to address this and linking it here.
There was a problem hiding this comment.
This issue was root caused and no issue should currently exist because Notify Server always subscribes first. Confirmed with logs.
Removing this error handling logic in 6e4cb5c as the error should not happen.
nopestack
approved these changes
Mar 5, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Use more lightweight renewal process now that IRN has shipped. Details here. Instead of needing to extend the TTL of the topic subscription with 4050 messages, the topic subscription is 30 days by default.
Remaining work:
Blocked by https://github.com/WalletConnect/rs-relay/issues/1457Update to SDK version aboveHow Has This Been Tested?
Not tested
Due Diligence