Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

attach ignores --duration #701

Open
1 task done
jonashaag opened this issue Dec 1, 2024 · 5 comments
Open
1 task done

attach ignores --duration #701

jonashaag opened this issue Dec 1, 2024 · 5 comments
Labels
bug Something isn't working

Comments

@jonashaag
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

$ time memray attach --method lldb 22 --trace-python-allocators -o leak4 --duration 1000

real    0m9.631s
user    0m4.406s
sys     0m5.394s

$ time memray attach --method lldb 22 --trace-python-allocators -o leak5 --duration 500

real    0m9.535s
user    0m4.204s
sys     0m5.504s

It ignores duration.

Expected Behavior

No response

Steps To Reproduce

I don't know, I just attached to a running FastAPI/uvicorn process.

Memray Version

1.14.0

Python Version

3.12

Operating System

Linux

Anything else?

No response

@jonashaag jonashaag added the bug Something isn't working label Dec 1, 2024
@kushal12340
Copy link

I can also confirm that it does not work for me.

Version

1.13.4

Python Version

3.8

Steps To Reproduce

Try attaching to a running uvicorn process.

@kushal12340
Copy link

kushal12340 commented Dec 5, 2024

Ok, I believe I figured out the issue. The tracking works, but the CLI exits before the set duration is complete.

If you check the terminal running the uvicorn process, you'll see a message:

memray: Deactivating tracking: 60 seconds have elapsed

The captured file updates for the full duration, as seen in the modification time, and the flamegraph reflects the correct duration.

This behavior is confusing. It also means that if you press Ctrl+C to stop the memray process, the capture will still run in the background until the set duration ends.

@godlygeek
Copy link
Contributor

godlygeek commented Dec 5, 2024

Ok, I believe I figured out the issue. The tracking works, but the CLI exits before the set duration is complete.

That's right - time memray attach -o ... times how long it takes to attach a Memray tracker to the process, while --duration controls how long to track for once tracking has started. You're timing how long until tracking starts, rather than how long until tracking finishes.

It also means that if you press Ctrl+C to stop the memray process, the capture will still run in the background until the set duration ends.

What do you mean by "stop the memray process" - do you mean that you ctrl-c the call to memray attach? There's two possibilities there - either the process is interrupted before it installs the tracker (in which case no tracking will be performed) or it is interrupted after it has installed the tracker in the process it's attaching to (in which case that process will track allocations until the --duration has elapsed or a call to memray detach tells that process to stop tracking allocations). memray attach in -o mode exits as soon as the tracker has been injected.

This behavior is confusing.

The docs do say:

If you use the -o option, the process will continue recording allocations to the capture file even after memray attach has returned.

as well as that

memray attach works by injecting executable code into a running process.

and memray detach is documented immediately after memray attach. I'm not sure how to be clearer about how this works. Can you suggest any way to improve the docs?

@kushal12340
Copy link

@godlygeek I am sorry for not reading. Thank you for being patient with me and kindly pointing it out.

I can see adding the following line right below Begin tracking allocations in an already-started process might help (I’m happy to make the change if others feel it would be helpful):

Tip

Use memray detach to stop tracking manually, or allow it to stop automatically when the specified duration ends.

That said, in my opinion, this may not significantly improve the documentation, as the current version already seems clear and sufficient.

@jonashaag
Copy link
Author

@godlygeek thank you for the explanation. I agree that this behavior is document perfectly fine.

I do think though that it is bad (or at least surprising) UX. It is to me very unexpected that a command-line tool returns before its work is finished without explicit request:

  • I assume that most of the time users want to actively wait until recording is done; I consider this the "normal case" and so the tool should be optimized for this case.
  • If you really want to not actively wait, for example because you want to record for a long time, I would expected there to be a command-line option like -d/--detach in Docker and other programs.
  • I would expect there to be a warning that recording is done in the background in the output of the program.
  • If using attach without -o, recording is done exactly as long as the TUI is running, which makes the -o behavior especially surprising to me.
  • At the very least there should be an option to have the attach command line wait actively; otherwise I'll have to roll my own waiting code or use an external wall clock.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants