perf: optimize compact progress query from O(m*n) to O(n+m)#35115
perf: optimize compact progress query from O(m*n) to O(n+m)#35115guanshengliang merged 9 commits into3.0from
Conversation
Replace per-vnode progress requests with dnode-level broadcast: - Add TDMT_DND_QUERY_COMPACT_PROGRESS message type - Add SDnodeQueryCompactProgressReq/Rsp structures and serialization - Refactor mndCompactSendProgressReq to broadcast to all dnodes - Add mndProcessDnodeCompactProgressRsp with hash-based O(n+m) lookup - Add vmProcessDnodeQueryCompactProgressReq to collect vnode progress - Register new message handler in mnode and vnode management queue This optimization reduces the number of RPC calls from O(m*n) to O(n) where m is the number of vnodes and n is the number of dnodes. Reference: PR #35093
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new mechanism for querying compaction progress at the dnode level, replacing the previous vnode-by-vnode query approach. It adds new message types (TDMT_DND_QUERY_COMPACT_PROGRESS), serialization logic, and handlers on both the dnode and mnode sides to efficiently collect and update progress information. Review feedback identifies a potential memory leak when RPC requests fail, suggests an optimization to skip processing when no vnode updates are present, and highlights a potential safety issue with string formatting during debug logging.
There was a problem hiding this comment.
Pull request overview
This PR optimizes compaction progress collection by replacing per-vnode progress RPCs with a single broadcast per dnode, reducing RPC fanout while keeping progress updates accurate and efficient.
Changes:
- Introduces
TDMT_DND_QUERY_COMPACT_PROGRESSrequest/response flow and corresponding message (de)serialization. - Refactors mnode progress polling to broadcast to all dnodes and processes replies using a hash-based lookup to avoid nested scans.
- Adds vnode-mgmt handling to aggregate per-vnode compact progress into a single dnode-level response.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| source/dnode/mnode/impl/src/mndCompact.c | Broadcasts progress queries to dnodes and processes aggregated dnode responses with hash-based matching. |
| source/dnode/mnode/impl/inc/mndCompact.h | Exposes new compact progress response handler (and adds a new DB-name API declaration). |
| source/dnode/mgmt/mgmt_vnode/src/vmWorker.c | Routes the new dnode compact-progress query message type to a vnode-mgmt handler. |
| source/dnode/mgmt/mgmt_vnode/src/vmHandle.c | Implements the dnode request handler that aggregates compact progress across local vnodes. |
| source/common/src/msg/tmsg.c | Adds serializers/deserializers for the new dnode compact-progress req/rsp. |
| include/common/tmsgdef.h | Registers the new message type. |
| include/common/tmsg.h | Declares new req/rsp structs and (de)serialization APIs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…duplicate message definition
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Replace per-vnode progress requests with dnode-level broadcast:
This optimization reduces the number of RPC calls from O(m*n) to O(n)
where m is the number of vnodes and n is the number of dnodes.
Reference: PR #35093
Description
Issue(s)
Checklist
Please check the items in the checklist if applicable.