Skip to content

[2.8] [MOD-12694] [MOD-12069] Add active_worker_threads metric#7645

Merged
meiravgri merged 5 commits into2.8from
backport-meiravg_active_threads-2.8
Dec 8, 2025
Merged

[2.8] [MOD-12694] [MOD-12069] Add active_worker_threads metric#7645
meiravgri merged 5 commits into2.8from
backport-meiravg_active_threads-2.8

Conversation

@meiravgri
Copy link
Collaborator

@meiravgri meiravgri commented Dec 4, 2025

Backport #7538 to the 2.8 branch with necessary adaptations

Key Differences from Master Version

Conditional MT_BUILD Support (src/info/global_stats.c)

Test Scope Reduction (tests/pytests/test_info_modules.py)

  • Implemented only for standalone mode (test_active_worker_threads()) because the query debug pause mechanism is not supported on cluster in 2.8.

Note

Add active_worker_threads to multi-threading stats and expose it via INFO MODULES, with supporting tests and a small threading helper.

  • Metrics/Stats:
    • Add active_worker_threads to MultiThreadingStats in src/info/global_stats.h.
    • Compute it in GlobalStats_GetMultiThreadingStats() (guarded by MT_BUILD, using workersThreadPool_WorkingThreadCount()), add include util/workers.h.
  • INFO output:
    • Expose search_active_worker_threads in AddToInfo_MultiThreading() in src/info/info_redis.c.
  • Tests:
    • New test_active_worker_threads() in tests/pytests/test_info_modules.py validating metric during paused queries and after drain.
    • Add call_and_store() helper in tests/pytests/common.py; use it in tests/pytests/test_debug_commands.py for threading.
    • Minor imports added (e.g., threading).

Written by Cursor Bugbot for commit 5f3fc56. This will update automatically on new commits. Configure here.

@meiravgri meiravgri changed the title [MOD-12694] [MOD-12069] Add active_worker_threads metric (#7538) [2.8] [MOD-12694] [MOD-12069] Add active_worker_threads metric (#7538) Dec 4, 2025
@meiravgri meiravgri changed the title [2.8] [MOD-12694] [MOD-12069] Add active_worker_threads metric (#7538) [2.8] [MOD-12694] [MOD-12069] Add active_worker_threads metric Dec 7, 2025
@meiravgri meiravgri requested a review from lerman25 December 7, 2025 09:39
@meiravgri meiravgri enabled auto-merge December 7, 2025 09:40
@codecov
Copy link

codecov bot commented Dec 7, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.59%. Comparing base (c500ddb) to head (5f3fc56).
⚠️ Report is 2 commits behind head on 2.8.

Additional details and impacted files
@@            Coverage Diff             @@
##              2.8    #7645      +/-   ##
==========================================
- Coverage   87.61%   87.59%   -0.02%     
==========================================
  Files         203      203              
  Lines       35102    35119      +17     
==========================================
+ Hits        30755    30763       +8     
- Misses       4347     4356       +9     
Flag Coverage Δ
flow 81.98% <100.00%> (-0.23%) ⬇️
unit 42.32% <50.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

meiravgri added a commit that referenced this pull request Dec 7, 2025
Copy link
Collaborator

@lerman25 lerman25 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work - some comments

Comment on lines 114 to 122
#ifdef MT_BUILD
if (RSGlobalConfig.numWorkerThreads) {
stats.active_worker_threads = workersThreadPool_WorkingThreadCount();
} else {
stats.active_worker_threads = 0;
}
#else
stats.active_worker_threads = 0;
#endif // MT_BUILD
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WDYT?

Suggested change
#ifdef MT_BUILD
if (RSGlobalConfig.numWorkerThreads) {
stats.active_worker_threads = workersThreadPool_WorkingThreadCount();
} else {
stats.active_worker_threads = 0;
}
#else
stats.active_worker_threads = 0;
#endif // MT_BUILD
stats.active_worker_threads = 0;
#ifdef MT_BUILD
if (RSGlobalConfig.numWorkerThreads) {
stats.active_worker_threads = workersThreadPool_WorkingThreadCount();
}
#endif // MT_BUILD

Comment on lines 623 to 626
# NOTE: Currently query debug pause mechanism only supports pausing one query at a time, and is not supported on cluster.
def test_active_worker_threads():
if COORD or not MT_BUILD:
raise SkipTest()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not:

Suggested change
# NOTE: Currently query debug pause mechanism only supports pausing one query at a time, and is not supported on cluster.
def test_active_worker_threads():
if COORD or not MT_BUILD:
raise SkipTest()
# NOTE: Currently query debug pause mechanism only supports pausing one query at a time, and is not supported on cluster.
@skip(cluster=True)
def test_active_worker_threads():
if not MT_BUILD:
raise SkipTest('MT_BUILD is not set')

if COORD ... SkipTest() in not common in tests

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Originally, it was a helper function. just leftovers i forgot to change

@meiravgri meiravgri requested a review from lerman25 December 7, 2025 17:02
@meiravgri meiravgri added this pull request to the merge queue Dec 8, 2025
Merged via the queue into 2.8 with commit b24a354 Dec 8, 2025
13 checks passed
@meiravgri meiravgri deleted the backport-meiravg_active_threads-2.8 branch December 8, 2025 07:23
github-merge-queue bot pushed a commit that referenced this pull request Dec 10, 2025
* align to Add active_worker_threads metric #7645

* [2.8] [MOD-12694] [MOD-12069] Add active_coord_threads metric (#7623)

* backport 7546

* move idfed cpp

(cherry picked from commit a8ecd32)

* fix calling coord func from SA

* fix declartion

* clenaups
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants