-
Notifications
You must be signed in to change notification settings - Fork 73
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove network observer module #115
Merged
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
…-xyz/clockwork into nick/pool-position-bug
nickgarfield
changed the title
Resolve issue where workers are not observing pool rotations
[🚧 WIP] Resolve issue where workers are not observing pool rotations
Dec 16, 2022
nickgarfield
changed the title
[🚧 WIP] Resolve issue where workers are not observing pool rotations
[🚧 WIP] Investigate issue where workers are not observing pool rotations
Dec 16, 2022
nickgarfield
changed the title
[🚧 WIP] Investigate issue where workers are not observing pool rotations
Remove network observer module
Dec 17, 2022
I have this deployed on the mainnet nodes, and all looks good. Will continue monitoring for a few more hours and if things continue to look good this evening, we can land. |
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.
This PR removes the network observer module from the plugin. The network observer was designed to listen for changes to accounts belonging to the network program. It would then cache these accounts in memory and use them to in the transaction execution flows. We began noticing issues a few days ago (see ticket below for details) where this cached data was stale or incorrect. This would cause a worker would think it's in the pool even when it is not. Or visa versa, a worker would think it's not in the pool when it is.
Looking at transaction histories, it's still clear why the cached account data was incorrect. The only theory that makes sense to me is that workers can sometimes be on the bad fork and when jumping to the correct fork, account updates are not pushed to geyser. This theory is not confirmed, but in the meantime this PR serves as a fix for the issue. Instead of relying on the cached account data, this PR updates the plugin to simply use the client to fetch the fresh account data for registry, pool, and snapshot accounts.
https://linear.app/clockwork-xyz/issue/CLO-122/workers-not-recognizing-pool-position-updates