Conversation
|
This is not exactly a low risk change, so good 👀 required! |
ApprovabilityVerdict: Needs human review This PR introduces a new database pruning capability that drops empty table partitions, including new SQL migrations and dynamic DROP TABLE operations. While the author owns all changed files and the code includes safeguards, the feature introduces significant runtime behavior changes to database management that warrant human review. You can customize Macroscope's approvability policy. Learn more. |
|
|
||
| for tableName, ceiling := range deletableTables { | ||
| result, err := e.writerDB.Exec( | ||
| constructVariableMetaTableQuery(tableName, e.config.BatchSize, ceiling), |
There was a problem hiding this comment.
Perhaps this gets renamed to say what it actually does - this is a constructVariableMetaTableDeleteQuery, right?
There was a problem hiding this comment.
this is all existing code, just moved
| zap.Error(err), | ||
| zap.String("table", tableName), | ||
| ) | ||
| delete(deletableTables, tableName) |
There was a problem hiding this comment.
Why might also leave it so we retry it again later?
There was a problem hiding this comment.
it might get retried on the next full run, 15/60min later depending on config
| for { | ||
| if cyclesCompleted >= e.config.MaxCycles { | ||
| e.logger.Warn( | ||
| "reached maximum pruning cycles", | ||
| zap.Int("max_cycles", e.config.MaxCycles), | ||
| ) | ||
| break | ||
| } |
There was a problem hiding this comment.
Why not just have this in the for itself?
| @@ -0,0 +1,232 @@ | |||
| package prune_test | |||
There was a problem hiding this comment.
Tests that confirm that originators 0 and 1 are ignored might be good.
Also, tests that drop multiple empty partitions might be cool. Though I don't think it's very likely that pruner will in one swoop prune so many rows that there's multiple empty partitions.
Add partition dropping to pruner to remove empty non-ceiling meta/blob partition pairs
get_prunable_meta_partitions()(migration00022) that identifies empty, non-ceiling leaf partitions ofgateway_envelopes_metaper originator.Executor.DropPrunablePartitionsinpartition_prune.gothat queries prunable partitions and drops paired meta and blob tables viaDROP TABLE IF EXISTS ... CASCADE.Executor.Runinprune.goto delegate toPruneRows()(extracted from previous inline logic) and the newDropPrunablePartitions().DryRunflag (no tables dropped when true).Runnow drops empty lower partitions on each execution in addition to pruning rows.Changes since #1819 opened
Macroscope summarized c86df5b.