-
Notifications
You must be signed in to change notification settings - Fork 70
feat: add reload_plugin and unload_plugin admin RPCs #369
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
base: main
Are you sure you want to change the base?
Conversation
|
That's awesome, thanks @0xsouravm! |
|
No worries @lgalabru !! |
|
@0xsouravm what's the issue you're seeing? if you're using Yellowstone, I think there's an issue with the proposed config file from the example in the repo - something with the plugin name that should not be there or something. |
|
@lgalabru I was trying with the example config as @MicaiahReid suggested but the config JSON doesn't sit correctly. Can you tell me how |
bf11a07 to
e930744
Compare
…ouravm/surfpool into feat/reload-unload-admin-rpc
e930744 to
439db23
Compare
|
Apologies on the latency for this PR, @0xsouravm 🙏 I spent some time testing this today. Here are the steps to test:
I found that this does not work out of the box, but actually crashes surfpool. |
crates/core/src/rpc/admin.rs
Outdated
| }; | ||
| Box::pin(async move { Ok(endpoint_url) }) | ||
| // Return a JSON string containing both UUID and endpoint URL | ||
| let response = format!(r#"{{"uuid": "{}", "endpoint": "{}"}}"#, uuid, endpoint_url); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added logging through simnet_events_tx as you mentioned!
…ouravm/surfpool into feat/reload-unload-admin-rpc
|
Hey @MicaiahReid ! No worries and thank you for guiding me how to test it. I ran into the same issues as you and now I fixed those and The issue - We weren't destroying the associated collection after unloading the plugin
Please check out the above screenshot for the logs! |
crates/core/src/runloops/mod.rs
Outdated
| let _ = subgraph_commands_tx.send(SubgraphCommand::CreateCollection( | ||
| uuid, | ||
| config.data.clone(), | ||
| crossbeam_channel::bounded(0).0, // Temporary sender, will be replaced |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will need to be fixed before merge, you should be using the notifier passed into PluginManagerCommand::LoadConfig

Changes:
unload_plugin(meta, name)andreload_plugin(meta, name, config_file)RPC methodsPluginManagerCommandenum withUnloadPluginandReloadPluginvariantsload_subgraph_plugin()andunload_plugin_by_uuid()to eliminate code duplicationLoadConfigto use helpers and addedUnloadPluginandReloadPluginhandlerplugin_uuid_map: HashMap<Uuid, usize>to track plugin indices for efficient unload/reloadDestoryCollectioncommand toSubgraphCommandto destroy associated collection after unloading a pluginunregister_collectionandremove_collectionmethods to use inDestroyCollectioncommandReference: #330