Skip to content

Conversation

@mkysel
Copy link
Collaborator

@mkysel mkysel commented Oct 23, 2025

SelectVectorClock never finishes. Even with a proper secondary index, the query is just too complicated. I've played around with a few alternatives and having a dedicated table that contains exactly 1 record seems like the best way forward.

The trigger keeps the table updated and transactional.

The time went down to sub 100ms.

If the best attempt fails, the consequence is that the publish worker will have to do some useless work when spinning up.

@mkysel mkysel requested a review from a team as a code owner October 23, 2025 15:59
@graphite-app
Copy link

graphite-app bot commented Oct 23, 2025

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • Queue - adds this PR to the back of the merge queue
  • Hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

@macroscopeapp
Copy link

macroscopeapp bot commented Oct 23, 2025

Route Queries.SelectVectorClock to read from gateway_envelopes_latest to speed up vector clock selection

Replace the vector clock query to read latest rows directly from gateway_envelopes_latest and update types to match the new source.

  • Rewrite sqlc SelectVectorClock to select originator_node_id, originator_sequence_id, gateway_time from gateway_envelopes_latest with ORDER BY originator_node_id in envelopes.sql
  • Add queries.GatewayEnvelopesLatest struct with OriginatorNodeID, OriginatorSequenceID, GatewayTime in models.go
  • Change generated code to return []queries.GatewayEnvelopesLatest in envelopes.sql.go
  • Update db.ToVectorClock to accept []queries.GatewayEnvelopesLatest in types.go
  • Add migrations to create and seed gateway_envelopes_latest with a trigger and function, and provide a down migration in 00020_add_latest_envelopes.up.sql and 00020_add_latest_envelopes.down.sql

📍Where to Start

Start with the SelectVectorClock query in envelopes.sql, then review the corresponding generated code in envelopes.sql.go and the struct in models.go.

Changes since #1272 opened

  • Modified the update_latest_envelope trigger function to conditionally update records based on sequence ID comparison [535aa23]

📊 Macroscope summarized 535aa23. 1 files reviewed, 2 issues evaluated, 1 issues filtered, 0 comments posted

🗂️ Filtered Issues

pkg/db/types.go — 0 comments posted, 2 evaluated, 1 filtered
  • line 37: ToVectorClock silently overwrites entries in vc when multiple rows share the same OriginatorNodeID. The last row in the slice 'wins', which may not be the latest sequence if the input isn't strictly unique and correctly ordered. There is no explicit enforcement of uniqueness, adjacency, or recency constraints for a vector clock, leading to potential data loss or inconsistent state if the query returns duplicates or out-of-order results. The function should validate uniqueness per OriginatorNodeID (and optionally enforce monotonicity) or fail loudly to avoid silent corruption. [ Low confidence ]

Comment on lines +19 to +21
CREATE TRIGGER gateway_latest_upd
AFTER INSERT ON gateway_envelopes
FOR EACH ROW EXECUTE FUNCTION update_latest_envelope();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doing this for every row seems kind of heavy. Is there a way to do this once for each transaction from the app?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

by definition every insert is always the newest. Out-of-order inserts would be a problem. There are multiple app backend services that might be doing inserts to their corresponding originator_id.

Comment on lines 13 to 14
SET originator_sequence_id = EXCLUDED.originator_sequence_id,
gateway_time = EXCLUDED.gateway_time;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These can move backward. Probably want greatest(OLD.gateway_time, EXCLUDED.gateway_time) to enforce that they only move forward.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

valid. There is no application path that would move backwards. All inserts are inserted in sequenceID order in all cases. Otherwise last_seen and vectors break.

It can not move even in an HA case because we have

-- name: InsertGatewayEnvelope :execrows
INSERT INTO gateway_envelopes(
		originator_node_id,
		originator_sequence_id,
		topic,
		originator_envelope,
		payer_id,
		gateway_time,
        expiry
	)
VALUES (
		@originator_node_id,
		@originator_sequence_id,
		@topic,
		@originator_envelope,
		@payer_id,
		COALESCE(@gateway_time, NOW()),
        @expiry
	) ON CONFLICT DO NOTHING;

so DO NOTHING won't trigger the helper table in HA cases.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added WHERE EXCLUDED.originator_sequence_id > g.originator_sequence_id; to be defensive.

@mkysel mkysel merged commit 3a7dfd9 into main Oct 24, 2025
11 of 12 checks passed
@mkysel mkysel deleted the mkysel/speed-up-vectors branch October 24, 2025 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants