Graphman new command: analyze
#3170
Conversation
tilacog
force-pushed
the
tiago/graphman-command-analyze
branch
from
25c1ef1 to
fa66713
Compare
node/src/bin/manager.rs
Outdated
| @@ -179,6 +179,13 @@ pub enum Command { | |||
| Chain(ChainCommand), | |||
| /// Manipulate internal subgraph statistics | |||
| Stats(StatsCommand), | |||
| /// Perform a SQL ANALYZE in a Entity table | |||
| Analyze { | |||
There was a problem hiding this comment.
Perhaps the command could accept a verbose argument?
I would even argue we want verbose as default.
There was a problem hiding this comment.
I agree .. it might be a little tricky to get the command output out of diesel, but if it's not too much work, would be nice to just dump whatever Postgres says onto the tty.
There was a problem hiding this comment.
I think this command should be graphman stats analyze Qm.. Entity, i.e., the analyze should be a subcommand of stats
There was a problem hiding this comment.
I couldn't find a way to capture PostgreSQL output from diesel. Not sure if this is supported at all.
node/src/manager/commands/analyze.rs
Outdated
| println!("Running ANALYZE for {entity_name} entity"); | ||
| let entity_type = EntityType::new(entity_name); | ||
| let deployment_hash = | ||
| DeploymentHash::new(hash).expect("Subgraph hash must be a valid IPFS hash"); |
There was a problem hiding this comment.
Could .context()? from anyhow provide better error info for graphman users? 🤔
| let layout = store.layout(conn, site)?; | ||
| let table = layout.table_for_entity(&entity_type)?; | ||
| let table_name = &table.qualified_name; | ||
| let sql = format!("analyze {table_name}"); |
There was a problem hiding this comment.
It would be nice if diesel provided analyze in their pg module, perhaps we can add there someday 🙂
node/src/bin/manager.rs
Outdated
| @@ -179,6 +179,13 @@ pub enum Command { | |||
| Chain(ChainCommand), | |||
| /// Manipulate internal subgraph statistics | |||
| Stats(StatsCommand), | |||
| /// Perform a SQL ANALYZE in a Entity table | |||
| Analyze { | |||
There was a problem hiding this comment.
I agree .. it might be a little tricky to get the command output out of diesel, but if it's not too much work, would be nice to just dump whatever Postgres says onto the tty.
node/src/bin/manager.rs
Outdated
| @@ -179,6 +179,13 @@ pub enum Command { | |||
| Chain(ChainCommand), | |||
| /// Manipulate internal subgraph statistics | |||
| Stats(StatsCommand), | |||
| /// Perform a SQL ANALYZE in a Entity table | |||
| Analyze { | |||
There was a problem hiding this comment.
I think this command should be graphman stats analyze Qm.. Entity, i.e., the analyze should be a subcommand of stats
|
As follow-on work (i.e., in a separate PR since I think having the command in its current form in use is more important than adding these bells and whistles) these things would be nice:
with that, And while we're at it, it would also be neat if the command could print when the table was last analyzed before running analyze. You can get that information with |
Introduces a new Graphman
analyzecommand that calls SQLANALYZEfor the given entity table.Closes #3152