Add chunks to schema-based publications - #10128
Conversation
|
@pnthao, @svenklemm: please review this pull request.
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
0667179 to
403fc97
Compare
ee04e63 to
56832f3
Compare
efe183f to
2a5decb
Compare
| { | ||
| continue; | ||
| } | ||
| chunk_add_to_publication(pubid, chunk); |
There was a problem hiding this comment.
You add individual chunks to the publication when it's hypertable's schema gets added to the publication. What happens when you remove the schema from the publication? These chunks remain and continue publicating updates?
| Publication *pub = GetPublicationByName(pubname, true); | ||
| if (pub) | ||
| { | ||
| ts_chunk_publication_backfill(pub->oid); |
There was a problem hiding this comment.
It scans all schemas even if no schema was added (AP_AddObjects could mean a table) or only one schema added -- not the most efficient. The function name is also misleading IMHO, it only backfills schemas, not hypertables.
047ec2d to
9335964
Compare
zilder
left a comment
zilder
left a comment
There was a problem hiding this comment.
I believe one more case needs special handling -- changing hypertable's schema doesn't remove its chunks from the publication if the old schema was published and doesn't add the chunks if the new schema is.
| const char *pubname = NULL; | ||
| bool is_set_objects = false; | ||
| if (IsA(args.parsetree, CreatePublicationStmt)) | ||
| { | ||
| pubname = castNode(CreatePublicationStmt, args.parsetree)->pubname; | ||
| } | ||
| else if (IsA(args.parsetree, AlterPublicationStmt)) | ||
| { | ||
| AlterPublicationStmt *stmt = castNode(AlterPublicationStmt, args.parsetree); | ||
| pubname = stmt->pubname; | ||
| is_set_objects = (stmt->action == AP_SetObjects); | ||
| } | ||
|
|
||
| /* | ||
| * Snapshot the publication's schema set before the command runs so we can | ||
| * diff it against the post-command set and reconcile chunk membership. The | ||
| * ALTER/CREATE PUBLICATION path takes an AccessExclusiveLock on the | ||
| * publication, so the schema set is stable across prev_ProcessUtility() and | ||
| * a concurrent writer cannot race the before/after snapshots. | ||
| */ | ||
| Publication *old_pub = pubname ? GetPublicationByName(pubname, true) : NULL; | ||
| List *old_schemas = old_pub ? GetPublicationSchemas(old_pub->oid) : NIL; | ||
|
|
There was a problem hiding this comment.
This code (and the code past prev_ProcessUtility) shouldn't go directly into the top level process utility hook handler -- there is an already established way of handling commands in process_ddl_command_start and process_ddl_event_command_end. It best to keep command processing consistent with the rest of the project. We don't really need to make snapshot before and after -- we have the exact schema names being added/dropped/set in the parse tree.
- ALTER PUBLICATION ... ADD/DROP/SET processing should go in
process_ddl_command_start - CREATE PUBLICATION processing should go to
process_ddl_event_command_end(because we need publication's Oid)
e66bfab to
5b9b256
Compare
5b9b256 to
cb4fc2f
Compare
Chunks live in _timescaledb_internal, so FOR TABLES IN SCHEMA publications miss them and replication silently skips chunk data. Backfill chunk membership by reading the schema set straight from the CREATE/ALTER PUBLICATION parse tree (no before/after snapshots): - CREATE PUBLICATION is reconciled in the ddl_command_end event trigger (the publication Oid exists by then); add CREATE PUBLICATION to the timescaledb_ddl_command_end WHEN TAG list. - ALTER PUBLICATION ADD/DROP/SET TABLES IN SCHEMA is reconciled in process_ddl_command_start after prev_ProcessUtility. SET rebuilds the full new schema set (PG drops every tracked chunk row on SET); only DROP removes them. Options-only ALTER is a no-op. ALTER TABLE ... SET SCHEMA on a hypertable now reconciles its chunks: remove from the old schema publications, add to the new ones. Runs before ts_hypertable_set_schema flips the catalog schema. Skip adding a redundant explicit pg_publication_rel row when a chunk already lives in a schema this publication covers natively (chunks placed via associated_schema_name); the explicit row would survive a later DROP TABLES IN SCHEMA and orphan. OSM (foreign-table) chunks are skipped throughout; publication_add_relation would fail on them. Tests 6b-6i cover CREATE/ADD/SET/DROP/CURRENT_SCHEMA, schema moves, the redundancy guard (counts of pg_publication_rel rows), options-only ALTER, and GUC-off on the ALTER/schema-change paths. Signed-off-by: Arunprasad Rajkumar <ar.arunprasad@gmail.com>
cb4fc2f to
02e95d2
Compare
4 participants
When a publication is created with
FOR TABLES IN SCHEMAcovering a hypertable's schema, chunks live in _timescaledb_internal so PG does not include them. Replication then silently misses chunk data.Extend the per-chunk auto-publication hook to also consider schema publications via GetSchemaPublications. For pre-existing chunks, a process utility hook runs after CREATE/ALTER PUBLICATION and syncs the publication to match its schema set: it snapshots the publication's schemas before and after the command, then adds chunks for schemas that were added and removes chunks for schemas that were dropped. This keeps chunk membership consistent for ADD, DROP and SET TABLES IN SCHEMA. Without the removal step, chunks added while a schema was published stay orphaned in pg_publication_rel and keep replicating after the schema is
dropped from the publication.
SET replaces the whole object list, so PG drops every chunk row we added even for schemas that remain; that path rebuilds chunks for all schemas still in the publication rather than just the newly added ones.
OSM chunks are foreign tables and would make publication_add_relation fail, so the sync skips them. PG already expands inheritance children for FOR TABLE and FOR ALL TABLES, so no changes are needed for those variants.
Signed-off-by: Arunprasad Rajkumar ar.arunprasad@gmail.com