Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow drop_chunks on raw data while keeping continuous aggregate data #1589

Merged
merged 3 commits into from Dec 30, 2019

Conversation

cevian
Copy link
Contributor

@cevian cevian commented Dec 16, 2019

Detailed description in the commit messages.

@cevian cevian force-pushed the cont_agg_data_retention branch 2 times, most recently from afd275d to 81844f8 Compare December 17, 2019 02:54
@codecov
Copy link

codecov bot commented Dec 17, 2019

Codecov Report

Merging #1589 into master will increase coverage by 0.29%.
The diff coverage is 96.04%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1589      +/-   ##
==========================================
+ Coverage   91.58%   91.87%   +0.29%     
==========================================
  Files         144      142       -2     
  Lines       21684    20883     -801     
==========================================
- Hits        19859    19186     -673     
+ Misses       1825     1697     -128
Flag Coverage Δ
#cron ?
#pr 91.87% <96.04%> (-0.02%) ⬇️
Impacted Files Coverage Δ
src/chunk.h 100% <ø> (ø) ⬆️
src/catalog.h 100% <ø> (ø) ⬆️
src/cross_module_fn.c 68.54% <0%> (ø) ⬆️
src/interval.c 84.86% <100%> (+0.81%) ⬆️
tsl/src/bgw_policy/drop_chunks_api.c 98.3% <100%> (+0.02%) ⬆️
src/chunk_constraint.c 93.25% <100%> (-0.1%) ⬇️
src/bgw_policy/policy.c 100% <100%> (ø) ⬆️
src/dimension.c 96.16% <100%> (+0.43%) ⬆️
tsl/src/compression/create.c 95.85% <100%> (+0.23%) ⬆️
src/continuous_agg.c 97.54% <100%> (+0.43%) ⬆️
... and 96 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ff78290...a31997b. Read the comment docs.

@cevian cevian changed the title [WIP] Allow drop_chunks on raw data while keeping continuous aggregate data Allow drop_chunks on raw data while keeping continuous aggregate data Dec 17, 2019
@cevian cevian force-pushed the cont_agg_data_retention branch 2 times, most recently from 472721e to cdaac1a Compare December 17, 2019 03:11
Copy link
Contributor

@gayyappan gayyappan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you elaborate why we need transaction/non-transactional materialization? Why is it not always transactional?

@cevian
Copy link
Contributor Author

cevian commented Dec 18, 2019

@gayyappan When we move the completion_threshold up (materialize forward in time) we have to use multiple transaction so as to not block inserts for too long because we have to move the invalidation_threshold up. That's why we use multiple transactions to begin with. Thankfully here we only need to process invalidations, not move forward in time and so we can use transactional semantics. If you look at the code we only allow using transactional semantics in this case.

ts_time_bucket_by_type(bucket_width, invalidate_prior_to_time, type);
/* the last bucket should cover prior_to_time */
if (prior_to_time_bucketed != invalidate_prior_to_time)
prior_to_time_bucketed = int64_saturating_add(prior_to_time_bucketed, bucket_width);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we validate the invalidate_prior_to_time < completed_threshold?

@@ -620,12 +649,33 @@ materialization_invalidation_log_get_range(int32 materialization_id, Oid type, i
if (entry_range.start >= completed_threshold)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we also have if (entry_range.start >= invalidate_prior_to_time) break ?

Copy link
Contributor

@gayyappan gayyappan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reviewed some of the files. still more to go. Questions related to need for 3-value state for cascade-to_materialization.

sql/pre_install/tables.sql Show resolved Hide resolved
sql/updates/latest-dev.sql Show resolved Hide resolved
src/bgw_policy/drop_chunks.h Show resolved Hide resolved
{
/* mark as not dropped and recreate the pg objects */
ScanIterator iterator =
ts_scan_iterator_create(CHUNK, RowExclusiveLock, CurrentMemoryContext);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when is the lock on the row released? we should be holding it till the pg object creation is complete.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we add an assert that the chunk is not a compressed chunk? Or should it be applicable to compressed chunks as well?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should test this scenario: 1) drop chunk on table 2) change hypertable's chunk interval 3) create some new chunks 4) now try to recreate the old chunk and insert/update/delete/select from the old chunk that was recreated with the old settings.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lock on line 716 is not released until end of txn. (see comment above 716). This should work on compressed chunks too. I have a test for this but will add the chunk_interval change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh wait no it shouldn't be allowed on compressed chunks. I'll fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh the compressed chunk thing is checked during drop.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

