<fix>[query]: ZSTAC-80742 limit API query concurrency#3975
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Walkthrough
Changes查询处理逻辑提取
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
search/src/main/java/org/zstack/query/QueryFacadeImpl.java (1)
352-362:⚠️ Potential issue | 🟠 Major | ⚡ Quick win并发查询路径下的
HashMap缓存存在线程安全风险
handle(APIQueryMessage)已改为并发SyncTask执行（getSyncLevel()> 1），但 Line 352-362/Line 369 仍会并发访问autoQueryMap、replySetter（当前是HashMap）。这会引入竞态与可见性问题，建议改为并发容器。🔧 建议修改
@@ import java.util.*; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeUnit; @@ - private Map<Class, Method> replySetter = new HashMap<>(); - private Map<Class, AutoQuery> autoQueryMap = new HashMap<>(); + private final Map<Class, Method> replySetter = new ConcurrentHashMap<>(); + private final Map<Class, AutoQuery> autoQueryMap = new ConcurrentHashMap<>();Also applies to: 369-370
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@search/src/main/java/org/zstack/query/QueryFacadeImpl.java` around lines 352 - 362, The code accesses shared maps (autoQueryMap and replySetter) concurrently from handle(APIQueryMessage)/doQuery causing race/visibility bugs; replace their HashMap declarations with thread-safe ConcurrentHashMap and update usages to atomic helpers (e.g., use computeIfAbsent for autoQueryMap to load/put the AutoQuery annotation atomically) so gets/puts are safe under concurrent SyncTask execution; ensure any put-then-get patterns on replySetter are converted to atomic operations or synchronized blocks if necessary.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@search/src/main/java/org/zstack/query/QueryFacadeImpl.java`:
- Around line 352-362: The code accesses shared maps (autoQueryMap and
replySetter) concurrently from handle(APIQueryMessage)/doQuery causing
race/visibility bugs; replace their HashMap declarations with thread-safe
ConcurrentHashMap and update usages to atomic helpers (e.g., use computeIfAbsent
for autoQueryMap to load/put the AutoQuery annotation atomically) so gets/puts
are safe under concurrent SyncTask execution; ensure any put-then-get patterns
on replySetter are converted to atomic operations or synchronized blocks if
necessary.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)
Review profile: CHILL
Plan: Pro
Run ID: 081c0bc1-1a81-4b2a-8598-491f887515db
📒 Files selected for processing (1)
search/src/main/java/org/zstack/query/QueryFacadeImpl.java
Route AutoQuery handling through the query sync queue so bursty GraphQL refreshes cannot run unlimited APIQueryMessage requests concurrently. Resolves: ZSTAC-80742 Change-Id: If2c052e539526cd0511e037d1718838bdb65a7a3
MatheMatrix
force-pushed
the
sync/shan.wu/fix/5.5.22/ZSTAC-80742
branch
from
0374da1 to
bb01702
Compare
|
Comment from gitlab: 自上次添加REVIEWED标签(2026-05-17 17:43:28.000Z)后, 有新的COMMIT更新(2026-05-17 19:16:23.853Z), 所以移除了REVIEWED标签 |
|
Comment from gitlab: 检测到REVIEWED标签添加者(shan.wu)不属于Maintainers:[ye.tian wenhao.zhang xin.zhang yaohua.wu lock-files yuerong.su gitlab qun.li jin.ma shixin.ruan lei.liu zstackio wei.wang ye.zou zhangjianjun], 所以移除了REVIEWED标签 |
3 participants
Summary
Limit backend AutoQuery concurrency to prevent bursty GraphQL VM list refreshes from overwhelming the management node.
Root Cause
Refreshing the VM list can fan out into many GraphQL-backed
APIQueryMessagerequests such asGuestToolsStateandModelServiceInstancequeries. ZQL and BatchQuery were already routed through the query sync queue, but ordinary AutoQuery handling executed directly, allowing large bursts to run concurrently and overload the management node.Changes
APIQueryMessagehandling through the existing query sync queue.Testing
JAVA_HOME=$(/usr/libexec/java_home -v 1.8) mvn -pl search -am -DskipTests compileResolves: ZSTAC-80742
sync from gitlab !9866