gitserver: Simplify invocation of p4-fusion (#62070)

This wrapper existed to track performance issues, but we have since added better logging options for that through storing the last output and the ability to log to a file.

This adds another layer of indirection that could cause trouble, so simplifying here and calling p4-fusion directly, like we do in the p4-fusion CI pipeline.

Test plan:

The p4 integration test still passes.
This commit is contained in:
Erik Seliger 2024-05-02 01:44:53 +02:00 committed by GitHub
parent 84fb67ddd8
commit ab4477d9b2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 18 additions and 196 deletions

View File

@ -27,19 +27,6 @@ pkg_tar(
srcs = [":gitserver"],
)
pkg_tar(
name = "tar_p4_fusion_wrappers",
srcs = [
"p4-fusion-wrapper-detect-kill.sh",
"process-stats-watcher.sh",
],
remap_paths = {
"/p4-fusion-wrapper-detect-kill.sh": "/usr/local/bin/p4-fusion",
"/process-stats-watcher.sh": "/usr/local/bin/process-stats-watcher.sh",
},
visibility = ["//visibility:public"],
)
oci_image(
name = "image",
base = ":base_image",
@ -50,7 +37,6 @@ oci_image(
],
tars = [
":tar_gitserver",
":tar_p4_fusion_wrappers",
],
user = "sourcegraph",
workdir = "/",

View File

@ -52,20 +52,3 @@ To use `p4-fusion` while developing Sourcegraph, there are a couple of options.
#### Bazel
Native binaries are provided through Bazel, built via Nix in [our fork of p4-fusion](https://github.com/sourcegraph/p4-fusion/actions/workflows/nix-build-and-upload.yaml). It can be invoked either through `./dev/p4-fusion-dev` or directly with `bazel run //dev/tools:p4-fusion`.
#### Native binary executable
The `p4-fusion` native binary has been built on Linux and macOS, but is untested on Windows.
Read the [comprehensive instructions](https://sourcegraph.com/docs/dev/background-information/build_p4_fusion).
If you do go the native binary route, you may also want to enable using the wrapper shell script that detects when the process has been killed and outputs an error so that the calling process can handle it.
That wrapper shell script is `p4-fusion-wrapper-detect-kill.sh`, and in order to use it:
1. Rename the `p4-fusion` binary executable to `p4-fusion-binary` and move it to a location in the `PATH`.
1. Copy the shell script `p4-fusion-wrapper-detect-kill.sh` to a location in the `PATH`, renaming it `p4-fusion`.
1. Copy the shell script `process-stats-watcher.sh` to a location in the `PATH`.
1. Ensure all three of those are executable.
After those steps, when a native `gitserver` process runs `p4-fusion`, it will run the wrapper shell script, which will itself run the `p4-fusion-binary` executable, and the `process-stats-watcher.sh` executable.

View File

@ -38,7 +38,7 @@ commandTests:
- name: "coursier is runnable"
command: "coursier"
- name: "p4-fusion is runnable"
command: "p4-fusion-binary"
command: "p4-fusion"
args:
expectedOutput: ["--noBaseCommit"]
exitCode: 1
@ -56,16 +56,3 @@ fileExistenceTests:
shouldExist: true
uid: 100
gid: 101
# p4-fusion wrappers
- name: '/usr/local/bin/p4-fusion'
path: '/usr/local/bin/p4-fusion'
shouldExist: true
uid: 0
gid: 0
permissions: '-r-xr-xr-x'
- name: '/usr/local/bin/process-stats-watcher.sh'
path: '/usr/local/bin/process-stats-watcher.sh'
shouldExist: true
uid: 0
gid: 0
permissions: '-r-xr-xr-x'

View File

@ -1,75 +0,0 @@
#!/usr/bin/env bash
# shellcheck disable=SC2064,SC2207
# create a file to hold the output of p4-fusion
# TODO: consider recording/storing/capturing the file for logs display in the UI if there's a problem
fusionout=$(mktemp || mktemp -t fusionout_XXXXXXXX)
# create a pipe to use for capturing output of p4-fusion
# so that it can be sent to stdout and also to a file for analyzing later
fusionpipe=$(mktemp || mktemp -t fusionpipe_XXXXXXXX)
rm -f "${fusionpipe}"
mknod "${fusionpipe}" p
tee <"${fusionpipe}" "${fusionout}" &
# create a file to hold the output of `wait`
waitout=$(mktemp || mktemp -t waitout_XXXXXXXX)
# create a file to hold the resource usage of the child process
stats=$(mktemp || mktemp -t resource_XXXXXXXX)
# make sure to clean up on exit
trap "rm -f \"${fusionout}\" \"${fusionpipe}\" \"${waitout}\" \"${stats}\"" EXIT
# launch p4-fusion in the background, sending all output to the pipe for capture and re-echoing
# depends on the p4-fusion binary executable being copied to p4-fusion-binary in the gitserver Dockerfile
p4-fusion-binary "${@}" >"${fusionpipe}" 2>&1 &
# capture the pid of the child process
fpid=$!
# start up a "sidecar" process to capture resource usage.
# it will terminate when the p4-fusion process terminates.
process-stats-watcher.sh "${fpid}" "p4-fusion-binary" >"${stats}" &
spid=$!
# Wait for the child process to finish
wait ${fpid} >"${waitout}" 2>&1
# capture the result of the wait, which is the result of the child process
# or the result of external action on the child process, like SIGKILL
waitcode=$?
# the sidecar process should have exited by now, but just in case, wait for it
wait "${spid}" >/dev/null 2>&1
[ ${waitcode} -eq 0 ] || {
# if the wait exit code indicates a problem,
# check to see if the child process was killed
killed=""
# if the process was killed with SIGKILL, the `wait` process will have generated a notification
grep -qs "Killed" "${waitout}" && killed=y
[ -z "${killed}" ] && {
# If the wait process did not generate an error message, check the process output.
# The process traps SIGINT and SIGTERM; uncaught signals will be displayed as "uncaught"
tail -5 "${fusionout}" | grep -Eqs "Signal Received:|uncaught target signal" && killed=y
}
[ -z "${killed}" ] || {
# include the signal if it's SIGINT, SIGTERM, or SIGKILL
# not gauranteed to work, but nice if we can include the info
signal="$(kill -l ${waitcode})"
[ -z "${signal}" ] || signal=" (SIG${signal})"
# get info if available from the sidecar process
rusage=""
[ -s "${stats}" ] && {
# expect the last (maybe only) line to be four fields:
# RSS VSZ ETIME TIME
x=($(tail -1 "${stats}"))
# NOTE: bash indexes from 0; zsh indexes from 1
[ ${#x[@]} -eq 4 ] && rusage=" At the time of its demise, it had been running for ${x[2]}, had used ${x[3]} CPU time, reserved ${x[1]} RAM and was using ${x[0]}."
}
echo "p4-fusion was killed by an external signal${signal}.${rusage}"
}
}
exit ${waitcode}

View File

@ -1,45 +0,0 @@
#!/usr/bin/env bash
# shellcheck disable=SC2064,SC2207,SC2009
humanize() {
local num=${1}
[[ ${num} =~ ^[0-9][0-9]*$ ]] && num=$(bc <<<"scale=2;${num}/1024/1024")m
printf -- '%s' "${num}"
return 0
}
# read resource usage statistics for a process
# several times a second until it terminates
# at which point, output the most recent stats on stdout
# the output format is "RSS VSZ ETIME TIME"
# input is the pid of the process
pid="${1}"
# and its name, which is used to avoid tracking
# another process in case the original process completed,
# and another started up and got assigned the same pid
cmd="${2}"
unset rss vsz etime time
while true; do
# Alpine has a rather limited `ps`
# it does not limit output to just one process, even when specifying a pid
# so we need to filter the output by pid
x="$(ps -o pid,stat,rss,vsz,etime,time,comm,args "${pid}" | grep "^ *${pid} " | grep "${cmd}" | tail -1)"
[ -z "${x}" ] && break
IFS=" " read -r -a a <<<"$x"
# drop out of here if the process has died or become a zombie - no coming back from the dead
[[ "${a[1]}" =~ ^[ZXx] ]] && break
# only collect stats for processes that are active (running, sleeping, disk sleep, which is waiting for I/O to complete)
# but don't stop until it is really is dead
[[ "${a[1]}" =~ ^[RSD] ]] && {
rss=${a[2]}
vsz=${a[3]}
etime=${a[4]}
time=${a[5]}
}
sleep 0.2
done
printf '%s %s %s %s' "$(humanize "${rss}")" "$(humanize "${vsz}")" "${etime}" "${time}"

View File

@ -177,7 +177,6 @@ oci_image(
":tar_postgres_optimize",
":tar_postgres_reindex",
":tar_prom-wrapper",
"//cmd/gitserver:tar_p4_fusion_wrappers",
"//monitoring:generate_grafana_config_tar",
"tar_image_test_scripts",
] + dependencies_tars(DEPS) + dependencies_tars(ZOEKT_DEPS),

View File

@ -32,7 +32,7 @@ commandTests:
# P4-fusion
- name: "p4-fusion is runnable"
command: "p4-fusion-binary"
command: "p4-fusion"
args:
expectedOutput: [ "--noBaseCommit" ]
exitCode: 1
@ -89,19 +89,6 @@ fileExistenceTests:
# uid: 0
# gid: 0
# permissions: '-r-xr-xr-x'
# p4-fusion wrappers
- name: '/usr/local/bin/p4-fusion'
path: '/usr/local/bin/p4-fusion'
shouldExist: true
uid: 0
gid: 0
permissions: '-r-xr-xr-x'
- name: '/usr/local/bin/process-stats-watcher.sh'
path: '/usr/local/bin/process-stats-watcher.sh'
shouldExist: true
uid: 0
gid: 0
permissions: '-r-xr-xr-x'
# /sg_config_prometheus configuration
- name: '/sg_config_prometheus/prometheus.yml'
path: '/sg_config_prometheus/prometheus.yml'

View File

@ -964,22 +964,22 @@
},
{
"architecture": "x86_64",
"checksum": "Q1LbqvA8R46fS+Zwl18guXt5zvYrw=",
"checksum": "Q1NyPDKnpnwrKJ7+7foWCAaDw7V5s=",
"control": {
"checksum": "sha1-LbqvA8R46fS+Zwl18guXt5zvYrw=",
"range": "bytes=660-1037"
"checksum": "sha1-NyPDKnpnwrKJ7+7foWCAaDw7V5s=",
"range": "bytes=660-1036"
},
"data": {
"checksum": "sha256-Onbp332eMogVPHfUpduSsjA5w+xpyHcCb7VuMA4T1x8=",
"range": "bytes=1038-20466045"
"checksum": "sha256-4VGdMs4AxZGZuJXWW2ztq3UWSnWb0z6+Aa4s5uInmzI=",
"range": "bytes=1037-20466010"
},
"name": "p4-fusion-sg",
"signature": {
"checksum": "sha1-29MX0Y8RvdpmHPm4IPFCo3diYzY=",
"checksum": "sha1-3jfZRXx840UdCOYzfJKSoffWnJY=",
"range": "bytes=0-659"
},
"url": "https://packages.sgdev.org/main/x86_64/p4-fusion-sg-1.13.2-r3.apk",
"version": "1.13.2-r3"
"url": "https://packages.sgdev.org/main/x86_64/p4-fusion-sg-1.13.2-r4.apk",
"version": "1.13.2-r4"
},
{
"architecture": "x86_64",

View File

@ -1424,22 +1424,22 @@
},
{
"architecture": "x86_64",
"checksum": "Q1LbqvA8R46fS+Zwl18guXt5zvYrw=",
"checksum": "Q1NyPDKnpnwrKJ7+7foWCAaDw7V5s=",
"control": {
"checksum": "sha1-LbqvA8R46fS+Zwl18guXt5zvYrw=",
"range": "bytes=660-1037"
"checksum": "sha1-NyPDKnpnwrKJ7+7foWCAaDw7V5s=",
"range": "bytes=660-1036"
},
"data": {
"checksum": "sha256-Onbp332eMogVPHfUpduSsjA5w+xpyHcCb7VuMA4T1x8=",
"range": "bytes=1038-20466045"
"checksum": "sha256-4VGdMs4AxZGZuJXWW2ztq3UWSnWb0z6+Aa4s5uInmzI=",
"range": "bytes=1037-20466010"
},
"name": "p4-fusion-sg",
"signature": {
"checksum": "sha1-29MX0Y8RvdpmHPm4IPFCo3diYzY=",
"checksum": "sha1-3jfZRXx840UdCOYzfJKSoffWnJY=",
"range": "bytes=0-659"
},
"url": "https://packages.sgdev.org/main/x86_64/p4-fusion-sg-1.13.2-r3.apk",
"version": "1.13.2-r3"
"url": "https://packages.sgdev.org/main/x86_64/p4-fusion-sg-1.13.2-r4.apk",
"version": "1.13.2-r4"
},
{
"architecture": "x86_64",