referring to the lock on line 751. RowExclusiveLock on CHUNK. when does that get released?

Copy link
Contributor Author

@cevian cevian Dec 27, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lock is on line 742 LockRelationOid(ht->main_table_relid, ShareUpdateExclusiveLock); Released only at end of txn

src/chunk.c Show resolved Hide resolved
src/chunk.c Outdated Show resolved Hide resolved
@@ -2151,23 +2266,143 @@ ts_chunk_drop(Chunk *chunk, DropBehavior behavior, int32 log_level)
chunk->fd.table_name.data);

/* Remove the chunk from the hypertable table */
ts_chunk_delete_by_relid(chunk->table_id, behavior);
ts_chunk_delete_by_relid(chunk->table_id, behavior, preserve_catalog_row);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need a concurrent test where 1) 1 txn drops a chunk 2) the other txn is trying to recreate the chunk. this is to check that we are holding all locks till the first txn completes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lock on line 742 LockRelationOid(ht->main_table_relid, ShareUpdateExclusiveLock); prevents this and hasn't changed with this PR so I don't think we need more tests here.

src/chunk.c Show resolved Hide resolved
tsl/src/continuous_aggs/materialize.c Outdated Show resolved Hide resolved
tsl/src/continuous_aggs/materialize.c Outdated Show resolved Hide resolved
Copy link
Contributor

@gayyappan gayyappan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Went over the other files too.

src/process_utility.c Outdated Show resolved Hide resolved
Allow dropping raw chunks on the raw hypertable while keeping
the continuous aggregate. This allows for downsampling data
and allows users to save on TCO. We only allow dropping
such data when the dropped data is older than the
`ignore_invalidation_older_than` parameter on all the associated
continuous aggs. This ensures that any modifications to the
region of data which was dropped should never be reflected
in the continuous agg and thus avoids semantic ambiguity
if chunks are dropped but then again recreated due to an
insert.

Before we drop a chunk we need to make sure to process any
continuous aggregate invalidations that were registed on
data inside the chunk. Thus we add an option to materialization
to perform materialization transactionally, to only process
invalidations, and to process invalidation only before a timestamp.

We fix drop_chunks and policy to properly process
`cascade_to_materialization` as a tri-state variable (unknown,
true, false); Existing policy rows should change false to NULL
(unknown) and true stays as true since it was explicitly set.
Remove the form data for bgw_policy_drop_chunk because there
is no good way to represent the tri-state variable in the
form data.

When dropping chunks with cascade_to_materialization = false, all
invalidations on the chunks are processed before dropping the chunk.
If we are so far behind that even the  completion threshold is inside
the chunks being dropped, we error. There are 2 reasons that we error:
1) We can't safely process new ranges transactionally without taking
   heavy weight locks and potentially locking the entire sytem
2) If a completion threshold is that far behind the system probably has
   some serious issues anyway.
If a chunk is dropped but it has a continuous aggregate that is
not dropped we want to preserve the chunk catalog row instead of
deleting the row. This is to prevent dangling identifiers in the
materialization hypertable. It also preserves the dimension slice
and chunk constraints rows for the chunk since those will be necessary
when enabling this with multinode and is necessary to recreate the
chunk too. The postgres objects associated with the chunk are all
dropped (table, constraints, indexes).

If data is ever reinserted to the same data region, the chunk is
recreated with the same dimension definitions as before. The postgres
objects are simply recreated.
Add test for changing chunk interval when recreating
chunks. Fix tests for pg10 and pg9.6.
@cevian cevian merged commit 1a38796 into timescale:master Dec 30, 2019
@cevian cevian added this to the 1.6.0 milestone Jan 8, 2020
svenklemm added a commit to svenklemm/timescaledb that referenced this pull request Jan 15, 2020
This release adds major new features and bugfixes since the 1.5.1 release.
We deem it moderate priority for upgrading.

The major new feature in this release allows users to keep the aggregated
data in a continuous aggregate while dropping the raw data with drop_chunks.
This allows users to save storage by keeping only the aggregates.

The semantics of the refresh_lag parameter for continuous aggregates has
been changed to be relative to the current timestamp instead of the maximum
value in the table. This change requires that an integer_now func be set on
hypertables with integer-based time columns to use continuous aggregates on
this table.

We added a timescaledb.ignore_invalidation_older_than parameter for continuous
aggregatess. This parameter accept a time-interval (e.g. 1 month). if set,
it limits the amount of time for which to process invalidation. Thus, if
timescaledb.ignore_invalidation_older_than = '1 month', then any modifications
for data older than 1 month from the current timestamp at modification time may
not cause continuous aggregate to be updated. This limits the amount of work
that a backfill can trigger. By default, all invalidations are processed.

**Major Features**
* timescale#1589 Allow drop_chunks while keeping continuous aggregates

**Minor Features**
* timescale#1568 Add ignore_invalidation_older_than option to continuous aggs
* timescale#1575 Reorder group-by clause for continuous aggregates
* timescale#1592 Improve continuous agg user messages

**Bugfixes**
* timescale#1565 Fix partial select query for continuous aggregate
* timescale#1591 Fix locf treat_null_as_missing option
* timescale#1594 Fix error in compression constraint check
* timescale#1603 Add join info to compressed chunk
* timescale#1606 Fix constify params during runtime exclusion
* timescale#1607 Delete compression policy when drop hypertable
* timescale#1608 Add jobs to timescaledb_information.policy_stats
* timescale#1609 Fix bug with parent table in decompression

**Thanks**
* @optijon for reporting an issue with locf treat_null_as_missing option
* @acarrera42 for reporting an issue with constify params during runtime exclusion
* @ChristopherZellermann for reporting an issue with the compression constraint check
* @SimonDelamare for reporting an issue with joining hypertables with compression
svenklemm added a commit to svenklemm/timescaledb that referenced this pull request Jan 15, 2020
This release adds major new features and bugfixes since the 1.5.1 release.
We deem it moderate priority for upgrading.

The major new feature in this release allows users to keep the aggregated
data in a continuous aggregate while dropping the raw data with drop_chunks.
This allows users to save storage by keeping only the aggregates.

The semantics of the refresh_lag parameter for continuous aggregates has
been changed to be relative to the current timestamp instead of the maximum
value in the table. This change requires that an integer_now func be set on
hypertables with integer-based time columns to use continuous aggregates on
this table.

We added a timescaledb.ignore_invalidation_older_than parameter for continuous
aggregatess. This parameter accept a time-interval (e.g. 1 month). if set,
it limits the amount of time for which to process invalidation. Thus, if
timescaledb.ignore_invalidation_older_than = '1 month', then any modifications
for data older than 1 month from the current timestamp at modification time may
not cause continuous aggregate to be updated. This limits the amount of work
that a backfill can trigger. By default, all invalidations are processed.

**Major Features**
* timescale#1589 Allow drop_chunks while keeping continuous aggregates

**Minor Features**
* timescale#1568 Add ignore_invalidation_older_than option to continuous aggs
* timescale#1575 Reorder group-by clause for continuous aggregates
* timescale#1592 Improve continuous agg user messages

**Bugfixes**
* timescale#1565 Fix partial select query for continuous aggregate
* timescale#1591 Fix locf treat_null_as_missing option
* timescale#1594 Fix error in compression constraint check
* timescale#1603 Add join info to compressed chunk
* timescale#1606 Fix constify params during runtime exclusion
* timescale#1607 Delete compression policy when drop hypertable
* timescale#1608 Add jobs to timescaledb_information.policy_stats
* timescale#1609 Fix bug with parent table in decompression
* timescale#1624 Fix drop_chunks for ApacheOnly
* timescale#1632 Check for NULL before dereferencing variable

**Thanks**
* @optijon for reporting an issue with locf treat_null_as_missing option
* @acarrera42 for reporting an issue with constify params during runtime exclusion
* @ChristopherZellermann for reporting an issue with the compression constraint check
* @SimonDelamare for reporting an issue with joining hypertables with compression
svenklemm added a commit that referenced this pull request Jan 15, 2020
This release adds major new features and bugfixes since the 1.5.1 release.
We deem it moderate priority for upgrading.

