Fix Sentry 1GDC/1GEB/1GAF/1G8Z: move Reader DB writes off main thread#22861
Conversation
… off main Volley's ResponseDeliveryRunnable runs callbacks on the main thread, and two Reader paths then performed a SQLite compileStatement + execute on that callback. On a busy SQLite connection pool this exceeded the ANR budget. ReaderTagTable.setTagLastUpdated (called from ReaderPostRepository.requestPostsWithTag and requestPostsForLatestStream) now runs on a background Thread. ReaderBlogActions.handleUpdateBlogInfoResponse now performs ReaderBlogTable.addOrUpdateBlog on a background Thread and posts UpdateBlogInfoListener.onResult back on the main thread, since the existing callers (ReaderSiteHeaderView, ReaderFetchSiteUseCase) expect the result on main. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Generated by 🚫 Danger |
|
|
|
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #22861 +/- ##
==========================================
- Coverage 37.22% 37.22% -0.01%
==========================================
Files 2318 2318
Lines 124561 124564 +3
Branches 16917 16917
==========================================
Hits 46371 46371
- Misses 74440 74443 +3
Partials 3750 3750 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…er-volley-db-writes
| updateAction == ReaderPostServiceStarter.UpdateAction.REQUEST_REFRESH | ||
| ) { | ||
| ReaderTagTable.setTagLastUpdated(tag) | ||
| Thread { ReaderTagTable.setTagLastUpdated(tag) }.start() |
There was a problem hiding this comment.
This is a Kotlin file, why not use coroutines instead of threads?
| } | ||
| final ReaderBlog blogInfo = ReaderBlog.fromJson(jsonObject); | ||
| // Move the INSERT OR REPLACE off the main thread; callers expect onResult on main. | ||
| new Thread(() -> { |
There was a problem hiding this comment.
⛏️ I'm not sure using raw threads is the best approach here. I asked Claude about using a handler and a Loop:
HandlerThread + Handler — a long-lived background thread with its own Looper. Arguably better here because all these DB writes serialize through SQLite anyway, so a single HandlerThread would naturally queue them and avoid spawning N short-lived threads on response bursts. The cost is managing its lifecycle (or making it static and never tearing it down).
Both call sites already have applicationScope + ioDispatcher injected, and the newer functions in this file use the same coroutine pattern. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the per-response new Thread() in handleUpdateBlogInfoResponse with a static single-thread ExecutorService + static main-thread Handler. DB writes now serialize through one shared worker, avoiding thread churn on response bursts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
You're correct on both counts. I've updated this PR with your suggestions. |
adalpari
left a comment
adalpari
left a comment
There was a problem hiding this comment.
Thank you for the changes, LGTM!
4 participants
Summary
Fixes a cluster of Reader ANRs where Volley response callbacks ran a SQLite
compileStatement+executeon the main thread:ReaderTagTable.updateTagLastUpdated(17 users)ReaderBlogTable.addOrUpdateBlog(13 users)Volley's
ResponseDeliveryRunnableruns callbacks on the main thread. When the SQLite connection pool is busy with concurrent writes (Reader sync), parking onacquireConnectionfrom the main thread tipped these calls over the 5s ANR threshold.Changes
ReaderPostRepository.kt— bothsetTagLastUpdated(tag)call sites (requestPostsWithTag,requestPostsForLatestStream) now run on a backgroundThread, matching the existing pattern inhandleUpdatePostsResponse.ReaderBlogActions.java—handleUpdateBlogInfoResponsenow performsaddOrUpdateBlogon a backgroundThreadand postsUpdateBlogInfoListener.onResultback on the main thread viaHandler(Looper.getMainLooper()). The two existing callers (ReaderSiteHeaderView,ReaderFetchSiteUseCase) expect the result on main —ReaderSiteHeaderViewcallsisAttachedToWindow()and updates views in the callback.Test plan