[coordinator] Delete related etcd keys when aggregator placement deleted #2133
Conversation
| flushTimesMgrOpts := aggregator.NewFlushTimesManagerOptions() | ||
| electionMgrOpts := aggregator.NewElectionManagerOptions() |
There was a problem hiding this comment.
meganit: do this after getting the store?
There was a problem hiding this comment.
Also, are these guaranteed to be default?
There was a problem hiding this comment.
Sure thing.
| return fmt.Errorf( | ||
| "error check flush times key exists for deleted instance: %v", | ||
| flushTimesKeyErr) |
There was a problem hiding this comment.
Think this would work better as a multierror, so you can still delete the rest of the flush time keys if one errors? Also lets us try to delete election keys too
There was a problem hiding this comment.
Good point, will do.
| return fmt.Errorf( | ||
| "error checking election key exists for deleted instance: %v", err) |
There was a problem hiding this comment.
Think this would work better as a multierror, so you can still delete the rest of the election keys if one errors?
There was a problem hiding this comment.
Agreed, will do.
| continue | ||
| } | ||
| if elem.ShardSetID() == shardSetID { | ||
| existingShardSetIDs++ |
There was a problem hiding this comment.
Agreed, will do.
| if found { | ||
| continue | ||
| } | ||
| shardSetIDs = append(shardSetIDs, value) |
There was a problem hiding this comment.
nit; probably a bit cleaner as
if !found {
shardSetIDs = append(shardSetIDs, value)
}
There was a problem hiding this comment.
Sure thing.
robskillington
deleted the
r/delete-aggregator-keys-on-delete-placement
branch
What this PR does / why we need it:
Keys related to the aggregator were not being cleaned up after placement deletion and as such there's no way to really cleanly reset an environment after aggregator has been setup (i.e. coordinator placement deleted, aggregator placement deleted and aggregator topic deleted).
Here were the remaining keys that should be removed if they still exist after placement deletion:
Special notes for your reviewer:
Does this PR introduce a user-facing and/or backwards incompatible change?:
Does this PR require updating code package or user-facing documentation?: