mirror of
https://github.com/langchain-ai/langgraph.git
synced 2026-10-14 20:25:23 +02:00
Fixes langchain-ai/langgraph#8382 `DeltaChannel` rebuilds its value by replaying ancestor writes through the reducer. Every saver ordered a checkpoint's writes by `(task_id, idx)`, but live execution applies a super-step's writes in task-path order (`apply_writes` sorts tasks by `task_path_str(task.path[:3])`). `task_id` is a hash of the path, so when two or more tasks write one `DeltaChannel` in the same super-step, replay applies them in an arbitrary permutation. Reducers must be batching-invariant, not order-invariant, so the permutation changes the value: `get_state` disagrees with what `invoke` returned, and continuing the thread saves the reordered value as the base for every later write. ## Fix Replay orders by `(task_path, task_id, idx)`, the order `SELECT_PENDING_SENDS_SQL` already uses for sends. `apply_writes` and every saver now sort with one `writes_sort_key` from `langgraph.checkpoint.base`, in Python, so live and replay can't drift apart and don't depend on a database collation. `apply_writes` sorts by the full task path, the string `put_writes` stores, instead of `path[:3]`. The paths that reach `apply_writes` no longer carry task ids, so the live order doesn't change. `BaseCheckpointSaver`'s default `get_delta_channel_history` can't sort by path, since `PendingWrite` has no task path, so it replays each checkpoint's writes in `get_tuple` order. `get_tuple` now has to return `pending_writes` in `writes_sort_key` order, and memory, SQLite and Postgres do, so the default walk is right for any saver that follows that. `AsyncPostgresSaver` also gets the sync `get_delta_channel_history` that `AsyncSqliteSaver` already had; sync reads used to fall back to the default walk. Memory and Postgres already stored `task_path`. SQLite accepted it on `put_writes` and dropped it, so `writes` gains the column and `setup()` adds it to existing databases. Writes without a task path (graph input, `update_state` updates, rows written before the column existed) sort first by `task_id`, which is also where live execution applies input. ## Compatibility - Memory and Postgres already had `task_path` on disk, so existing threads replay in the corrected order after upgrade. - On SQLite, rows from before the upgrade read back as `''` and keep their old order. Only new writes get the corrected order, since there is no stored path to recover. - SQLite has no `ADD COLUMN IF NOT EXISTS`, so `setup()` runs the `ALTER` and treats `duplicate column name` as success, which also covers two processes migrating one file. That error comes while SQLite prepares the statement, before it asks for the write lock, so `setup()` on an up-to-date database doesn't wait on other writers. A read-only database from before the column is read with empty paths instead of failing setup. - Downgrade is safe: older code's `INSERT` omits the column and the column has a default. - `get_tuple` and `list` return `pending_writes` in `writes_sort_key` order instead of task id order (SQLite, Postgres) or put order (memory). Nothing in langgraph depends on the old order. - Postgres reads `task_path` and `idx` with each pending write, so a subclass that pairs `SELECT_SQL` with its own `_load_writes`, or the other way round, has to match the new row shape. - A third-party saver has to replay in `writes_sort_key` order: in its own `get_delta_channel_history` if it has one, otherwise by returning `pending_writes` from `get_tuple` in that order. Otherwise it fails the new conformance tests; the `get_tuple` one is a base test, so it runs for every saver. - langgraph, checkpoint-sqlite and checkpoint-postgres import `writes_sort_key` from langgraph-checkpoint, so this bumps langgraph-checkpoint to 4.3.0 and requires it there. 4.3.0 has to be released first. ## Limits - Several updates in one `bulk_update_state` super-step share a task id, so the saver keeps only the first one's writes and the `DeltaChannel` loses the rest (plain channels are fine). That's on main too; #9128 fixes it and stores a task path per update, so with this PR those updates also replay in the order given. - Exit durability stores its writes differently; resume ordering there is fixed in #9114, stacked on this PR. - langgraphjs applies `DeltaChannel` writes in task id order, live and on replay (langchain-ai/langgraphjs#2544), so its reads match too. Moving JS to task path order is a follow-up. ## Tests Each fails on main and passes here: - `test_delta_channel_parallel_order.py`, across memory, SQLite and Postgres: parallel writers and a `Send` fan-out, comparing `get_state` with the live result, including sync `get_state` on the async savers. - Three conformance tests whose task ids sort opposite to their task paths, so they cannot pass by chance: two on `get_delta_channel_history`, one on `get_tuple`'s `pending_writes` order. - SQLite migration tests from a pre-column database, sync and async, including a read-only file. One more that passes on main too: SQLite `setup()` while another connection holds the write lock, with no busy timeout, sync and async. It keeps the `ALTER` from waiting on that lock. Thanks to @ErenAta16 for the report, the reproduction, and for tracing `task_path` through all three backends, and to @ragnarok268 for the conformance-test design with task ids ordered opposite to their paths. --------- Co-authored-by: ErenAta16 <149434812+ErenAta16@users.noreply.github.com> Co-authored-by: ragnarok268 <58264829+ragnarok268@users.noreply.github.com>
152 lines
5.0 KiB
Python
152 lines
5.0 KiB
Python
import sqlite3
|
|
from collections.abc import Iterator
|
|
from contextlib import closing
|
|
from pathlib import Path
|
|
|
|
import aiosqlite
|
|
import pytest
|
|
from langgraph.checkpoint.base import empty_checkpoint
|
|
|
|
from langgraph.checkpoint.sqlite import SqliteSaver
|
|
from langgraph.checkpoint.sqlite.aio import AsyncSqliteSaver
|
|
|
|
WRITES_BEFORE_TASK_PATH = """
|
|
CREATE TABLE writes (
|
|
thread_id TEXT NOT NULL,
|
|
checkpoint_ns TEXT NOT NULL DEFAULT '',
|
|
checkpoint_id TEXT NOT NULL,
|
|
task_id TEXT NOT NULL,
|
|
idx INTEGER NOT NULL,
|
|
channel TEXT NOT NULL,
|
|
type TEXT,
|
|
value BLOB,
|
|
PRIMARY KEY (thread_id, checkpoint_ns, checkpoint_id, task_id, idx)
|
|
);
|
|
INSERT INTO writes VALUES ('t', '', 'c', 'old-task', 0, 'ch', 'null', X'');
|
|
"""
|
|
|
|
|
|
@pytest.fixture
|
|
def legacy_db(tmp_path: Path) -> Path:
|
|
db = tmp_path / "legacy.sqlite"
|
|
with sqlite3.connect(db) as conn:
|
|
conn.executescript(WRITES_BEFORE_TASK_PATH)
|
|
return db
|
|
|
|
|
|
def test_setup_migrates_legacy_writes_table_repeatably(legacy_db: Path) -> None:
|
|
for _ in range(2):
|
|
with SqliteSaver.from_conn_string(str(legacy_db)) as saver:
|
|
saver.setup()
|
|
rows = saver.conn.execute(
|
|
"SELECT task_id, task_path FROM writes"
|
|
).fetchall()
|
|
assert rows == [("old-task", "")]
|
|
|
|
|
|
@pytest.mark.parametrize("fresh", [True, False], ids=["fresh", "legacy"])
|
|
def test_put_writes_persists_task_path(
|
|
tmp_path: Path, legacy_db: Path, fresh: bool
|
|
) -> None:
|
|
db = tmp_path / "fresh.sqlite" if fresh else legacy_db
|
|
with SqliteSaver.from_conn_string(str(db)) as saver:
|
|
config = saver.put(
|
|
{"configurable": {"thread_id": "t", "checkpoint_ns": ""}},
|
|
empty_checkpoint(),
|
|
{},
|
|
{},
|
|
)
|
|
saver.put_writes(config, [("ch", "v")], "task-1", "~__pregel_pull, node")
|
|
stored = saver.conn.execute(
|
|
"SELECT task_path FROM writes WHERE task_id = 'task-1'"
|
|
).fetchall()
|
|
assert stored == [("~__pregel_pull, node",)]
|
|
|
|
|
|
async def test_async_setup_migrates_legacy_writes_table_repeatably(
|
|
legacy_db: Path,
|
|
) -> None:
|
|
for _ in range(2):
|
|
async with AsyncSqliteSaver.from_conn_string(str(legacy_db)) as saver:
|
|
await saver.setup()
|
|
config = await saver.aput(
|
|
{"configurable": {"thread_id": "t", "checkpoint_ns": ""}},
|
|
empty_checkpoint(),
|
|
{},
|
|
{},
|
|
)
|
|
await saver.aput_writes(
|
|
config, [("ch", "v")], "task-1", "~__pregel_pull, node"
|
|
)
|
|
|
|
async with aiosqlite.connect(legacy_db) as conn:
|
|
async with conn.execute(
|
|
"SELECT DISTINCT task_id, task_path FROM writes ORDER BY task_id"
|
|
) as cur:
|
|
assert await cur.fetchall() == [
|
|
("old-task", ""),
|
|
("task-1", "~__pregel_pull, node"),
|
|
]
|
|
|
|
|
|
@pytest.fixture
|
|
def busy_db(tmp_path: Path) -> Iterator[Path]:
|
|
db = tmp_path / "busy.sqlite"
|
|
with SqliteSaver.from_conn_string(str(db)) as saver:
|
|
saver.setup()
|
|
with closing(sqlite3.connect(db, isolation_level=None)) as writer:
|
|
writer.execute("BEGIN IMMEDIATE")
|
|
yield db
|
|
writer.execute("ROLLBACK")
|
|
|
|
|
|
def test_setup_does_not_wait_on_another_writer(busy_db: Path) -> None:
|
|
with closing(sqlite3.connect(busy_db, timeout=0)) as conn:
|
|
SqliteSaver(conn).setup()
|
|
|
|
|
|
async def test_async_setup_does_not_wait_on_another_writer(busy_db: Path) -> None:
|
|
async with aiosqlite.connect(busy_db, timeout=0) as conn:
|
|
await AsyncSqliteSaver(conn).setup()
|
|
|
|
|
|
def _legacy_database_with_history(db: Path) -> dict:
|
|
root = empty_checkpoint()
|
|
root["channel_values"] = {"ch": "seed"}
|
|
root["channel_versions"] = {"ch": 1}
|
|
with SqliteSaver.from_conn_string(str(db)) as saver:
|
|
root_config = saver.put(
|
|
{"configurable": {"thread_id": "t", "checkpoint_ns": ""}},
|
|
root,
|
|
{},
|
|
{"ch": 1},
|
|
)
|
|
saver.put_writes(root_config, [("ch", "write")], "task", "~__pregel_pull, n")
|
|
child = saver.put(root_config, empty_checkpoint(), {}, {})
|
|
saver.conn.execute("ALTER TABLE writes DROP COLUMN task_path")
|
|
saver.conn.commit()
|
|
return child
|
|
|
|
|
|
def test_read_only_legacy_database_still_reads_delta_history(tmp_path: Path) -> None:
|
|
db = tmp_path / "legacy.sqlite"
|
|
child = _legacy_database_with_history(db)
|
|
|
|
saver = SqliteSaver(sqlite3.connect(f"file:{db}?mode=ro", uri=True))
|
|
got = saver.get_delta_channel_history(config=child, channels=["ch"])
|
|
|
|
assert got["ch"] == {"seed": "seed", "writes": [("task", "ch", "write")]}
|
|
|
|
|
|
async def test_async_read_only_legacy_database_still_reads_delta_history(
|
|
tmp_path: Path,
|
|
) -> None:
|
|
db = tmp_path / "legacy.sqlite"
|
|
child = _legacy_database_with_history(db)
|
|
|
|
async with aiosqlite.connect(f"file:{db}?mode=ro", uri=True) as conn:
|
|
saver = AsyncSqliteSaver(conn)
|
|
got = await saver.aget_delta_channel_history(config=child, channels=["ch"])
|
|
|
|
assert got["ch"] == {"seed": "seed", "writes": [("task", "ch", "write")]}
|