Add drop chunk hook for OSM #6067
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 @@
## main #6067 +/- ##
==========================================
- Coverage 81.52% 81.50% -0.03%
==========================================
Files 246 246
Lines 56714 56727 +13
Branches 12572 12568 -4
==========================================
- Hits 46236 46233 -3
+ Misses 8103 8083 -20
- Partials 2375 2411 +36
... and 41 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
gayyappan
force-pushed
the
bugfix_2
branch
2 times, most recently
from
df06528
to
f882c94
Compare
gayyappan
changed the title
svenklemm
reviewed
Sep 15, 2023
src/chunk.c
Outdated
| @@ -3890,6 +3889,7 @@ ts_chunk_do_drop_chunks(Hypertable *ht, int64 older_than, int64 newer_than, int3 | |||
|
|
|||
| { | |||
| uint64 num_chunks = 0; | |||
| int32 osm_chunk_id = 0; | |||
There was a problem hiding this comment.
Suggested change
| int32 osm_chunk_id = 0; |
svenklemm
reviewed
Sep 15, 2023
gayyappan
force-pushed
the
bugfix_2
branch
4 times, most recently
from
0db4a5b
to
8b2bf1f
Compare
fabriziomello
approved these changes
Sep 18, 2023
svenklemm
approved these changes
Sep 18, 2023
Merged
svenklemm
reviewed
Sep 19, 2023
tsl/test/src/test_chunk_stats.c
Outdated
| osm_ht_drop_chunks_hook_mock(Oid osm_chunk_oid, const char *schema_name, const char *table_name, | ||
| int64 range_start, int64 range_end) | ||
| { | ||
| elog(NOTICE, "hypertable_drop_chunks_hook (%ld %ld)", range_start, range_end); |
There was a problem hiding this comment.
Suggested change
| elog(NOTICE, "hypertable_drop_chunks_hook (%ld %ld)", range_start, range_end); | |
| elog(NOTICE, "hypertable_drop_chunks_hook (" INT64_FORMAT " " INT64_FORMAT ")", range_start, range_end); |
svenklemm
reviewed
Sep 19, 2023
tsl/test/src/test_chunk_stats.c
Outdated
Comment on lines
63
to
68
| for (int i = 1; i < 2; i++) | ||
| { | ||
| char *chunk_name; | ||
| chunk_name = psprintf("%s", "_timescaledb_internal.dummy"); | ||
| ret = lappend(ret, chunk_name); | ||
| } |
There was a problem hiding this comment.
what is the purpose of this for loop?
svenklemm
reviewed
Sep 19, 2023
| int64 range_start = ts_internal_to_time_int64(newer_than, dim->fd.column_type); | ||
| int64 range_end = ts_internal_to_time_int64(older_than, dim->fd.column_type); | ||
| Chunk *osm_chunk = ts_chunk_get_by_id(osm_chunk_id, true); | ||
| List *osm_dropped_names = osm_drop_chunks_hook(osm_chunk->table_id, |
There was a problem hiding this comment.
Since those are osm-internal objects it makes no sense to return them to timescaledb.
There was a problem hiding this comment.
the drop_chunks api prints the names of the dropped chunks today.
I think we need to make the call on the OSM side whether these values should be printed or not when drop_chunks is invoked.
So far, we have been consistent with the behavior across APIs.
I don't want to change the hook definition in case we decide that drop_chunks should also behave consistently.
gayyappan
force-pushed
the
bugfix_2
branch
2 times, most recently
from
b6fb493
to
644b58b
Compare
Introduce version number for OsmCallbacks struct Add a callback for cascading drop chunks to OSM and integrate with drop_chunks Add backward compatibility for OsmCallbacks
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.
Add a callback for cascading drop chunks to OSM and integrate with drop_chunks
Add tests to verify that callbacks are triggered for drop_chunks