fix(graph): make PostgresSaver latest checkpoint deterministic#4753
Open
GaoSSR wants to merge 1 commit into
Open
fix(graph): make PostgresSaver latest checkpoint deterministic#4753GaoSSR wants to merge 1 commit into
GaoSSR wants to merge 1 commit into
Conversation
3e9131b to
73250fa
Compare
Contributor
Author
|
CI note: the
The same failures are present on a recent The |
Order PostgreSQL checkpoints by an auto-increment sequence instead of saved_at so same-timestamp writes return the real latest checkpoint. Add a regression test that forces tied saved_at values and verifies list/get both return checkpoints in insert order.
73250fa to
326ada8
Compare
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.
Describe what this PR does / why we need it
PostgresSavercurrently orders checkpoints bysaved_atwhen loading checkpoint history and the latest checkpoint. In concurrent writes, two checkpoints can share the same timestamp, so the database can return either row first.This PR makes PostgreSQL checkpoint ordering deterministic by using an auto-increment sequence column, matching the MySQL saver behavior.
Does this pull request fix one issue?
NONE
Describe how you did it
checkpoint_seqsequence column toGraphCheckpointand create it for existing tables when tables are initialized.checkpoint_seq DESCinstead ofsaved_at DESC.saved_atvalues and verifies list/get return the newest inserted checkpoint.Describe how to verify it
CI=true ./mvnw -pl spring-ai-alibaba-graph-core -Dtest=PostgresSaverTest#testPostgresSaverOrdersCheckpointsByInsertSequenceWhenSavedAtTies testCI=true ./mvnw -pl spring-ai-alibaba-graph-core -Dtest=PostgresSaverTest testgit diff --checkSpecial notes for reviews
The sequence is only used for deterministic ordering. Existing
saved_atvalues remain unchanged, andCreateOption.CREATE_NONEstill does not modify existing schemas.