events: snapshot dense index terms lazily so apply latency does not grow with chunk fill

[marwen-abid]

Note

This is a refactor of one of the optimizations originally introduced by @tamirms in #902.
👉 concurrent_bitmaps.go#902

Problem

Hot ingest latency grows as the chunk fills. For one 10,000-ledger chunk: apply p99 goes from 28 ms (first 1,000 ledgers) to 344 ms (all 10,000). Ingest p99 goes from 163 ms to 508 ms. The other five phases do not change.

Cause: ConcurrentBitmaps.AddTo clones the term bitmap on every write to a dense term. With copy-on-write enabled, roaring Clone still allocates and copies the keys, containers and needCopyOnWrite slices and marks every container on both bitmaps for copy. The next AddMany then deep-copies the touched container. The cost is O(containers) per (term, ledger), and the container count grows with the chunk.

Change

Changes how dense terms in the live index are updated: the writer mutates a private bitmap in place under a per-term mutex instead of cloning on every write, and readers get a snapshot that the first reader after a write clones once and shares.

Result

Full-length phase 2 run on c6id.8xlarge, hot leg 10,000 ledgers.

profile	ingest p99 before	ingest p99 after	apply p99 before	apply p99 after
sac-5000	508.5 ms	153.5 ms	344.2 ms	14.2 ms
custom_token-4000	146.4 ms	96.0 ms	65.1 ms	7.9 ms
soroswap-1500	127.1 ms	96.2 ms	39.3 ms	3.8 ms

Before: phase2-pr961-full-c110e623-20260831T141126Z. After: phase2-lazy-snapshot-optimization-full-6cd08c13-20260903T020032Z.

closes #974

[Copilot]

Pull request overview

Lazily snapshots dense event-index bitmaps to prevent ingest latency from growing with chunk size.

Changes:

• Adds writer-private dense bitmaps with lazy immutable snapshots.
• Adds concurrency, freshness, immutability, and warmup tests.
• Updates existing tests for the new dense-state model.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
concurrent_bitmaps.go	Implements lazy dense snapshots.
concurrent_bitmaps_writer_test.go	Adds writer/reader concurrency tests.
concurrent_bitmaps_test.go	Adapts existing dense-state tests.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

cmd/stellar-rpc/internal/rpcv2/stores/event/concurrent_bitmaps.go:24

• The “allocates nothing per AddTo” claim is incorrect: after a snapshot, AddMany allocates when it copy-on-writes a touched container, and normal container growth can also allocate, as the later AddTo documentation acknowledges. Please narrow this claim so the performance contract is accurate.

// denseState is the whole state of one dense term. Exactly one is
// allocated per promoted term and it lives for the term's lifetime,
// so a dense term allocates nothing per AddTo and nothing per
// published snapshot beyond the snapshot itself.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

[marwen-abid]

@claude review

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

cmd/stellar-rpc/internal/rpcv2/stores/event/concurrent_bitmaps_writer_test.go:112

• This race test can pass without exercising any concurrent read: the writer is launched before the readers, and a valid schedule lets it set done before they enter their loops. Gate both sides on a start barrier (and signal reader readiness before releasing the writer) so the test reliably covers AddMany overlapping Get.

This issue also appears in the following locations of the same file:

• line 169
• line 405

cmd/stellar-rpc/internal/rpcv2/stores/event/concurrent_bitmaps_writer_test.go:173

