Skip to content

Conversation

@mkysel
Copy link
Collaborator

@mkysel mkysel commented Oct 23, 2025

Reduce subscribe worker database poll batch size from 10000 to 1000 to support the motivation to reduce subscribe worker batch size in message.subscribe_worker

Update the constant that controls the subscribe worker poll size in message.subscribe_worker and add comments documenting measurement context.

📍Where to Start

Start with the subscribeWorkerPollRows constant definition and related comments in subscribe_worker.go.


📊 Macroscope summarized 6379f07. 1 files reviewed, 1 issues evaluated, 1 issues filtered, 0 comments posted

🗂️ Filtered Issues

pkg/api/message/subscribe_worker.go — 0 comments posted, 1 evaluated, 1 filtered
  • line 23: The subscribeWorkerPollRows constant change (from 10000 to 1000) has no effect because the pollableQuery function ignores its numRows parameter. In startSubscribeWorker, db.NewDBSubscription is given PollingOptions{ NumRows: subscribeWorkerPollRows }, and it calls the provided pollableQuery(ctx, lastSeen, numRows). However, pollableQuery does not use numRows and calls q.SelectGatewayEnvelopes(ctx, ...) without passing or enforcing any row limit. This silently defeats the intended throughput/row cap and contradicts the newly added comments about limiting to 1000 rows. Depending on the database size, this can cause the worker to fetch arbitrarily large batches, increasing latency, memory usage, and potentially delaying vector clock advancement. The externally visible contract of limiting polled rows per interval is not honored on this execution path. [ Low confidence ]

@mkysel mkysel requested a review from a team as a code owner October 23, 2025 14:14
@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.

@mkysel mkysel enabled auto-merge (squash) October 23, 2025 14:16
@mkysel mkysel merged commit 2d1b00e into main Oct 23, 2025
11 checks passed
@mkysel mkysel deleted the mkysel/reduce-batch-size branch October 23, 2025 14:23
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.

3 participants