Skip to content

Shutdown worker server only when all opened connections are closed#419

Merged
nsavoire merged 1 commit into
mainfrom
nsavoire/fix_workerserver_shutdown
Jul 17, 2024
Merged

Shutdown worker server only when all opened connections are closed#419
nsavoire merged 1 commit into
mainfrom
nsavoire/fix_workerserver_shutdown

Conversation

@nsavoire

Copy link
Copy Markdown
Collaborator

What does this PR do?

Finish handling all opened connections before shutting down worker server.

Motivation

This fixes an issue when profiling library opens a connection to the profiler just before a profiler worker restart:

  • library opens connection
  • worker accepts connection
  • worker initiates shutdown
  • worker sends termination signal to server thread
  • server threads closes all connections and terminates
  • library loses connection and fails to get profiler information

@pr-commenter

pr-commenter Bot commented Jul 17, 2024

Copy link
Copy Markdown

Benchmark results for collatz

Parameters

Baseline Candidate
config baseline candidate
profiler-version ddprof 0.18.0+fbf71d5a.39261263 ddprof 0.18.0+9d33c787.39430593

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 1 metrics, 0 unstable metrics.

See unchanged results
scenario Δ mean execution_time
scenario:ddprof -S bench-collatz --preset cpu_only collatz_runner.sh same

@pr-commenter

pr-commenter Bot commented Jul 17, 2024

Copy link
Copy Markdown

Benchmark results for BadBoggleSolver_run

Parameters

Baseline Candidate
config baseline candidate
profiler-version ddprof 0.18.0+fbf71d5a.39261263 ddprof 0.18.0+9d33c787.39430593

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 1 metrics, 0 unstable metrics.

See unchanged results
scenario Δ mean execution_time
scenario:ddprof -S bench-bad-boggle-solver BadBoggleSolver_run work 1000 same

@nsavoire
nsavoire force-pushed the nsavoire/fix_workerserver_shutdown branch from 1c25372 to 9d33c78 Compare July 17, 2024 14:58

@r1viollet r1viollet left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Well done figuring this one out. 🙇
LGTM

@nsavoire
nsavoire merged commit 5849ebf into main Jul 17, 2024
@nsavoire
nsavoire deleted the nsavoire/fix_workerserver_shutdown branch July 17, 2024 15:11
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