Skip to content

test(profiling): regression test for #16657#16660

Closed
taegyunkim wants to merge 2 commits into
mainfrom
taegyunkim/regression-16657
Closed

test(profiling): regression test for #16657#16660
taegyunkim wants to merge 2 commits into
mainfrom
taegyunkim/regression-16657

Conversation

@taegyunkim

@taegyunkim taegyunkim commented Feb 25, 2026

Copy link
Copy Markdown
Contributor

Description

image

Testing

Risks

Additional Notes

@taegyunkim taegyunkim added changelog/no-changelog A changelog entry is not required for this PR. Profiling Continous Profling labels Feb 25, 2026
@cit-pr-commenter-54b7da

Copy link
Copy Markdown

Codeowners resolved as

tests/profiling/collector/test_stack.py                                 @DataDog/profiling-python

@datadog-datadog-prod-us1

datadog-datadog-prod-us1 Bot commented Feb 25, 2026

Copy link
Copy Markdown
Contributor

⚠️ Tests

Fix all issues with BitsAI or with Cursor

⚠️ Warnings

🧪 6 Tests failed

test_gevent_greenlet_switch_not_blocked_by_profiler from test_stack.py (Datadog) (Fix with Cursor)
Expected status 0, got 1.
=== Captured STDOUT ===
=== End of captured STDOUT ===
=== Captured STDERR ===
Traceback (most recent call last):
  File "tests/profiling/collector/test_stack.py", line 1060, in <module>
    assert ratio < MAX_SCALING_RATIO, (
           ^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: Greenlet switch time scaled 42.9x when adding 2000 tracked greenlets (low=0.0287s, high=1.2324s). This indicates greenlet_info_map_lock contention during sampling.
=== End of captured STDERR ===
test_gevent_greenlet_switch_not_blocked_by_profiler from test_stack.py (Datadog) (Fix with Cursor)
Expected status 0, got 1.
=== Captured STDOUT ===
=== End of captured STDOUT ===
=== Captured STDERR ===
Traceback (most recent call last):
  File "tests/profiling/collector/test_stack.py", line 1060, in <module>
    assert ratio < MAX_SCALING_RATIO, (
           ^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: Greenlet switch time scaled 44.5x when adding 2000 tracked greenlets (low=0.0297s, high=1.3230s). This indicates greenlet_info_map_lock contention during sampling.
=== End of captured STDERR ===
test_gevent_greenlet_switch_not_blocked_by_profiler from test_stack.py (Datadog) (Fix with Cursor)
Expected status 0, got 1.
=== Captured STDOUT ===
=== End of captured STDOUT ===
=== Captured STDERR ===
Traceback (most recent call last):
  File "tests/profiling/collector/test_stack.py", line 1060, in <module>
    assert ratio < MAX_SCALING_RATIO, (
           ^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: Greenlet switch time scaled 44.2x when adding 2000 tracked greenlets (low=0.0285s, high=1.2602s). This indicates greenlet_info_map_lock contention during sampling.
=== End of captured STDERR ===
View all

ℹ️ Info

❄️ No new flaky tests detected

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 5e9940a | Docs | Datadog PR Page | Was this helpful? React with 👍/👎 or give us feedback!

gh-worker-dd-mergequeue-cf854d Bot pushed a commit that referenced this pull request Mar 6, 2026
…16657)

## Summary
- Split `unwind_greenlets()` into two phases to reduce lock hold time on `greenlet_info_map_lock`
  - **Phase 1 (under lock):** snapshot greenlet IDs, names, frame pointers, and parent chains into lightweight `GreenletSnapshot` structs
  - **Phase 2 (lock released):** perform the expensive stack unwinding (`process_vm_readv` / `copy_type`) outside the lock
- Add `GreenletSnapshot` struct in `greenlets.h` to hold the snapshotted state

## Motivation
Before this fix, `unwind_greenlets()` held `greenlet_info_map_lock` for the entire duration of stack unwinding across **all** tracked greenlets. Since every greenlet switch calls `update_greenlet_frame()` under the same lock, applications with many tracked greenlets (e.g. gunicorn + gevent + psycopg2) experienced severe lock contention — greenlet switches stalled waiting for the sampler, leading to connection pool exhaustion and request timeouts.

### Why asyncio doesn't have this problem
Asyncio task unwinding (`unwind_tasks`) doesn't suffer from this because it reads task state directly from CPython internals (linked lists in 3.14+, or `WeakSet` in older versions) via `copy_type()` — no profiler-owned lock is involved during discovery or unwinding. The only profiler lock (`task_link_map_lock`) is held briefly for link-map cleanup, then released before the expensive coroutine stack unwinding begins.

Greenlets lack CPython-native tracking, so the profiler must maintain its own `greenlet_info_map` with frame pointers updated on every switch via `update_greenlet_frame()`. This creates a lock shared between the sampler thread (reading) and the application thread (writing on every switch) — the exact contention pattern this fix addresses by making the read-side (Phase 1 snapshot) fast.

### Regression test confirms the issue on main
The regression test (`test_gevent_greenlet_switch_not_blocked_by_profiler`) was cherry-picked to main in [#16660](#16660) and fails there, confirming this branch fixes the contention.

## JIRA
[SCP-1039](https://datadoghq.atlassian.net/browse/SCP-1039)

## Testing
- Added `test_gevent_greenlet_switch_not_blocked_by_profiler` regression test that:
  - Measures greenlet-switch wall time with 0 vs 2000 idle tracked greenlets (each with 50-deep stacks) while the profiler samples at 5ms intervals
  - Asserts the ratio stays below 3x to catch lock-contention regressions
  - Uses `gevent.joinall` with a 30s timeout and `try/finally` cleanup to prevent CI hangs

## Risks
- Snapshotted frame pointers may become stale between Phase 1 and Phase 2, but `unwind_frame()` already handles invalid pointers gracefully via `copy_type()` which returns non-zero on failure
- No change to public API

🤖 Generated with [Claude Code](https://claude.com/claude-code)

[SCP-1039]: https://datadoghq.atlassian.net/browse/SCP-1039?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

Co-authored-by: taegyun.kim <[email protected]>
@taegyunkim taegyunkim closed this Mar 6, 2026
@taegyunkim
taegyunkim deleted the taegyunkim/regression-16657 branch March 6, 2026 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/no-changelog A changelog entry is not required for this PR. Profiling Continous Profling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant