Skip to content

perf(session): bulk-load journal segments so replay costs one round-trip - #3666

Open
ningmao-hlyz wants to merge 5 commits into
agentscope-ai:mainfrom
ningmao-hlyz:perf/sqlite-connection-reuse
Open

ningmao-hlyz wants to merge 5 commits into
agentscope-ai:mainfrom
ningmao-hlyz:perf/sqlite-connection-reuse

Conversation

@ningmao-hlyz

@ningmao-hlyz ningmao-hlyz commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

AgentScope-Java Version

2.1.0-SNAPSHOT (main)

Description

Reported in #3612,#3647 : listing the sessions of a user with 17 sessions of a few thousand events

took over 6s on Windows.

Core issue: connection setup overhead during replay. In the original implementation on main, every storage.read call opens a new JDBC connection and executes a table-creation DDL statement. Replay traverses the commit chain segment by segment, reading each segment twice. With 17 sessions × 3,000 events, this results in over 20,000 connection setups, which accounts for the 12.6-second runtime. After switching to batch reads, the entire replay requires only two connections—one for listPaths and one for readAll—bringing the measured runtime down to 1.6 seconds.

Additional benefits: All code paths that use scan benefit as well, including getAgentState, session restoration, and session search.

Changes

  • AtomicSessionStorage gains a readAll(paths) default that falls back to per-path reads.
    SqliteSessionStorage overrides it to serve the whole batch over one connection.
  • JournalSessionLog.scan bulk-loads every committed segment up front (one listPaths + one
    readAll — two round-trips per journal replay instead of two per segment) and walks the hash
    chain in memory. The chain checks (cycle detection, contiguity, checksum) are unchanged; they
    just run over in-memory data. Payload blobs are still hydrated per read. The listing surfaces
    physically-present rows — including segments a concurrent writer has not yet referenced from
    head — so a bulk read that trips over one of those degrades to per-segment reads instead of
    failing the scan.
  • Connection lifetimes are unchanged — every operation still opens and closes its connection —
    so Windows file-handle semantics and all existing tests are untouched.

Measurement

17 sessions × 100 events (one commit per event), traversing all sessions via readAfter(0, N):

storage instance model main this PR
new instance per session ~2.8s ~0.375s
one shared instance ~2.6s ~0.25s

And at a larger scale, 17 sessions × 3000 events (51,000 events, one commit per 5 events):

storage instance model main this PR
new instance per session ~12.6s ~1.6s
one shared instance ~12.8s ~1.4s

How to test

mvn -pl agentscope-harness -am test -Dtest='SqliteSessionStorageTest,WorkspaceSessionLogStoreTest'

All green, plus the full agentscope-core + agentscope-harness suites (4.5k tests) pass locally.

@ningmao-hlyz
ningmao-hlyz marked this pull request as draft October 20, 2026 07:27
@ningmao-hlyz
ningmao-hlyz force-pushed the perf/sqlite-connection-reuse branch from cc51c83 to 4833661 Compare October 20, 2026 17:39
@ningmao-hlyz
ningmao-hlyz deleted the perf/sqlite-connection-reuse branch October 20, 2026 17:39
Journal replay walks the commit chain segment by segment, and each segment
read paid a fresh JDBC connection plus a CREATE TABLE round-trip in
SqliteSessionStorage. Listing the sessions of a user with 17 sessions of 100
events therefore spent most of its time in connection setup (~2.8s measured).

Two changes:

- AtomicSessionStorage gains a readAll(paths) default that falls back to
  per-path reads; SqliteSessionStorage overrides it to serve the whole batch
  over one connection.
- JournalSessionLog.scan bulk-loads every committed segment up front (one
  listPaths + one readAll) and walks the hash chain in memory. Backends
  without path discovery fall back to the previous per-segment reads. Payload
  blobs are still hydrated per read.

Measured traversal of 17 sessions x 100 events: ~2.8s on main, ~0.375s with
per-session storage instances, ~0.25s with one shared instance.
@ningmao-hlyz
ningmao-hlyz restored the perf/sqlite-connection-reuse branch October 20, 2026 18:44
@ningmao-hlyz ningmao-hlyz reopened this Oct 20, 2026
@ningmao-hlyz
ningmao-hlyz force-pushed the perf/sqlite-connection-reuse branch from 4833661 to a0f4730 Compare October 20, 2026 18:47
@ningmao-hlyz
ningmao-hlyz marked this pull request as ready for review October 20, 2026 18:54
@ningmao-hlyz
ningmao-hlyz force-pushed the perf/sqlite-connection-reuse branch from 83fa13c to 2aba8c3 Compare October 20, 2026 19:32
@ningmao-hlyz ningmao-hlyz changed the title perf(harness): reuse the SQLite connection across journal reads perf(session): bulk-load journal segments so replay costs one round-trip Oct 21, 2026
@ningmao-hlyz
ningmao-hlyz force-pushed the perf/sqlite-connection-reuse branch from d223f90 to 74d4ddf Compare October 21, 2026 07:48
JournalSessionLog.scan now lists paths on every replay, which exposed
LocalSessionStorage.listPaths (POSIX branch) to a pre-existing race: a
concurrent delete of the namespace directory made Files.walk throw
NoSuchFileException mid-iteration, failing the whole scan with
UncheckedIOException. Discovery is a best-effort snapshot, so a namespace
that vanishes mid-walk is reported as an empty listing instead.
@ningmao-hlyz
ningmao-hlyz force-pushed the perf/sqlite-connection-reuse branch from 74d4ddf to 9c1a6ac Compare October 21, 2026 07:58
The ticket's own poller thread releases the continuation as soon as the
durable decision becomes visible, so resolveManaged can return
OWNER_UNAVAILABLE when the poller drops the local waiter between the
durable resolution and the owner lookup. The decision is accepted in
both interleavings; accept either return code and keep the effect
assertions exact.
@ningmao-hlyz

Copy link
Copy Markdown
Contributor Author

The Windows failure on the previous head was ToolConfirmationCoordinatorTest.managedTimeoutWaitsForControlPlaneCancelledDecision in service-dataplane — unrelated to the storage changes here. The same race class was already deflaked in #3616 for a sibling test in that file: the ticket's own 200ms poller thread observes the durable decision and releases the continuation, so resolveManaged can legitimately return OWNER_UNAVAILABLE when the poller drops the local waiter between the durable resolution and the owner lookup. The decision is accepted either way; only the return-code assertion was scheduling-dependent.

This branch deflakes it the same way (accepting RESOLVED or OWNER_UNAVAILABLE for that one assertion; the effect assertions — waiter released as cancelled, resolution status, decision event — stay exact).

This branch has not been deployed

No deployments
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.

1 participant