From 14a77d9a123a8732d70ad98ce021d61835b47b3d Mon Sep 17 00:00:00 2001 From: Stephen von Takach Date: Wed, 16 Sep 2026 11:38:45 +1000 Subject: [PATCH 1/3] docs: plan control system metadata exclusions --- tasks/146-metadata.md | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 tasks/146-metadata.md diff --git a/tasks/146-metadata.md b/tasks/146-metadata.md new file mode 100644 index 00000000..466d0952 --- /dev/null +++ b/tasks/146-metadata.md @@ -0,0 +1,9 @@ +# ControlSystem metadata changefeed exclusion + +Issue: https://github.com/PlaceOS/local/issues/146 +Owner: codex-146-metadata-20260916-root + +- [ ] Regress each of name, description, display_name and version: persisted with no CDC row or PG notification. +- [ ] Retain heartbeat coverage and prove modules-only and mixed modules/metadata updates notify, plus insert/delete and Zone. +- [ ] Extend model declaration; document explicit transition from installed two-column policy. +- [ ] Run focused regression, formatting/lint, full stable/unstable CI; independent review then squash merge. From caa6f0213b03687b18d44918a2ac1459f8491f39 Mon Sep 17 00:00:00 2001 From: Stephen von Takach Date: Wed, 16 Sep 2026 11:52:40 +1000 Subject: [PATCH 2/3] fix: ignore control system metadata changefeed updates --- README.md | 20 +++++++-- shard.override.yml | 4 ++ spec/control_system_changefeed_spec.cr | 60 ++++++++++++++++++++++++-- src/placeos-models/control_system.cr | 7 ++- tasks/146-metadata.md | 10 +++-- 5 files changed, 89 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index ac26e518..3bd67b86 100644 --- a/README.md +++ b/README.md @@ -25,13 +25,25 @@ We use [RethinkDB](https://rethinkdb.com) to unify our database and event bus, g | `PG_LOCK_TIMEOUT` | Timeout on retrying Advisory lock in seconds | 5 | | `PG_DATABASE_URL` | Or provide a Database DSN | | -## Control-system telemetry notifications +## Control-system changefeed notifications -`ControlSystem` uses pg-orm's model-level changefeed policy to ignore updates confined to `signage_last_seen` and `playlist_item_id`. The SQL trigger skips creating a CDC event for those updates, while the timestamp and current item are still persisted. Configuration updates, including updates that also change telemetry, continue to notify; inserts and deletes are unchanged. +`ControlSystem` uses pg-orm's model-level changefeed policy to ignore updates confined to `signage_last_seen`, `playlist_item_id`, `name`, `description`, `display_name`, `version`, `updated_at` and the derived `search_vector` column. The automatic `updated_at` and generated search-vector changes are ignored so ordinary metadata saves are also silent. Other search-vector source fields, such as `code`, remain notification-producing. The SQL trigger skips creating a CDC event for these updates while still persisting the values. Changes to other fields, such as `modules`, continue to notify even when the same update changes ignored fields. Inserts and deletes are unchanged. -The policy is installed when the control-system changefeed is registered. It applies to all writers of these two fields, and no-op updates on `sys` are also silent. Consumers that need current telemetry should query PostgreSQL rather than rely on configuration changefeeds. +The policy is installed when the control-system changefeed is registered. It applies to all writers of these fields, and no-op updates on `sys` are also silent. Consumers that need current telemetry or descriptive metadata should query PostgreSQL rather than rely on changefeeds. -Deploy EventBus 1.1.0 or newer to every service that installs CDC triggers before enabling this models version. Older installers can restore the combined trigger and produce unwanted or duplicate update events. pg-orm 2.4.0 passes the model declaration to EventBus; no database migration or core-side filter is required. For rollback, use EventBus's `replace_cdc_update_policy` with the expected current columns; merely removing the model declaration preserves the installed policy. +Deploy EventBus 1.1.0 or newer to every service that installs CDC triggers before enabling this models version. Older installers can restore the combined trigger and produce unwanted or duplicate update events. pg-orm passes the model declaration, including explicit database-only columns, to EventBus; no core-side filter is required. + +If the earlier two-column policy is already installed, coordinate upgrading all ControlSystem subscribers and explicitly replace that policy before they register the new declaration. Old two-column declarations conflict with the new policy, so avoid overlapping registration by the two versions. From a Crystal process with EventBus loaded and access to the database: + +```crystal +EventBus.new(ENV["PG_DATABASE_URL"]).replace_cdc_update_policy( + "sys", + ignore_update_columns: ["signage_last_seen", "playlist_item_id", "name", "description", "display_name", "version", "updated_at", "search_vector"], + expected_ignore_update_columns: ["signage_last_seen", "playlist_item_id"] +) +``` + +Fresh installations need no replacement. For rollback, use `replace_cdc_update_policy` with the expected current columns; merely removing the model declaration preserves the installed policy. ## Testing diff --git a/shard.override.yml b/shard.override.yml index 80d96a96..876d7f25 100644 --- a/shard.override.yml +++ b/shard.override.yml @@ -1,4 +1,8 @@ dependencies: + # Temporary until pg-orm's database_columns support is released. + pg-orm: + github: spider-gazelle/pg-orm + commit: 50d1f5bffb26b8d33c67e15e914253e26e38dc12 neuroplastic: github: place-labs/neuroplastic commit: 2c156ab8ce688d676ca912b25ccdc631b265952c diff --git a/spec/control_system_changefeed_spec.cr b/spec/control_system_changefeed_spec.cr index 9d81e82a..bc7f0dd0 100644 --- a/spec/control_system_changefeed_spec.cr +++ b/spec/control_system_changefeed_spec.cr @@ -20,6 +20,8 @@ module PlaceOS::Model it "persists heartbeats without CDC rows or notifications and retains configuration events" do system = Generator.control_system item = Generator.item.save! + driver = Generator.driver(role: Driver::Role::Device).save! + mod = Generator.module(driver: driver).save! notifications = Channel(String).new(32) barrier = "signage-barrier-#{RANDOM.hex(8)}" listener = PG::ListenConnection.new(ENV["PG_DATABASE_URL"], ["cdc_events", "signage_spec_barrier"]) do |notification| @@ -47,25 +49,77 @@ module PlaceOS::Model PgORM::Database.connection { |db| db.exec("SELECT pg_notify('signage_spec_barrier', $1)", args: [barrier]) } receive_signage_notification(notifications).should eq(barrier) - system.name = "renamed-#{RANDOM.hex(8)}" + system.modules = [mod.id.as(String)] + system.save! + JSON.parse(receive_signage_notification(notifications))["action"].as_s.should eq("update") + + # Other inputs to the generated search vector still require notifications. + system.code = "room-#{RANDOM.hex(8)}" system.save! JSON.parse(receive_signage_notification(notifications))["action"].as_s.should eq("update") PgORM::Database.connection do |db| - db.exec("UPDATE sys SET description = 'mixed update', signage_last_seen = now(), playlist_item_id = $2 WHERE id = $1", args: [id, item.id]) + db.exec("UPDATE sys SET modules = ARRAY[]::text[], name = $3, description = 'mixed update', display_name = 'Display', version = version + 1, signage_last_seen = now(), playlist_item_id = $2 WHERE id = $1", args: [id, item.id, "mixed-#{RANDOM.hex(8)}"]) end JSON.parse(receive_signage_notification(notifications))["action"].as_s.should eq("update") system.reload! + system.modules.should be_empty system.description.should eq("mixed update") system.playlist_item_id.should eq(item.id) system.destroy JSON.parse(receive_signage_notification(notifications))["action"].as_s.should eq("delete") - signage_cdc_actions(id).should eq(["insert", "update", "update", "delete"]) + signage_cdc_actions(id).should eq(["insert", "update", "update", "update", "delete"]) ensure listener.try &.close feed.try &.stop system.try &.delete item.try &.delete + mod.try &.delete + driver.try &.delete + end + + {"name", "description", "display_name", "version"}.each do |column| + it "persists #{column}-only changes without CDC rows or notifications" do + system = Generator.control_system.save! + id = system.id.as(String) + notifications = Channel(String).new(4) + barrier = "metadata-barrier-#{RANDOM.hex(8)}" + listener = PG::ListenConnection.new(ENV["PG_DATABASE_URL"], ["cdc_events", "metadata_spec_barrier"]) do |notification| + payload = notification.payload + if payload == barrier || (JSON.parse(payload)["table"].as_s == "sys" && JSON.parse(payload)["id"].as_s == id) + notifications.send(payload) + end + end + feed = ControlSystem.changes + before = signage_cdc_actions(id) + value = column == "version" ? "7" : "metadata-#{RANDOM.hex(8)}" + previous_updated_at = system.updated_at + case column + when "name" then system.name = value + when "description" then system.description = value + when "display_name" then system.display_name = value + when "version" then system.version = value.to_i + end + system.save! + system.reload! + system.updated_at.should be > previous_updated_at + PgORM::Database.connection do |db| + db.query_one("SELECT #{column}::text FROM sys WHERE id = $1", args: [id], as: String).should eq(value) + end + if column == "display_name" + system.display_name = nil + system.save! + system.reload! + system.display_name.should be_nil + end + signage_cdc_actions(id).should eq(before) + PgORM::Database.connection { |db| db.exec("SELECT pg_notify('metadata_spec_barrier', $1)", args: [barrier]) } + receive_signage_notification(notifications).should eq(barrier) + ensure + listener.try &.close + feed.try &.stop + system.try &.delete + end end it "keeps other models' update notifications" do diff --git a/src/placeos-models/control_system.cr b/src/placeos-models/control_system.cr index a66e4476..a9210b40 100644 --- a/src/placeos-models/control_system.cr +++ b/src/placeos-models/control_system.cr @@ -73,8 +73,11 @@ module PlaceOS::Model attribute signage_last_seen : Time = -> { 5.hours.ago }, converter: PlaceOS::Model::Timestamps::EpochConverter, type: "integer", format: "Int64", mass_assignment: false belongs_to Playlist::Item, foreign_key: "playlist_item_id" - # Signage telemetry persists without notifying services to reload running drivers. - changefeed_ignore_updates :signage_last_seen, :playlist_item_id + # Telemetry and descriptive metadata do not require running drivers to reload. + # ORM saves advance updated_at; PostgreSQL also regenerates search_vector. + changefeed_ignore_updates :signage_last_seen, :playlist_item_id, + :name, :description, :display_name, :version, :updated_at, + database_columns: [:search_vector] attribute space_config : Hash(String, JSON::Any) = {} of String => JSON::Any diff --git a/tasks/146-metadata.md b/tasks/146-metadata.md index 466d0952..3817e3c6 100644 --- a/tasks/146-metadata.md +++ b/tasks/146-metadata.md @@ -3,7 +3,11 @@ Issue: https://github.com/PlaceOS/local/issues/146 Owner: codex-146-metadata-20260916-root -- [ ] Regress each of name, description, display_name and version: persisted with no CDC row or PG notification. -- [ ] Retain heartbeat coverage and prove modules-only and mixed modules/metadata updates notify, plus insert/delete and Zone. -- [ ] Extend model declaration; document explicit transition from installed two-column policy. +- [x] Regress ordinary saves of each of name, description, display_name and version: persisted with no CDC row or PG notification. +- [x] Retain heartbeat coverage and prove modules-only and mixed modules/metadata updates notify, plus insert/delete and Zone. +- [x] Extend model declaration, including automatic updated_at bookkeeping; document explicit transition from installed two-column policy. - [ ] Run focused regression, formatting/lint, full stable/unstable CI; independent review then squash merge. + +Replan from regression: original policy fails all four metadata save cases. Adding updated_at plus requested fields still fails name/description/display_name because PostgreSQL regenerates unmapped search_vector. Add explicit database_columns in pg-orm PR22 before final models adoption. Do not map fake application attributes for database-derived values. Add code-only positive coverage because it is another search_vector source. Models completion gated on released pg-orm support. + +Final focused green: 6 specs, zero failures/errors against pg-orm PR22. Original regression red all four metadata cases; intermediate red three generated-search-vector cases. Formatting/Ameba pass. pg-orm PR22 merged with 488 specs and CI green. Models uses temporary exact merged commit override for full CI; replace with released minimum and remove override before merge. From 95d236e75c06773be30c9808eb8660ac5cd87a4d Mon Sep 17 00:00:00 2001 From: Stephen von Takach Date: Wed, 16 Sep 2026 11:59:42 +1000 Subject: [PATCH 3/3] chore: adopt pg-orm 2.4.1 release --- README.md | 2 +- shard.override.yml | 4 ---- shard.yml | 2 +- tasks/146-metadata.md | 2 ++ 4 files changed, 4 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 3bd67b86..199a1995 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,7 @@ We use [RethinkDB](https://rethinkdb.com) to unify our database and event bus, g The policy is installed when the control-system changefeed is registered. It applies to all writers of these fields, and no-op updates on `sys` are also silent. Consumers that need current telemetry or descriptive metadata should query PostgreSQL rather than rely on changefeeds. -Deploy EventBus 1.1.0 or newer to every service that installs CDC triggers before enabling this models version. Older installers can restore the combined trigger and produce unwanted or duplicate update events. pg-orm passes the model declaration, including explicit database-only columns, to EventBus; no core-side filter is required. +Deploy EventBus 1.1.0 or newer to every service that installs CDC triggers before enabling this models version. Older installers can restore the combined trigger and produce unwanted or duplicate update events. pg-orm 2.4.1 or newer passes the model declaration, including explicit database-only columns, to EventBus; no core-side filter is required. If the earlier two-column policy is already installed, coordinate upgrading all ControlSystem subscribers and explicitly replace that policy before they register the new declaration. Old two-column declarations conflict with the new policy, so avoid overlapping registration by the two versions. From a Crystal process with EventBus loaded and access to the database: diff --git a/shard.override.yml b/shard.override.yml index 876d7f25..80d96a96 100644 --- a/shard.override.yml +++ b/shard.override.yml @@ -1,8 +1,4 @@ dependencies: - # Temporary until pg-orm's database_columns support is released. - pg-orm: - github: spider-gazelle/pg-orm - commit: 50d1f5bffb26b8d33c67e15e914253e26e38dc12 neuroplastic: github: place-labs/neuroplastic commit: 2c156ab8ce688d676ca912b25ccdc631b265952c diff --git a/shard.yml b/shard.yml index 8701e988..4e5701d4 100644 --- a/shard.yml +++ b/shard.yml @@ -41,7 +41,7 @@ dependencies: # ORM for Postgres built on active-model pg-orm: github: spider-gazelle/pg-orm - version: ">= 2.4.0" + version: ">= 2.4.1" # PlaceOS calendar for Tenant model place_calendar: diff --git a/tasks/146-metadata.md b/tasks/146-metadata.md index 3817e3c6..d5acab3f 100644 --- a/tasks/146-metadata.md +++ b/tasks/146-metadata.md @@ -11,3 +11,5 @@ Owner: codex-146-metadata-20260916-root Replan from regression: original policy fails all four metadata save cases. Adding updated_at plus requested fields still fails name/description/display_name because PostgreSQL regenerates unmapped search_vector. Add explicit database_columns in pg-orm PR22 before final models adoption. Do not map fake application attributes for database-derived values. Add code-only positive coverage because it is another search_vector source. Models completion gated on released pg-orm support. Final focused green: 6 specs, zero failures/errors against pg-orm PR22. Original regression red all four metadata cases; intermediate red three generated-search-vector cases. Formatting/Ameba pass. pg-orm PR22 merged with 488 specs and CI green. Models uses temporary exact merged commit override for full CI; replace with released minimum and remove override before merge. + +Release gate cleared: verified pg-orm v2.4.1 at 41715b4 contains PR22 with only a version change afterward. Require >=2.4.1 and remove temporary override. Prior stable/unstable CI and independent review passed; rerun final CI against the release before merge. Owner resumed as codex-146-release-20260916-root.