gh-154194: Degrade frames in Tachyon instead of failing the sample#154195
gh-154194: Degrade frames in Tachyon instead of failing the sample#154195maurycy wants to merge 7 commits into
Conversation
|
I believe that the CI failures are caused by the fact that |
pablogsal
left a comment
pablogsal
left a comment
There was a problem hiding this comment.
I still need more time but some nits
maurycy
changed the title
| # Build coroutine stack string | ||
| frames = [frame for coro in task_info.coroutine_stack | ||
| for frame in coro.call_stack] | ||
| coro_stack = " -> ".join(_format_stack_entry(x).split(" ")[0] |
There was a problem hiding this comment.
This does not work for the <unknown function> sentinel.
| def _format_stack_entry(elem: str|FrameInfo) -> str: | ||
| if not isinstance(elem, str): |
There was a problem hiding this comment.
I believe that isinstance(elem, str) is dead since #135436. Perhaps we should clean this defensive code everywhere.
| # ─── indexing helpers ─────────────────────────────────────────── | ||
| def _format_stack_entry(elem: str|FrameInfo) -> str: | ||
| if not isinstance(elem, str): | ||
| if elem.location is None: |
There was a problem hiding this comment.
Regular collectors rely on these helpers:
cpython/Lib/profiling/sampling/collector.py
Lines 46 to 47 in 40f7fbf
cpython/Lib/profiling/sampling/collector.py
Lines 29 to 30 in 40f7fbf
| PyObject *func = NULL; | ||
| PyObject *file = NULL; | ||
| PyObject *linetable = NULL; | ||
| int code_metadata_incomplete = 0; |
There was a problem hiding this comment.
Perhaps parse_code_object() should return this information, and we should not store incomplete objects in the frame_cache?
Perhaps we should collect degradation stats? Important caveat is that we will likely have more types of degradation.
| "<GC>", | ||
| "<native>", | ||
| "<unknown function>", | ||
| "<unknown file>", | ||
| "<unreadable frame>", |
There was a problem hiding this comment.
Perhaps we could add something like is_synthetic to FrameInfo?
2 participants
As always, see #154194 for the context. Shortly: right now remote inspection fails the whole sample when it hits the limits (and we're really not sure what are the best limits) OR always when reading a part fails. This is too strict, wasteful and introduces scary bias, as observed in https://discuss.python.org/t/tachyon-97-error-rate/107619.
The PR introduces graceful degradation with the sentinels like
<unreadable frame>(funcname),<unknown function>(funcname),<unknown file>(filename) and the(len=N)suffix if we hit the cap. Also, attemps to better differentiate between transient failures in_Py_RemoteDebug_IsFatalReadError().Importantly: degraded frames are never cached, while the truncated strings are cached.
_remote_debugging: Degrade gracefully reads instead of failing the whole read completely #154194