Update README for continuous aggregates #2405
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #2405 +/- ##
==========================================
+ Coverage 88.91% 90.09% +1.18%
==========================================
Files 212 213 +1
Lines 34788 34300 -488
==========================================
- Hits 30932 30904 -28
+ Misses 3856 3396 -460
Continue to review full report at Codecov.
|
pmwkaa
approved these changes
Sep 16, 2020
mkindahl
reviewed
Sep 16, 2020
There was a problem hiding this comment.
Just a few things that I think would make the text easier to follow:
- Change the order of the object list so that the references refer to previously introduced concepts.
- You talk about "below" and "above" for timestamps. I would suggest to consistently use "older" and "newer", or "before" and "after". It is not clear what "below" and "above" means for timestamps.
tsl/src/continuous_aggs/Readme.md
Outdated
Comment on lines
84
to
86
| instead. Write amplification is further reduced by never writing | ||
| invalidations that modify time ranges that are more recent the current | ||
| invalidation threshold. |
There was a problem hiding this comment.
It is not clear why this reduces write amplification.
erimatnor
force-pushed
the
caggs-update-readme
branch
from
b5381fc
to
9620db3
Compare
mkindahl
approved these changes
Sep 18, 2020
tsl/src/continuous_aggs/Readme.md
Outdated
Comment on lines
51
to
55
| 5. An invalidation threshold, which tracks how "far" the | ||
| materialization has reached and below which invalidations must be | ||
| logged. |
There was a problem hiding this comment.
Suggested change
| 5. An invalidation threshold, which tracks how "far" the | |
| materialization has reached and below which invalidations must be | |
| logged. | |
| 5. An invalidation threshold, which is a timestamp that tracks the latest | |
| materialization. Invalidations before this timestamp will be | |
| logged, but invalidations after this point will not be added to the invalidation log. |
tsl/src/continuous_aggs/Readme.md
Outdated
Comment on lines
39
to
40
| allows write amplification to be kept to a minimum by, e.g, never | ||
| refreshing the most recent time bucket that is still being "filled". |
There was a problem hiding this comment.
Are there any other things we currently do? Otherwise, just write what we do.
Suggested change
| allows write amplification to be kept to a minimum by, e.g, never | |
| refreshing the most recent time bucket that is still being "filled". | |
| allows write amplification to be kept to a minimum by not | |
| refreshing the most recent time bucket, which is assumed to still have data being added to it. |
tsl/src/continuous_aggs/Readme.md
Outdated
Comment on lines
54
to
57
| 6. A trigger on the source hypertable that invalidates regions of data | ||
| in every transaction, based on INSERT, UPDATE, and DELETE | ||
| statements. The trigger writes its invalidations to the hypertable | ||
| invalidation log. |
There was a problem hiding this comment.
It doesn't actually invalidate the regions, it just writes an invalidation record to the log. IMHO, it is better to be specific here.
Suggested change
| 6. A trigger on the source hypertable that invalidates regions of data | |
| in every transaction, based on INSERT, UPDATE, and DELETE | |
| statements. The trigger writes its invalidations to the hypertable | |
| invalidation log. | |
| 6. A trigger on the source hypertable that writes invalidations of data | |
| for every transaction, based on INSERT, UPDATE, and DELETE | |
| statements. The trigger writes its invalidations to the hypertable | |
| invalidation log. |
Comment on lines
+62
to
+68
| 8. A materialization invalidation log. Once a refresh runs on a given | ||
| continuous aggregate, this log tracks how invalidations from the | ||
| hypertable invalidation log are processed against the refresh | ||
| window for the refreshed continuous aggregate. Thus, a single | ||
| invalidation in the hypertable invalidation log becomes one entry | ||
| per continuous aggregate in the materialization invalidation log. |
There was a problem hiding this comment.
Suggested change
| 8. A materialization invalidation log. Once a refresh runs on a given | |
| continuous aggregate, this log tracks how invalidations from the | |
| hypertable invalidation log are processed against the refresh | |
| window for the refreshed continuous aggregate. Thus, a single | |
| invalidation in the hypertable invalidation log becomes one entry | |
| per continuous aggregate in the materialization invalidation log. | |
| 8. A materialization invalidation log for each continuous aggregate. Once a refresh runs on a given | |
| continuous aggregate, this log tracks how invalidations from the | |
| hypertable invalidation log are processed against the refresh | |
| window for the refreshed continuous aggregate. Thus, a single | |
| invalidation in the hypertable invalidation log becomes one entry | |
| per continuous aggregate in the materialization invalidation log. |
There was a problem hiding this comment.
There is only one log, and your suggestion could be interpreted as if there is one log per cagg.
Comment on lines
+82
to
+86
| Mutating transactions must record their mutations in the invalidation | ||
| log, so that a refresh knows to re-materialize the invalidated | ||
| range. This happens by installing a trigger on the source hypertable | ||
| when the first continuous aggregate on that hypertable is created. |
There was a problem hiding this comment.
Suggested change
| Mutating transactions must record their mutations in the invalidation | |
| log, so that a refresh knows to re-materialize the invalidated | |
| range. This happens by installing a trigger on the source hypertable | |
| when the first continuous aggregate on that hypertable is created. | |
| Mutating transactions must record their mutations in the invalidation | |
| log for the hypertable, so that a refresh knows to re-materialize the invalidated | |
| range. This happens by installing a trigger on the source hypertable | |
| when the first continuous aggregate on that hypertable is created. |
There was a problem hiding this comment.
Again, your suggestion could be interpreted as if there is one log per hypertable, which is not the case.
The README that gives an overview of the implementation of continuous aggregates is updated to reflect changes to the feature. Fixes timescale#2147
erimatnor
force-pushed
the
caggs-update-readme
branch
from
9620db3
to
a3a28a7
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The README that gives an overview of the implementation of continuous
aggregates is updated to reflect changes to the feature.
Fixes #2147