The major new feature in this release allows users to keep the aggregated
data in a continuous aggregate while dropping the raw data with drop_chunks.
This allows users to save storage by keeping only the aggregates.

The semantics of the refresh_lag parameter for continuous aggregates has
been changed to be relative to the current timestamp instead of the maximum
value in the table. This change requires that an integer_now func be set on
hypertables with integer-based time columns to use continuous aggregates on
this table.

We added a timescaledb.ignore_invalidation_older_than parameter for continuous
aggregatess. This parameter accept a time-interval (e.g. 1 month). if set,
it limits the amount of time for which to process invalidation. Thus, if
timescaledb.ignore_invalidation_older_than = '1 month', then any modifications
for data older than 1 month from the current timestamp at modification time may
not cause continuous aggregate to be updated. This limits the amount of work
that a backfill can trigger. By default, all invalidations are processed.

**Major Features**
* #1589 Allow drop_chunks while keeping continuous aggregates

**Minor Features**
* #1568 Add ignore_invalidation_older_than option to continuous aggs
* #1575 Reorder group-by clause for continuous aggregates
* #1592 Improve continuous agg user messages

**Bugfixes**
* #1565 Fix partial select query for continuous aggregate
* #1591 Fix locf treat_null_as_missing option
* #1594 Fix error in compression constraint check
* #1603 Add join info to compressed chunk
* #1606 Fix constify params during runtime exclusion
* #1607 Delete compression policy when drop hypertable
* #1608 Add jobs to timescaledb_information.policy_stats
* #1609 Fix bug with parent table in decompression
* #1624 Fix drop_chunks for ApacheOnly
* #1632 Check for NULL before dereferencing variable

**Thanks**
* @optijon for reporting an issue with locf treat_null_as_missing option
* @acarrera42 for reporting an issue with constify params during runtime exclusion
* @ChristopherZellermann for reporting an issue with the compression constraint check
* @SimonDelamare for reporting an issue with joining hypertables with compression
svenklemm added a commit that referenced this pull request Jan 15, 2020
This release adds major new features and bugfixes since the 1.5.1 release.
We deem it moderate priority for upgrading.

The major new feature in this release allows users to keep the aggregated
data in a continuous aggregate while dropping the raw data with drop_chunks.
This allows users to save storage by keeping only the aggregates.

The semantics of the refresh_lag parameter for continuous aggregates has
been changed to be relative to the current timestamp instead of the maximum
value in the table. This change requires that an integer_now func be set on
hypertables with integer-based time columns to use continuous aggregates on
this table.

We added a timescaledb.ignore_invalidation_older_than parameter for continuous
aggregatess. This parameter accept a time-interval (e.g. 1 month). if set,
it limits the amount of time for which to process invalidation. Thus, if
timescaledb.ignore_invalidation_older_than = '1 month', then any modifications
for data older than 1 month from the current timestamp at modification time may
not cause continuous aggregate to be updated. This limits the amount of work
that a backfill can trigger. By default, all invalidations are processed.

**Major Features**
* #1589 Allow drop_chunks while keeping continuous aggregates

**Minor Features**
* #1568 Add ignore_invalidation_older_than option to continuous aggs
* #1575 Reorder group-by clause for continuous aggregates
* #1592 Improve continuous agg user messages

**Bugfixes**
* #1565 Fix partial select query for continuous aggregate
* #1591 Fix locf treat_null_as_missing option
* #1594 Fix error in compression constraint check
* #1603 Add join info to compressed chunk
* #1606 Fix constify params during runtime exclusion
* #1607 Delete compression policy when drop hypertable
* #1608 Add jobs to timescaledb_information.policy_stats
* #1609 Fix bug with parent table in decompression
* #1624 Fix drop_chunks for ApacheOnly
* #1632 Check for NULL before dereferencing variable

**Thanks**
* @optijon for reporting an issue with locf treat_null_as_missing option
* @acarrera42 for reporting an issue with constify params during runtime exclusion
* @ChristopherZellermann for reporting an issue with the compression constraint check
* @SimonDelamare for reporting an issue with joining hypertables with compression
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants