Prevent drop_chunks from dropping unrefreshed data
#9006
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9006 +/- ##
==========================================
+ Coverage 82.42% 82.51% +0.08%
==========================================
Files 242 242
Lines 47602 47597 -5
Branches 12155 12159 +4
==========================================
+ Hits 39238 39274 +36
- Misses 3498 3512 +14
+ Partials 4866 4811 -55 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
should probably add tests for DROP TABLE too |
kpan2034
force-pushed
the
fix-data-retention-policy
branch
3 times, most recently
from
222164c to
13f9212
Compare
Added a single test for this to confirm behavior is as expected. We don't check the watermark if chunks are dropped using DROP TABLE. |
This PR changes `drop_chunks` behaviour to skip dropping chunks that contain data required for continuous aggregate refreshes by checking the CAgg watermark. A notice is emitted for each skipped chunk. This is mainly to prevent data retention policies (which use drop_chunks under-the-hood) from dropping chunks which haven't been refreshed yet by a corresponding refresh policy. This also introduces a 'force' option to `drop_chunks` to allow users to override the watermark-protection and drop chunks containing unrefreshed data. Using 'force' to drop a chunk containing unrefreshed CAgg data generates a warning for the user. Note that invalidation logs are not checked. It is still possible to drop chunks containing unrefreshed data that was backfilled.
kpan2034
force-pushed
the
fix-data-retention-policy
branch
from
13f9212 to
f75cd85
Compare
kpan2034
changed the title
drop_chunks from dropping CAgg-dependent chunksdrop_chunks from dropping unrefreshed data
|
The code changes look good. My only comment is that we should be more clear in the change of behavior with direct compress. Existing users that upgrade may not be aware that the default now uses direct compress. We are switching on a feature still in tech preview and setting it by default. Going forward we should add a CHANGELOG or highlight it better in the README. |
| // notice. If force is true, print a WARNING and drop it anyway | ||
| if (!force) | ||
| { | ||
| ereport(NOTICE, |
There was a problem hiding this comment.
Shouldn't this be a WARNING message?
There was a problem hiding this comment.
Also would be nice to set a proper ERRCODE
| .fd.column_type)))); | ||
| continue; | ||
| } | ||
| ereport(WARNING, |
| @@ -0,0 +1,3092 @@ | |||
| -- This file and its contents are licensed under the Timescale License. | |||
4 participants
This PR changes
drop_chunksbehaviour to skip dropping chunks that contain unrefreshed data that is required for continuous aggregate refreshes. This is done by checking the CAgg watermark. A notice is emitted for each skipped chunk.This is mainly to prevent data retention policies (which use drop_chunks under-the-hood) from dropping chunks which haven't been refreshed yet by a corresponding refresh policy.
This also introduces a 'force' option to
drop_chunksto allow users to override the watermark-protection and drop chunks containing unrefreshed data. Using 'force' to drop a chunk containing unrefreshed CAgg data generates a warning for the user.Note that invalidation logs are not checked. It is still possible to drop chunks containing unrefreshed data that was backfilled.