Repository navigation
fix: drop the oldest AI events while too many wait to be written to disk - #901
Open
turnipdabeets wants to merge 6 commits into
Open
turnipdabeets wants to merge 6 commits into
turnipdabeets wants to merge 6 commits into
Conversation
One executor both writes and sends AI events, so during a slow upload new multi-MiB events waited in memory with no limit. captureAi now returns null while captureAiMaxQueueSize events or 50 MiB are waiting, the same limits the disk queue keeps. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Contributor
posthog-android Compliance ReportDate: 2026-10-29T21:56:55.027090+00:00 ✅ All Tests Passed!232/232 tests passed Capture_V1 Tests✅ 105/105 tests passed View Details
Capture_Ai_V1 Tests✅ 110/110 tests passed View Details
Feature_Flags Tests✅ 17/17 tests passed View Details
|
dustinbyrne
approved these changes
Oct 28, 2026
While a slow upload holds the AI executor, captureAi now keeps the new event and drops the oldest events still waiting to be written once captureAiMaxQueueSize events or 50 MiB wait, the same as the disk queue already does. captureAi returns null again only for disabled, opted-out, beforeSend-dropped or oversized events. Co-Authored-By: Claude Opus 5.5 <[email protected]>
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 1 · PR risk: 0/10 |
Dropping a waiting AI event no longer leaves its executor task behind for nothing: a dropped event's task writes a newer one instead, so the executor never holds more tasks than events are waiting. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Contributor
There was a problem hiding this comment.
PostHog Review · Standard · reviewed cf53ab6: 1 Should fix.
See this review and its findings in PostHog Review. Comment @posthog review --deep for a Deep review. A Deep review takes longer and looks deeper than a Standard review.
…ai-pending-cap Moves this branch's queue changes into enqueueEvent(), where #904 put capture's queueing step.
This branch has not been deployed
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.
Stacked on #881.
💡 Motivation and Context
One single-threaded executor both writes AI events to disk and sends them. During a slow upload (up to
captureAiTimeout, 60 s, per request), newcaptureAievents wait in the executor's task queue, in memory, with no limit. The 100-event / 50 MiB caps only applied once an event was written, so a chat app capturing multi-MiB generations on a slow network could exhaust the heap. PostHog Review flagged this on #881 and #882.AI events now wait in a bounded list until the executor writes them. Once more than
captureAiMaxQueueSizeevents (100 by default) or 50 MiB wait, the oldest waiting events are dropped and the new one is kept, the same as the disk queue already does (and as the posthog-ios 4.0 AI queue and the sdk-specscapture-aiclient note describe). These are the disk queue's limits, so holding more in memory couldn't deliver more events; it only delayed the eviction.captureAistill returns the new event's uuid.PostHogQueue.addMeasured(record, bytes): adds the record to the waiting list, drops the oldest waiting records while over the count or bytes, and queues an executor task that writes the oldest waiting record, only while there are fewer tasks than waiting records, so dropped events leave no tasks behind. Only the AI lane uses it; analytics, replay and logs keepadd.captureAipasses the size it already measures for the per-event limit, so there's no second serialization.captureAiMaxQueueSizedescribe the drop.How the other SDKs bound waiting AI events
captureAiwrites to disk on the caller's threadErrQueueFullThis follows posthog-ios and the disk queue (drop the oldest) with mobile limits: a count-only cap of 100 would still allow ~800 MiB of 8 MiB events. A synchronous write like iOS would put disk I/O on Android's main thread. posthog-flutter needs no change: mobile calls the native SDKs, and the desktop Dart lane enforces its caps when it queues.
Splitting writing from sending into separate threads would remove the waiting entirely, but it changes the queue shared by analytics, replay and logs, so it's left out of this PR.
💚 How did you test it?
PostHogQueueByteLimitsTest: with the executor blocked, going over the waiting bytes and then the waiting count each drops the oldest waiting record; the queue ends with the newest three.PostHogCaptureAiTest: with the AI executor blocked andcaptureAiMaxQueueSize = 2, the thirdcaptureAistill returns a uuid and the oldest waiting event is dropped../gradlew apiCheck spotlessCheck :posthog:test :posthog-android:testReleaseUnitTest :posthog-server:testpass locally.📝 Checklist
If releasing new changes
pnpm changesetto generate a changeset file (updated the unreleased v4 major changeset'scaptureAibullet)🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Claude Code compared how posthog-ios, posthog-python, posthog-go and posthog-rs bound AI events waiting to be sent, from their v1/4.0 branch sources, and implemented the cap as a follow-up to #881; after review against sdk-specs #147, the maintainer switched it from drop-the-newest to drop-the-oldest. Rejected: a count-only cap (too much memory on a device), a synchronous write (main-thread I/O), separate write and send threads (touches every queue; possible later).
🤖 Generated with Claude Code