Enable Presto Multi-Container Profiling#260
Conversation
|
@paul-aiyedun please review this PR. |
paul-aiyedun
left a comment
paul-aiyedun
left a comment
There was a problem hiding this comment.
Changes overall look good to me. I had a question about the 10 seconds wait. Also, can we remove the --single-container option now? I believe this was added because of the profiling issue.
| # wait for 10 seconds | ||
| sleep 10 |
There was a problem hiding this comment.
The current implementation works without a 10 seconds wait. Why is this now needed?
There was a problem hiding this comment.
IIRC,nsys stop needs time to write the files. Otherwise it errors out sometimes.
There was a problem hiding this comment.
This function is executed for each query. Adding a 10 second delay here could be significant, especially when running benchmarks for the entire suite.
| fi | ||
|
|
||
| if [[ "$PROFILE" == "ON" && $NUM_WORKERS -gt 1 && "$SINGLE_CONTAINER" == "false" ]]; then | ||
| echo "Error: multi-worker --profile argument is only currently supported with the --single-container option" |
There was a problem hiding this comment.
Can we remove the --single-container option, since it was added as a workaround for this issue?
2 participants
This PR enables profiling with multiple containers.
Each worker container is profiled separately, and written to separate files with worker id as suffix in their profile file name.