BE-2: Use the Postgres clock for writing timestamps and resolving temporal axes - #9111
Draft
claude[bot] wants to merge 5 commits into
Draft
BE-2: Use the Postgres clock for writing timestamps and resolving temporal axes#9111claude[bot] wants to merge 5 commits into
claude[bot] wants to merge 5 commits into
Conversation
…poral axes Temporal axes were resolved with the graph host's clock while ontology writes stamp rows with the Postgres clock, so any drift between the two made archive/unarchive-then-read return stale results. The database clock is now the single time authority: - `determine_actor` gains a sibling `determine_actor_with_timestamp` which returns `statement_timestamp()` from the same statement, and `PolicyComponents` carries that reading so every operation gets a database clock value without an extra round trip. - All production `QueryTemporalAxesUnresolved::resolve()` call sites now use `resolve_with` with the operation's database timestamp; the parameterless `resolve()` is removed so the host clock can no longer leak into query resolution. - Entity writes source their transaction time from the database: `create_entities` from its first in-transaction statement, `patch_entity` from the locking statement (which now filters on and returns `statement_timestamp()`), and entity deletion from a `current_timestamp` read on the write transaction. - Adds an archive/unarchive round-trip integration test and a test asserting resolved axes come from the database clock.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #9111 +/- ##
==========================================
- Coverage 59.31% 59.26% -0.05%
==========================================
Files 1406 1406
Lines 135972 136087 +115
Branches 6376 6380 +4
==========================================
+ Hits 80650 80655 +5
- Misses 54338 54448 +110
Partials 984 984
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
`yarn install --immutable` rejects the install when a package.json declares a dependency the lockfile does not record. Also import `Cow` from `alloc` to match the integration-test crate's convention.
- add the `timestamp` field to the remaining `PolicyComponents` test initializer - drop the `EntityTypeStore` trait import which is no longer used now that the entity read paths call the inherent `get_closed_multi_entity_types_impl` - expect `clippy::too_many_lines` on `execute_entity_deletion`
Clippy's `missing_const_for_fn` flags `set_timestamp`, and making it const makes `with_timestamp` const-eligible as well.
Ontology create/archive/unarchive statements stamped rows with inline SQL `now()`, which is frozen for the lifetime of a transaction. Inside a shared transaction — such as the integration-test harness's rollback transaction — archiving a type created earlier in the same transaction therefore produced an empty interval, which cannot be decoded when the statement returns it. The statements now take the operation's database clock reading as a parameter, like the entity write paths: archive/unarchive reuse the reading carried by their `PolicyComponents`, while create/update fetch one at the start of their transaction. All statements of one operation keep sharing a single value, and separate operations get distinct stamps even within one transaction.
Contributor
Benchmark results
|
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| resolve_policies_for_actor | user: empty, selectivity: high, policies: 2002 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: medium, policies: 1002 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: high, policies: 3314 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: medium, policies: 1527 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: high, policies: 2078 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: medium, policies: 1033 | Flame Graph |
policy_resolution_medium
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| resolve_policies_for_actor | user: empty, selectivity: high, policies: 102 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: medium, policies: 52 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: high, policies: 269 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: medium, policies: 108 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: high, policies: 133 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: medium, policies: 63 | Flame Graph |
policy_resolution_none
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| resolve_policies_for_actor | user: empty, selectivity: high, policies: 2 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: medium, policies: 2 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: high, policies: 8 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: medium, policies: 3 | Flame Graph |
policy_resolution_small
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| resolve_policies_for_actor | user: empty, selectivity: high, policies: 52 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: medium, policies: 26 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: high, policies: 94 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: medium, policies: 27 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: high, policies: 66 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: medium, policies: 29 | Flame Graph |
read_scaling_complete
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_id;one_depth | 1 entities | Flame Graph | |
| entity_by_id;one_depth | 10 entities | Flame Graph | |
| entity_by_id;one_depth | 25 entities | Flame Graph | |
| entity_by_id;one_depth | 5 entities | Flame Graph | |
| entity_by_id;one_depth | 50 entities | Flame Graph | |
| entity_by_id;two_depth | 1 entities | Flame Graph | |
| entity_by_id;two_depth | 10 entities | Flame Graph | |
| entity_by_id;two_depth | 25 entities | Flame Graph | |
| entity_by_id;two_depth | 5 entities | Flame Graph | |
| entity_by_id;two_depth | 50 entities | Flame Graph | |
| entity_by_id;zero_depth | 1 entities | Flame Graph | |
| entity_by_id;zero_depth | 10 entities | Flame Graph | |
| entity_by_id;zero_depth | 25 entities | Flame Graph | |
| entity_by_id;zero_depth | 5 entities | Flame Graph | |
| entity_by_id;zero_depth | 50 entities | Flame Graph |
read_scaling_linkless
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_id | 1 entities | Flame Graph | |
| entity_by_id | 10 entities | Flame Graph | |
| entity_by_id | 100 entities | Flame Graph | |
| entity_by_id | 1000 entities | Flame Graph | |
| entity_by_id | 10000 entities | Flame Graph |
representative_read_entity
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/block/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/book/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/building/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/organization/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/page/v/2
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/person/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/playlist/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/song/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/uk-address/v/1
|
Flame Graph |
representative_read_entity_type
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| get_entity_type_by_id | Account ID: bf5a9ef5-dc3b-43cf-a291-6210c0321eba
|
Flame Graph |
representative_read_multiple_entities
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_property | traversal_paths=0 | 0 | |
| entity_by_property | traversal_paths=255 | 1,resolve_depths=inherit:1;values:255;properties:255;links:127;link_dests:126;type:true | |
| entity_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:0;links:0;link_dests:0;type:false | |
| entity_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:0;links:1;link_dests:0;type:true | |
| entity_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:2;links:1;link_dests:0;type:true | |
| entity_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:2;properties:2;links:1;link_dests:0;type:true | |
| link_by_source_by_property | traversal_paths=0 | 0 | |
| link_by_source_by_property | traversal_paths=255 | 1,resolve_depths=inherit:1;values:255;properties:255;links:127;link_dests:126;type:true | |
| link_by_source_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:0;links:0;link_dests:0;type:false | |
| link_by_source_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:0;links:1;link_dests:0;type:true | |
| link_by_source_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:2;links:1;link_dests:0;type:true | |
| link_by_source_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:2;properties:2;links:1;link_dests:0;type:true |
scenarios
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| full_test | query-limited | Flame Graph | |
| full_test | query-unlimited | Flame Graph | |
| linked_queries | query-limited | Flame Graph | |
| linked_queries | query-unlimited | Flame Graph |
This file contains hidden or 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
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.
Requested by Tim Diekmann · Slack thread
🌟 What is the purpose of this PR?
Before: the Graph resolved temporal axes (the "now" a query runs against) with the host's clock, while ontology writes stamp rows with the Postgres clock. With any drift between the two (~50 ms is enough), a client could archive an entity type and still get it back from the next read, or unarchive it and have it stay invisible — the read's "now" lands on the wrong side of the interval bound the write just recorded. Entity writes were host-clock-stamped too, so DB-
now()-based lookups (e.g. user-by-email) had the mirror-image race.After: the Postgres clock is the only clock for query-relevant timestamps. Reads resolve their temporal axes with a timestamp read from the database, and all writes — entity and ontology alike — take their transaction time from a database clock reading fetched once per operation and passed to every statement as a parameter. Archive-then-read and unarchive-then-read are consistent regardless of host clock drift, and all timestamps within one operation share a single value.
How (zero extra round trips on the hot paths): every operation already starts by building
PolicyComponents, whose first statement is thedetermine_actoractor lookup. That statement now additionally returnsstatement_timestamp(), andPolicyComponentscarries the reading so allresolve_with(...)calls and write stamps of the operation reuse it.statement_timestamp()(rather thannow()) is used for all database clock reads: it aligns with theREPEATABLE READsnapshot (taken at the transaction's first statement, not atBEGIN) and it still advances per statement inside a shared transaction — the integration-test harness runs each test in one rollback transaction, where a frozennow()gives distinct operations identical timestamps (and empty, undecodable intervals for archive-after-create).🔗 Related links
🔍 What does this change?
hash-graph-authorization:PrincipalStoregainsdetermine_actor_with_timestamp(actor lookup +statement_timestamp()in one statement; for the public actor only the clock is read) andcurrent_timestamp(standalone clock read, used as a fallback).PolicyComponentscarries the database clock reading, exposed viaPolicyComponents::timestamp(); the builder acceptswith_timestamp(...)for callers that already captured the operation's reading.hash-graph-store: the parameterlessQueryTemporalAxesUnresolved::resolve()is removed — callers must supply a timestamp viaresolve_with(...), so the host clock can no longer leak into query resolution.hash-graph-postgres-store, read paths:resolve_with(...)with the operation's database timestamp: entity reads (query_entities,query_entity_subgraph,summarize_entities,get_entity_by_id), ontology reads (query/count/subgraph for data/property/entity types),has_permission_for_*, and theStoreProvidervalidation lookups (which now carry the surrounding operation's timestamp).query_entities/query_entity_subgraphpreviously resolved twice per request (query pass and permission pass) with two different host-clock values; both passes now share the one database reading.hash-graph-postgres-store, write paths:create_entitiestakes its transaction time from its first in-transaction statement (the actor lookup) instead of the host clock before the transaction;remove_nanosecond()is dropped there since Postgres timestamps are µs-precision by construction.patch_entitytakes its transaction time fromlock_entity_edition, whose SQL now filters onCOALESCE($n, statement_timestamp())and returns the reading (LockedEntityEdition::locked_at); a client-supplied decision time is still honoured in the lock'sWHERE.Timestamp::now().create_ontology_temporal_metadata,archive_ontology_type,unarchive_ontology_type, and theircreate_ontology_metadata/update_owned_ontology_idwrappers) no longer stamp with inline SQLnow(): they take the operation's database clock reading as a parameter — archive/unarchive reuse thePolicyComponentsreading, create/update fetch one at the start of their transaction. This gives distinct per-operation stamps even when operations share a transaction, while all statements of one operation still share a single value.Round-trip accounting: authenticated reads and entity writes get the timestamp for free from the existing actor-lookup statement. A standalone
SELECT statement_timestamp()only happens on paths that never ran the lookup first: ontology create/update transactions, public-actor requests,PolicyComponentsbuilt from an already-resolvedActorIdwithout a supplied timestamp (create_web, policy CRUD internals), entity deletion, snapshot restore, and theget_closed_multi_entity_typesREST endpoint. No hot read path pays an extra round trip.Pre-Merge Checklist 🚀
🚢 Has this modified a publishable library?
This PR:
📜 Does this require a change to the docs?
The changes in this PR:
🕸️ Does this require a change to the Turbo Graph?
The changes in this PR:
policy_editionetc.) still stamp with inline SQLnow(). They are not resolved throughQueryTemporalAxesand are read with SQL-sidenow()comparisons on the same database clock, so they are unaffected by the drift this PR fixes; moving them onto the parameterized mechanism would be a mechanical follow-up.[patch.crates-io]git pins (libp2p et al.) are unreachable from the sandbox this was authored in, so localcargo check/clippy/nextestcould not run at all — rustfmt passes and the diff was reviewed carefully, but compile and test verification is deferred to CI. TheCargo.lockchange is the single hand-added dependency edge forhash-graph-authorization→hash-graph-temporal-versioning(workspace members carry no checksums);misewas likewise unavailable, so the generatedpackage.jsonwiring was mirrored by hand exactly like sibling crates.🐾 Next steps
disallowed-methodsguardrail forTimestamp::now()/OffsetDateTime::now_utc()in the store crates could enforce this statically; left out to avoid noise from the legitimate test-only uses.PolicyComponentsbuilding and should keep returning the clock reading.Interval::from_sqlstill panics (unimplemented!) on empty ranges. No write path can produce one anymore (per-operation stamps are strictly increasing within a transaction), but decoding one defensively as an error would be safer.🛡 What tests cover this?
archive_unarchive_round_tripandresolved_temporal_axes_use_database_clockintests/graph/integration/postgres/data_type.rs. The round-trip test also guards the ontology write stamps: with transaction-frozennow()stamps it fails inside the rollback-transaction harness (archive of a type created in the same test produced an empty, undecodable interval).query/compile/tests.rscovers the compiler against explicitly resolved axes.❓ How to test this?
cargo nextest run --package hash-graph-integration) against a local Postgres.