*: Fix incorrect mapped allocation per thread metric (#18126) by ti-chi-bot · Pull Request #18564 · tikv/tikv

ti-chi-bot · 2025-06-18T04:06:28Z

This is an automated cherry-pick of #18126

What is changed and how it works?

Issue Number: Close #18125

What's Changed:

Fix incorrect mapped allocation per thread metric

Not all thread builders are hooked by `thread_allocate_exclusive_arena`, so some threads are using shared arena, causing incorrect per thread allocation.

Related changes

PR to update pingcap/docs/pingcap/docs-cn:
Need to cherry-pick to the release branch

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Release note

Fix incorrect mapped allocation per thread metric

ti-chi-bot · 2025-06-18T04:06:31Z

@hhwyt This PR has conflicts, I have hold it.
Please resolve them or ask others to resolve them, then comment /unhold to remove the hold label.

hhwyt · 2025-06-19T07:34:27Z

Here is the test result: #18563 (comment).

hhwyt · 2025-06-19T07:37:36Z

Once #18563 is merged, I'll rebase the first commit from master.

Connor1996

LGTM

zhangjinpeng87 · 2025-06-24T02:41:25Z

bugbot run

cursor

Bug: Metric Descriptor Missing in Collector

The arena_count metric is initialized and collected, but its descriptor is not added to the AllocStatsCollector's descs field. This prevents proper registration of the metric with Prometheus.

components/tikv_util/src/metrics/allocator_metrics.rs#L51-L60

tikv/components/tikv_util/src/metrics/allocator_metrics.rs

Lines 51 to 60 in 3341338

    
           Ok(AllocStatsCollector { 
        
               descs: [&stats, &thread_stats, &allocation] 
        
                   .iter() 
        
                   .flat_map(|m| m.desc().into_iter().cloned()) 
        
                   .collect(), 
        
               memory_stats: stats, 
        
               thread_stats, 
        
               allocation, 
        
               arena_count, 
        
           })

Fix in Cursor

Bug: Thread Panic on Jemalloc Failure

The thread_allocate_exclusive_arena().unwrap() call can cause a thread panic if jemalloc operations (e.g., arena creation) fail. This unhandled panic, particularly in thread pool contexts, can prevent threads from starting or disrupt the entire pool. Graceful error handling (logging and continuing) is recommended, as thread startup failure is more disruptive than missing allocation metrics.

components/server/src/server2.rs#L307-L308

tikv/components/server/src/server2.rs

Lines 307 to 308 in 3341338

    
           unsafe { add_thread_memory_accessor() }; 
        
           thread_allocate_exclusive_arena().unwrap();

components/server/src/server.rs#L315-L316

tikv/components/server/src/server.rs

Lines 315 to 316 in 3341338

    
           unsafe { add_thread_memory_accessor() }; 
        
           thread_allocate_exclusive_arena().unwrap();

components/tikv_util/src/yatp_pool/mod.rs#L189-L190

tikv/components/tikv_util/src/yatp_pool/mod.rs

Lines 189 to 190 in 3341338

    
           tikv_alloc::add_thread_memory_accessor(); 
        
           tikv_alloc::thread_allocate_exclusive_arena().unwrap();

components/tikv_util/src/sys/thread.rs#L434-L435

tikv/components/tikv_util/src/sys/thread.rs

Lines 434 to 435 in 3341338

    
           unsafe { add_thread_memory_accessor() }; 
        
           thread_allocate_exclusive_arena().unwrap();

components/tikv_util/src/sys/thread.rs#L458-L459

tikv/components/tikv_util/src/sys/thread.rs

Lines 458 to 459 in 3341338

    
           unsafe { add_thread_memory_accessor() }; 
        
           thread_allocate_exclusive_arena().unwrap();

components/tikv_util/src/sys/thread.rs#L483-L484

tikv/components/tikv_util/src/sys/thread.rs

Lines 483 to 484 in 3341338

    
           unsafe { add_thread_memory_accessor() }; 
        
           thread_allocate_exclusive_arena().unwrap();

Fix in Cursor

BugBot free trial expires on July 22, 2025
You have used $0.00 of your $10.00 spend limit so far. Manage your spend limit in the Cursor dashboard.

Was this report helpful? Give feedback by reacting with 👍 or 👎

hhwyt · 2025-06-24T03:30:00Z

@zhangjinpeng87

Bug: Metric Descriptor Missing in Collector

This is true but this does not affect Grafana Dashboard display, so can be ignored.

Bug: Thread Panic on Jemalloc Failure

This is true. While the error-handling can be more graceful as the arena allocation failure is a non-core logic should not cause TiKV panic, such failures are rare in practice, so I think it's ok to keep current implementation.

ti-chi-bot · 2025-07-10T02:41:06Z

[LGTM Timeline notifier]

Timeline:

2025-06-19 08:21:08.152141632 +0000 UTC m=+346320.875320614: ☑️ agreed by Connor1996.
2025-07-10 02:41:05.028607477 +0000 UTC m=+2140317.751786453: ☑️ agreed by LykxSassinator.

hhwyt · 2025-07-10T02:56:41Z

/hold

glorv · 2025-07-11T04:44:06Z

/retest

hhwyt · 2025-07-11T06:24:28Z

/hold

close tikv#18125 Fix incorrect mapped allocation per thread metric Not all thread builders are hooked by `thread_allocate_exclusive_arena`, so some threads are using shared arena, causing incorrect per thread allocation. Signed-off-by: Connor1996 <zbk602423539@gmail.com> (cherry picked from commit 18f4419) Signed-off-by: hhwyt <hhwyt1@gmail.com>

ti-chi-bot · 2025-07-11T07:44:46Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cfzjywxk, Connor1996, LykxSassinator

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [Connor1996,LykxSassinator,cfzjywxk]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

hhwyt · 2025-07-11T07:44:54Z

/unhold

ti-chi-bot mentioned this pull request Jun 18, 2025

*: Fix incorrect mapped allocation per thread metric #18126

Merged

9 tasks

ti-chi-bot assigned hhwyt Jun 18, 2025

ti-chi-bot bot added the do-not-merge/cherry-pick-not-approved label Jun 18, 2025

hhwyt requested review from Connor1996, glorv and overvenus June 19, 2025 07:34

hhwyt force-pushed the cherry-pick-18126-to-release-7.5 branch from b11246b to 2107add Compare June 19, 2025 07:35

ti-chi-bot bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 19, 2025

hhwyt force-pushed the cherry-pick-18126-to-release-7.5 branch from 2107add to 1385073 Compare June 19, 2025 07:36

hhwyt force-pushed the cherry-pick-18126-to-release-7.5 branch 2 times, most recently from 6d43d21 to 3341338 Compare June 19, 2025 07:42

hhwyt requested a review from cfzjywxk June 19, 2025 07:42

hhwyt mentioned this pull request Jun 19, 2025

tikv_alloc: Add per thread memory usage (#16255) #18563

Merged

9 tasks

Connor1996 approved these changes Jun 19, 2025

View reviewed changes

ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Jun 19, 2025

cursor bot reviewed Jun 24, 2025

View reviewed changes

LykxSassinator approved these changes Jul 10, 2025

View reviewed changes

ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jul 10, 2025

ti-chi-bot bot added cherry-pick-approved Cherry pick PR approved by release team. and removed do-not-merge/cherry-pick-not-approved labels Jul 10, 2025

cfzjywxk approved these changes Jul 11, 2025

View reviewed changes

ti-chi-bot bot added the approved label Jul 11, 2025

hhwyt force-pushed the cherry-pick-18126-to-release-7.5 branch from 3341338 to 34ac9d6 Compare July 11, 2025 07:44

ti-chi-bot bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Jul 11, 2025

ti-chi-bot bot merged commit 664fb07 into tikv:release-7.5 Jul 11, 2025
4 checks passed

	Ok(AllocStatsCollector {
	descs: [&stats, &thread_stats, &allocation]
	.iter()
	.flat_map(\|m\| m.desc().into_iter().cloned())
	.collect(),
	memory_stats: stats,
	thread_stats,
	allocation,
	arena_count,
	})

	unsafe { add_thread_memory_accessor() };
	thread_allocate_exclusive_arena().unwrap();

	tikv_alloc::add_thread_memory_accessor();
	tikv_alloc::thread_allocate_exclusive_arena().unwrap();

Conversation

ti-chi-bot commented Jun 18, 2025

What is changed and how it works?

Related changes

Check List

Release note

Uh oh!

ti-chi-bot commented Jun 18, 2025

Uh oh!

hhwyt commented Jun 19, 2025

Uh oh!

hhwyt commented Jun 19, 2025

Uh oh!

Connor1996 left a comment

Choose a reason for hiding this comment

Uh oh!

zhangjinpeng87 commented Jun 24, 2025

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Bug: Metric Descriptor Missing in Collector

Bug: Thread Panic on Jemalloc Failure

Uh oh!

hhwyt commented Jun 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ti-chi-bot bot commented Jul 10, 2025

[LGTM Timeline notifier]

Uh oh!

hhwyt commented Jul 10, 2025

Uh oh!

glorv commented Jul 11, 2025

Uh oh!

hhwyt commented Jul 11, 2025

Uh oh!

ti-chi-bot bot commented Jul 11, 2025

Uh oh!

hhwyt commented Jul 11, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

hhwyt commented Jun 24, 2025 •

edited

Loading