fix(hermes): handle price feed removal properly #1166
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Ignored Deployments
|
hermes/src/api/ws.rs
Outdated
| // close the connection. | ||
| self.sender | ||
| .send( | ||
| serde_json::to_string(&ServerResponseMessage::Err { |
There was a problem hiding this comment.
Can't we just ignore the removed feeds? In the current solution, when a price feed is removed, all websocket connections need to be opened up again and we will miss a few slots in between.
There was a problem hiding this comment.
Yeah it's a fair point and I have changed the code to follow what you says. However I think it is an unintuitive business logic and might surprise consumers. I wouldn't do it if our data ingestion wasn't relying on this.
The better approach would be what you had suggested before, a subscribeToAll method (or a *) and it is intuitive for a subscription to all to keep the connection while a feed is removed. However I didn't do it because it is a big change and we are going to move away from this API soon.
|
|
||
| for key in keys_in_cache { | ||
| if !current_keys.contains(&key) { | ||
| message_cache.remove(&key); |
There was a problem hiding this comment.
Since this is not very often, it would be great if we add some log here to see it in action once live.
There was a problem hiding this comment.
I added a info log level for it.
ali-behjati
force-pushed
the
hermes/fix-issue-on-price-removal
branch
from
056c4f2 to
988e112
Compare
ali-behjati
force-pushed
the
hermes/fix-issue-on-price-removal
branch
from
988e112 to
5c85f97
Compare
No description provided.