Allow snapshot compactions during deletes #7165
Conversation
| @@ -532,10 +557,10 @@ func (c *Compactor) WriteSnapshot(cache *Cache) ([]string, error) { | |||
|
|
|||
| // See if we were closed while writing a snapshot | |||
There was a problem hiding this comment.
This comment should be updated since opened and closing were removed
There was a problem hiding this comment.
This comment still needs updating.
There was a problem hiding this comment.
There was a problem hiding this comment.
Oops... wrong line. Will fix
|
Not sure I'm reading this correctly, but it doesn't look like the snapshot compactions are ever actually disabled. There a reason to have that in there? |
If a delete takes a long time to process while writes to the shard are occuring, it was possible for the cache to fill up and writes to be rejected. This occurred because we disabled all compactions while writing tombstone file to prevent deleted data from re-appearing after a compaction completed. Instead, we only disable the level compactions and allow snapshot compactions to continue. Snapshots already handle deleted data with the cache and wal. Fixes #7161
3f5f96d to
629d117
Compare
| done chan struct{} | ||
| wg sync.WaitGroup | ||
| compactionsEnabled bool | ||
| mu sync.RWMutex |
There was a problem hiding this comment.
nit, would be good to separate out which fields are protected by mu.
| // Abort any running goroutines (this could take a while) | ||
| e.Compactor.Close() | ||
| e.snapshotCompactionsEnabled = true | ||
| e.snapshotterDone = make(chan struct{}) |
There was a problem hiding this comment.
snapshotterDone as far as I can see is never sent on or closed, so I'm not sure what its purpose it?
There was a problem hiding this comment.
@e-dard @joelegasse Added #7221 commits back in.
Required for all non-trivial PRs
If a delete takes a long time to process while writes to the
shard are occuring, it was possible for the cache to fill up
and writes to be rejected. This occurred because we disabled
all compactions while writing tombstone file to prevent deleted
data from re-appearing after a compaction completed.
Instead, we only disable the level compactions and allow snapshot
compactions to continue. Snapshots already handle deleted data
with the cache and wal.
Fixes #7161