• The writer may complete and set done before any reader starts, in which case this test only checks the final bitmap and never exercises the advertised promotion-under-readers path. Add a readiness/start barrier so at least one reader is active before promotion begins.

	wg.Go(func() {
		defer done.Store(true)
		for i := range uint32(total) {
			s.AddTo(key, i)
			writes.Store(i + 1)

cmd/stellar-rpc/internal/rpcv2/stores/event/concurrent_bitmaps_writer_test.go:407

• These liveness assertions are scheduler-dependent. Because writes are intentionally coalesced while pub is nil, a correct run may publish far fewer than 1,000 snapshots (or the writer may finish before any reader runs, yielding zero reads). Synchronize reader startup with the writer and assert freshness from deterministic handshakes rather than requiring an arbitrary number of observed pointer changes.

	require.Positive(t, reads.Load(), "the stress loop must have done real reads")
	require.GreaterOrEqual(t, republishes.Load(), uint64(numBatches/10),
		"readers must observe snapshots being republished, not one frozen bitmap")

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Suppressed comments (2)

cmd/stellar-rpc/internal/rpcv2/stores/event/concurrent_bitmaps_writer_test.go:169

• This concurrency test can be vacuous: the writer is started first and may set done before the readers are scheduled, after which all reader loops skip and only the final-state assertion runs. Add a startup/interleaving barrier and verify that reads occurred while promotion/writes were active.

	wg.Go(func() {

cmd/stellar-rpc/internal/rpcv2/stores/event/reader.go:108

• This updated contract leaves the preceding “no per-key Clone” claim stale. A lookup of each dirty dense term now calls snapshot() and clones that term, so describe this as an in-memory lookup with a possible lazy clone rather than no clone at all.

	// Bitmap ownership: callers MUST treat returned bitmaps as
	// read-only. The hot path returns immutable snapshots of the
	// live mirror — ConcurrentBitmaps mutates a writer-owned bitmap
	// under a per-term mutex and publishes snapshots lazily, so a
	// returned pointer will never be mutated by anyone; see
	// ConcurrentBitmaps.Get for the full contract. The cold path

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

cmd/stellar-rpc/internal/rpcv2/stores/event/reader.go:103

• This describes all hot snapshots as shared, but sparse terms take the st.ids path and build a fresh bitmap on every lookup. Documenting the two ownership modes separately keeps the Reader contract aligned with ConcurrentBitmaps.Get.

	// snapshots of the live mirror shared by all readers of a term;
	// a dense term written since its last lookup is cloned once, by
	// the first reader to look it up, and that clone is then shared.

cmd/stellar-rpc/internal/rpcv2/stores/event/concurrent_bitmaps_writer_test.go:407

• This is scheduler-dependent and republishes is not a distinct-snapshot count: each reader has its own last, so the same pointer is counted once per reader. The writer may also finish before readers run, leaving reads == 0. Use a start barrier and a deterministic handshake around known publications instead of asserting an aggregate timing-dependent count.

	require.Positive(t, reads.Load(), "the stress loop must have done real reads")
	require.GreaterOrEqual(t, republishes.Load(), uint64(numBatches/10),
		"readers must observe snapshots being republished, not one frozen bitmap")

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (2)

cmd/stellar-rpc/internal/rpcv2/stores/event/concurrent_bitmaps_writer_test.go:196

• This loop can likewise run entirely after all 512 writes: ready.Done only says the reader goroutine started, and the writer may complete immediately after ready.Wait unblocks. In that schedule the test never observes the sparse-to-dense transition it is intended to cover. Synchronize at least one reader observation before promotion and one after the writer begins crossing the threshold, rather than relying on scheduler interleaving.

			for n := 0; !done.Load() || n < minReads; {

cmd/stellar-rpc/internal/rpcv2/stores/event/concurrent_bitmaps_writer_test.go:133

• The readiness barrier does not guarantee any read overlaps the writer: after the last ready.Done, the scheduler can run the writer to completion before a reader enters this loop, and minReads then counts only post-write reads. That lets this race regression pass without exercising concurrent AddMany/snapshot access. Add a deterministic during-write handoff or count reads performed while done is false and require at least one before allowing the writer to finish.

			for i, n := 0, 0; !done.Load() || n < minReads; i++ {

[tamirms]

nice job! this implementation is much simpler than the one in #902

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

cmd/stellar-rpc/internal/rpcv2/stores/event/reader.go:108

• This ownership description is inaccurate for sparse hot terms: ConcurrentBitmaps.Get builds and returns a fresh bitmap on every lookup for those terms (concurrent_bitmaps.go:116-118), so only dense hot snapshots are shared. Please distinguish the two representations so this public reader contract accurately describes bitmap ownership.

cmd/stellar-rpc/internal/rpcv2/stores/event/concurrent_bitmaps_writer_test.go:137

• The new barrier and minReads condition still do not guarantee reader/writer overlap: once all readers call ready.Done, the writer may run to completion, after which each reader can satisfy all minReads iterations against the final snapshot. The test can therefore pass under -race without exercising concurrent Get and AddTo; add a deterministic handoff that requires a read between writer batches, as the freshness stress test below does.

			for i, n := 0, 0; !done.Load() || n < minReads; i++ {
				key := keys[(i+r)%numTerms]
				bm, err := s.Get(key)
				if err != nil || bm == nil {
					continue

cmd/stellar-rpc/internal/rpcv2/stores/event/concurrent_bitmaps_writer_test.go:200

• This loop has the same scheduling gap: ready.Done only proves that reader goroutines started, while n < minReads allows every checked read to happen after done becomes true. Thus the test can pass without observing the sparse-to-dense transition concurrently; synchronize writer progress with reader acknowledgements so at least the promotion and subsequent writes are read while the writer remains active.

			for n := 0; !done.Load() || n < minReads; {
				bm, err := s.Get(key)
				if err != nil || bm == nil {
					continue
				}

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

cmd/stellar-rpc/internal/rpcv2/stores/event/concurrent_bitmaps_writer_test.go:196

• This barrier has the same scheduling gap at the promotion boundary: the writer may execute all 512 additions immediately after ready.Wait() unblocks, after which readers perform their required reads only against the final dense state. In that execution the test never observes the sparse-to-dense transition. Pause the writer around promotionThreshold and require reader progress on both sides of the promotion before allowing it to finish.

cmd/stellar-rpc/internal/rpcv2/stores/event/concurrent_bitmaps_writer_test.go:133

• The readiness barrier does not guarantee that any Get overlaps the writer: after the last ready.Done(), the scheduler may run the writer to completion before a reader enters this loop, and minReads then only counts post-write reads. The test can therefore pass without exercising the reader/writer race it is intended to cover. Add a two-way progress handshake that keeps the writer active until readers have performed work during the write phase.

			ready.Done()
			for i, n := 0, 0; !done.Load() || n < minReads; i++ {