Granulate / gprofiler

gProfiler is a system-wide profiler, combining multiple sampling profilers to produce unified visualization of what your CPU is spending time on.
https://profiler.granulate.io
Apache License 2.0
741 stars 54 forks source link

grpcio 1.62.2 #909

Open slicklash opened 3 weeks ago

slicklash commented 3 weeks ago

Problem

Sometimes gprofilter hangs

Causes

[!CAUTION] The preexec_fn parameter is NOT SAFE to use in the presence of threads in your application. The child process could deadlock before exec is called. https://docs.python.org/3/library/subprocess.html#popen-constructor

Solution

Wait before profiling newly create container and replace preexec_fn with start_new_session flag.

slicklash commented 1 week ago
  1. Any particular reason you deprecated --databricks-job-name-as-service-name?

datbricks client was removed from granulate-utils https://github.com/Granulate/granulate-utils/commit/b65978b5cb11e8aed087d7c428062a53408477c1

  1. Our preexec fn was os.setpgrp and/or prctl(PR_SET_PDEATHSIG). Does start_new_session cover them both?

start_new_session covers os.setpgrp, it's not possible to set PR_SET_PDEATHSIG without preexec_fn

Jongy commented 1 week ago

start_new_session covers os.setpgrp, it's not possible to set PR_SET_PDEATHSIG without preexec_fn

But we needed the PR_SET_PDEATHSIG :/ If we give up on it, we need to verify that all existing uses of start_process are safe even w/o PR_SET_PDEATHSIG.

slicklash commented 3 days ago

start_new_session covers os.setpgrp, it's not possible to set PR_SET_PDEATHSIG without preexec_fn

But we needed the PR_SET_PDEATHSIG :/ If we give up on it, we need to verify that all existing uses of start_process are safe even w/o PR_SET_PDEATHSIG.

Added manual process clean up. SIGKILL is not supported though.