[fix](mtmv) reuse Gson instance in MTMVTask.getTvfInfo to avoid Master FE CPU spike - #66792
Open
xy720 wants to merge 1 commit into
Open
[fix](mtmv) reuse Gson instance in MTMVTask.getTvfInfo to avoid Master FE CPU spike#66792xy720 wants to merge 1 commit into
xy720 wants to merge 1 commit into
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Member
Author
|
run buildall |
Member
Author
|
/review |
xy720
changed the title
16 tasks
Contributor
TPC-H: Total hot run time: 17593 ms |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Review opinion: looks good. I found no blocking issue and no inline issue to raise.
Critical checkpoint conclusions:
- Goal and proof:
tasks('type'='mv')callsMTMVTask.getTvfInfoonce per visible task. This change removes up to threeGsonconstructions per emitted task row and reuses the resulting reflective adapters, directly addressing the reported Master FE CPU hotspot. ExistingMTMVTaskTestandtest_task_mtmv.groovyexercisegetTvfInfo/ the MV tasks TVF, although this PR adds no targeted concurrency test or performance benchmark. - Scope and clarity: the patch is minimal and focused: one private static final serializer plus the three replacements in the sole MTMV task-row producer. The default
new Gson()configuration and each existing null branch remain unchanged, so observable JSON formatting and null behavior are preserved. - Concurrency: concurrent metadata RPC workers can share this instance. Gson 2.10.1 explicitly supports cross-thread instance reuse; its implementation uses thread-local in-progress adapter construction, a concurrent adapter cache, and a separate writer for every
toJsoncall. The serializers reached here are stateless.completedPartitionsis aCopyOnWriteArrayList, and the change introduces no new task-state race, lock, lock-order, or deadlock risk. See the Gson 2.10.1 implementation. - Lifecycle and memory: JVM class initialization safely publishes the static final instance. The adapter cache is bounded by a small fixed population of context/list/string/enum types, retains no task instances, and has no static-initialization cycle.
- Configuration, compatibility, and parallel paths: no config, schema, thrift, storage, persisted-state, function-symbol, or FE/BE variable change is involved, so rolling-upgrade behavior is unchanged. All three plain-Gson allocations in this MTMV hot path were replaced; other task types use separate row producers and are not part of this workload.
- Conditions, errors, and observability: the existing null conditions and error behavior are untouched. The patch creates no new failure path, distributed operation, or state transition requiring extra logging, metrics, or error context.
- Transactions and data writes: not applicable; the change only serializes task metadata for display and does not alter transactions, edit logs, failover behavior, or data writes.
- Tests and results: no test or result file is changed. Existing coverage reaches the affected API, but does not quantify allocation/CPU improvement. I did not run builds or tests because this runner's review instructions prohibit them; correctness was checked from the full call path and Gson 2.10.1's concurrency contract and implementation.
- Performance and remaining issues: repeated constructor/factory-list allocation and repeated adapter discovery are removed without adding synchronization or per-row work. I found no additional correctness, lifecycle, compatibility, performance, or coverage defect requiring a review comment.
User focus: no additional review focus was provided; the complete PR was reviewed.
Contributor
TPC-DS: Total hot run time: 85966 ms |
Contributor
ClickBench: Total hot run time: 14.81 s |
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
2 participants
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.
What problem does this PR solve?
Problem Summary:
Related PR: (#28566)
防止在集群有大量物化视图刷新任务的场景下，
tasks('type'='mv')TVF 调用 将 Master Cpu 打满。复用Gson对象可以防止getAdapter的重复调用
200个刷新任务（包括pending）+ 30s 一次的
tasks('type'='mv')TVF 调用，32cFE效果就可能达到如下Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)