Skip to content
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

Add json-rpc api support for dropping subgraphs #5651

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mangas
Copy link
Contributor

@mangas mangas commented Sep 29, 2024

Add json-rpc method for dropping subgraphs. Using this endpoint will remove all the subgraphs for the deployment_id and will delete the underlying data. docker/bin/drop is a helper script to use the new endpoint.

Adding some sane defaults as discussed:

 ./docker/bin/drop Qmd3vU6y6pxxXPrvVWRZMN9soNB8AFQCEnqPa9jMSZZDEG
Dropping deployment Qmd3vU6y6pxxXPrvVWRZMN9soNB8AFQCEnqPa9jMSZZDEG
{"jsonrpc":"2.0","error":{"code":1,"message":"subgraph registrar error with store: store error: multiple subgraphs exist fo
r this hash, please remove them before trying to drop"},"id":"1"}
./docker/bin/remove y
{"jsonrpc":"2.0","result":{"type":"Null"},"id":"1"}
./docker/bin/drop Qmd3vU6y6pxxXPrvVWRZMN9soNB8AFQCEnqPa9jMSZZDEG
Dropping deployment Qmd3vU6y6pxxXPrvVWRZMN9soNB8AFQCEnqPa9jMSZZDEG
{"jsonrpc":"2.0","result":{"type":"Null"},"id":"1"}%

@alex-pakalniskis
Copy link

Perhaps explore Indexer Agent-based solution to avoid graphman drop danger zone?

@mangas
Copy link
Contributor Author

mangas commented Oct 7, 2024

Perhaps explore Indexer Agent-based solution to avoid graphman drop danger zone?

Not sure I understand the argument here. This is a private endpoint, it's as dangerous as letting someone add subgraphs unboundedly and fill up the disks, taking down the graph-node. It is also a basic operational task that currently cannot be automated because it's not exposed in any api.

Comment on lines +1357 to +1360
self.remove_subgraph(
SubgraphName::new(deployment.clone())
.map_err(|_| StoreError::DeploymentNotFound(deployment))?,
)?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dont this delete both the pending and current version of a subgraph that belongs to the same name as hash provided? Thought we are moving away from that behaviour?

Comment on lines +458 to +464
async fn remove_deployment(&self, id: &DeploymentHash) -> Result<(), SubgraphRegistrarError> {
self.store.drop_subgraph(id)?;

debug!(self.logger, "Removing deployment(s)"; "hash" => id.to_string());

Ok(())
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was the debug logs supposed to be before the self.store.drop_subgraph(id)?; ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants