fix: stop preloading entire preferences table into memory - #9747
Merged
Soulter merged 2 commits intoAug 19, 2026
Conversation
PR AstrBotDevs#9649 fixed an event-loop deadlock by preloading every row of the preferences table into an in-memory mirror at startup. The table can hold gigabytes of plugin KV data, so this can OOM the process. - initialize() no longer performs an unfiltered get_preferences(); the in-memory cache becomes a write overlay for read-after-write visibility only - get_async() falls back to an async point query on overlay miss and never backfills the overlay - deprecated sync get() falls back to a point query through a dedicated synchronous SQLite connection (the new SQLiteDatabase.get_preference_sync), so it never blocks on or deadlocks against the async pool; failures warn and return the default instead of raising - sync range_get() now only covers values written by this process (no internal callers remain); full scans stay available via range_get_async() - FIFO write queue semantics are unchanged
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
SQLiteDatabase.get_preference_sync, a newsqlite3connection is created and torn down on every call; if sync reads are used frequently, consider reusing a shared connection or a small sync pool to avoid repeated connection setup overhead. - The sync
get()method now does a point query on every cache miss without caching the result; if the same keys are read repeatedly via the deprecated sync API, an optional small LRU or per-key overlay for sync-only reads could significantly reduce repeated DB hits while still avoiding full-table preload. get_preference_syncimportssqlite3inside the function and assumesrow[0]is either a JSON string or native object; if there are existing rows with bytes or other types, you may want to normalize/validate the type and move the import to module scope for efficiency.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `SQLiteDatabase.get_preference_sync`, a new `sqlite3` connection is created and torn down on every call; if sync reads are used frequently, consider reusing a shared connection or a small sync pool to avoid repeated connection setup overhead.
- The sync `get()` method now does a point query on every cache miss without caching the result; if the same keys are read repeatedly via the deprecated sync API, an optional small LRU or per-key overlay for sync-only reads could significantly reduce repeated DB hits while still avoiding full-table preload.
- `get_preference_sync` imports `sqlite3` inside the function and assumes `row[0]` is either a JSON string or native object; if there are existing rows with bytes or other types, you may want to normalize/validate the type and move the import to module scope for efficiency.
## Individual Comments
### Comment 1
<location path="astrbot/core/utils/shared_preferences.py" line_range="446" />
<code_context>
raise ValueError(
"scope_id and key cannot be None when getting a specific preference.",
)
+ resolved_scope = scope or "unknown"
+ resolved_scope_id = scope_id or "unknown"
with self._cache_lock:
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting overlay lookup, synchronous DB fallback, and pending-write replay into small helpers to simplify `get()`/`get_async()` and initialization logic.
You can keep the new behavior but reduce complexity by pushing the sync/async fallback logic into small helpers and reusing the overlay lookup. That keeps `get()` and `get_async()` focused on orchestration.
**1. Extract overlay lookup into a helper**
```python
def _get_from_overlay(
self,
scope: str,
scope_id: str,
key: str,
default: _VT,
) -> _VT | _MISSING:
with self._cache_lock:
value = self._cache.get((scope, scope_id, key), _MISSING)
if value is _MISSING:
return _MISSING
# Preserve existing default semantics for None
return default if value is None else deepcopy(value)
```
Then:
```python
async def get_async(
self,
scope: str,
scope_id: str,
key: str,
default: _VT = None,
) -> _VT:
await self.initialize()
if scope_id is None or key is None:
return default
overlay_value = self._get_from_overlay(scope, scope_id, key, default)
if overlay_value is not _MISSING:
return overlay_value
preference = await self.db_helper.get_preference(scope, scope_id, key)
if preference is None:
return default
return deepcopy(preference.value["val"])
```
**2. Encapsulate sync DB fallback + warning**
```python
def _get_from_db_sync(
self,
scope: str,
scope_id: str,
key: str,
default: _VT,
) -> _VT:
get_sync = getattr(self.db_helper, "get_preference_sync", None)
if get_sync is None:
if not self._warned_sync_read_unsupported:
self._warned_sync_read_unsupported = True
logger.warning(
"SharedPreferences sync get() is not supported by database "
"backend %s; returning the default. Use get_async() instead.",
type(self.db_helper).__name__,
)
return default
try:
stored = get_sync(scope, scope_id, key)
except Exception as exc:
logger.warning(
"SharedPreferences sync get() failed for %s/%s/%s: %s",
scope,
scope_id,
key,
exc,
)
return default
if stored is None:
return default
value = stored.get("val")
return default if value is None else deepcopy(value)
```
Then `get()` becomes:
```python
@deprecated(version="4.0.0", reason="Use get_async() instead.")
def get(
self,
key: str,
default: _VT = None,
scope: str = "unknown",
scope_id: str | None = "",
) -> _VT:
if scope_id == "":
scope_id = "unknown"
if scope_id is None or key is None:
raise ValueError("scope_id and key cannot be None when getting a specific preference.")
resolved_scope = scope or "unknown"
resolved_scope_id = scope_id or "unknown"
overlay_value = self._get_from_overlay(resolved_scope, resolved_scope_id, key, default)
if overlay_value is not _MISSING:
return overlay_value
return self._get_from_db_sync(resolved_scope, resolved_scope_id, key, default)
```
This:
- Removes logging/feature-detection/exception-handling from the main `get()` path.
- Aligns the overlay-then-DB pattern between sync and async reads.
- Keeps `_warned_sync_read_unsupported` scoped to one helper, instead of threading its logic through `get()`.
**3. Clarify initialization semantics with a helper (optional but low-impact)**
To make `_cache_initialized`’s new meaning clearer without renaming everywhere, you can document and centralize the “replay pending writes and mark ready” step:
```python
def _warm_overlay_from_pending(self) -> None:
with self._cache_lock:
pending_writes = list(self._pending_writes)
self._pending_writes.clear()
for operation in pending_writes:
self._apply_cache_operation(operation)
self._cache_initialized = True
self._initializing = False
```
Then in `initialize()`:
```python
with self._cache_lock:
self._warm_overlay_from_pending()
```
This keeps the OOM-safe behavior but makes the initialization state machine easier to follow.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
BegoniaHe
pushed a commit
to Xero-Team/AstrBot
that referenced
this pull request
Aug 20, 2026
…s#9747) Upstream-Commit: b8cd04e Upstream-Author: Soulter <37870767+Soulter@users.noreply.github.com> Upstream-PR: AstrBotDevs#9747 Sync-Disposition: adapt Fork-Adaptation: Keep the async-only PreferenceStore and terminate() lifecycle; stop the startup full-table load and use the write overlay plus async point queries. Do not restore deprecated sync get()/range_get() or add sqlite3 sync helpers. Tested: uv run pytest tests/unit/test_shared_preferences.py -q
BegoniaHe
pushed a commit
to Xero-Team/AstrBot
that referenced
this pull request
Aug 20, 2026
Upstream-Commit: 4fe2975 Upstream-Author: Soulter <37870767+Soulter@users.noreply.github.com> Upstream-PR: AstrBotDevs#9749 Sync-Disposition: adapt Fork-Adaptation: Bump synchronized version files and finalize the existing fork changelog, including AstrBotDevs#9747 and previously absorbed 4.27.4 work. Do not copy upstream-only entries, skipped commits, or upstream compare links. Tested: node node_modules/prettier/bin/prettier.cjs --write changelogs/v4.27.4.md
BegoniaHe
added a commit
to Xero-Team/AstrBot
that referenced
this pull request
Aug 20, 2026
Record adapted AstrBotDevs#9747 and AstrBotDevs#9749 through 4fe2975.
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.
#9649 为修复同步 SharedPreferences API 跨事件循环
.result()导致的死锁,在启动时无过滤调用get_preferences()将 preferences 全表物化进内存并常驻缓存。但 preferences 表可能存储数 GB 级插件 KV 数据(scope="plugin"),全量预加载 + deepcopy 会让进程在启动时直接 OOM,峰值内存可达表大小的数倍。PR #9649 preloaded the entire preferences table into memory at startup to fix an event-loop deadlock. Since the table can hold gigabytes of plugin KV data, this can OOM the process on startup.
Modifications / 改动点
astrbot/core/utils/shared_preferences.pyinitialize()不再全表预加载,仅绑定事件循环并重放 pending writes;_cache从"全量镜像"降级为"本进程写入 overlay",只保证 read-after-write。get_async():overlay 未命中时await get_preference(...)单点查库,不回填缓存,内存占用有界。get():overlay 未命中时通过独立同步 SQLite 连接点查回源,不经过 async pool,因此不会阻塞/死锁事件循环;后端不支持或查询失败时告警并返回 default,不抛异常,兼容存量插件。range_get():注明现在仅覆盖本进程写入的值(全仓已无内部调用方);完整范围查询仍可用range_get_async()。flush()、提交顺序)完全未改动。astrbot/core/db/sqlite.pySQLiteDatabase.get_preference_sync():基于 stdlibsqlite3独立连接的唯一索引点查(WAL 模式下可与异步写入并发读取)。tests/unit/test_shared_preferences.py同步、不阻塞、能读取任意历史值三者不可兼得;本 PR 选择让 deprecated 同步读退化为短暂 SQLite 点查(微秒~毫秒级),换取无死锁 + 无全量加载。核心链路与插件 API(
PluginKVStoreMixin)本就已全部走 async,不受影响。Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
/ 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。
👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
/ 我的更改经过了良好的测试,并已在上方提供了"验证步骤"和"运行截图"。
🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in
requirements.txtandpyproject.toml./ 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到
requirements.txt和pyproject.toml文件相应位置。😮 My changes do not introduce malicious code.
/ 我的更改没有引入恶意代码。
Summary by Sourcery
Replace full preferences-table caching with bounded process-local overlays and on-demand database reads to prevent startup memory exhaustion without reintroducing synchronous API deadlocks.
Bug Fixes:
Enhancements:
Tests: