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

Disable git fsmonitor in git_worker to fix hangs when fetching repositories #23626

Conversation

luispadron
Copy link
Contributor

This commit adds the -c core.fsmonitor=false arguments to all git commands (excluding git init) in git_worker.bzl. The purpose of this is to disable git from spawning a file system monitor, when many of these are spawned on a machine that is already under load or out of file descriptors, the git commands will hang indefinitely.

Fixes #21438

…tories

This commit adds the `-c core.fsmonitor=false` arguments to all git commands (excluding `git init`) in `git_worker.bzl`.
The purpose of this is to disable git from spawning a file system monitor, when many of these are spawned on a machine that is already under load or out of file descriptors, the git commands will hang indefinitely.

Fixes #21438
@github-actions github-actions bot added the awaiting-review PR is awaiting review from an assigned reviewer label Sep 14, 2024
@luispadron
Copy link
Contributor Author

I performed tests by spawning many file watchers using the fswatch tool, using a released Bazel version 7.3.1 when running bazel fetch in the examples/lots_of_repos directory of the branch: https://github.com/cgrindel/rules_swift_package_manager/tree/luis/lots-of-repo-example I was able to consistently reproduce stuck fetches.

I confirmed that building //src:bazel and using that instead consistently allows all fetches to continue and I ran into no hangs during the process.


#!/bin/bash

# Number of monitors to create
NUM_MONITORS=100  # Adjust based on your stress test needs

# Directory to monitor (can be the same or different for each monitor)
MONITOR_DIR="/tmp/fsmonitor-test"

# Create the directory if it doesn't exist
mkdir -p "$MONITOR_DIR"

# Array to store process IDs of background fswatch processes
declare -a PIDS

# Function to create and run a file system monitor
create_monitor() {
  local dir=$1
  fswatch "$dir" >/dev/null &
  PIDS+=($!)  # Store the process ID
}

# Create the specified number of monitors
for i in $(seq 1 $NUM_MONITORS); do
  create_monitor "$MONITOR_DIR"
  
  # Optionally, introduce a slight delay to avoid overwhelming the system too quickly
  sleep 0.1
done

echo "Created $NUM_MONITORS file system monitors."


# Function to clean up by killing all background processes
cleanup() {
  echo "Cleaning up monitors..."
  for pid in "${PIDS[@]}"; do
    kill "$pid" 2>/dev/null
  done
  echo "All monitors terminated."
}

# Trap script exit (Ctrl+C or normal exit) to ensure cleanup happens
trap cleanup EXIT

# Keep the script running to simulate long-running monitors
echo "Press Ctrl+C to stop the script and clean up the monitors."
while true; do
  sleep 10  # Keep the script alive indefinitely
done

@@ -196,8 +196,11 @@ def _git_maybe_shallow(ctx, git_repo, command, *args):
return _execute(ctx, git_repo, start + args_list)

def _execute(ctx, git_repo, args):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved the passing of git to be implied in this function as its intended to run git commands anyway. This allows me to add -c which must come directly after git without searching through the args we are given

@fmeum
Copy link
Collaborator

fmeum commented Sep 14, 2024

@bazel-io fork 7.4.0

Copy link
Member

@Wyverald Wyverald left a comment

Choose a reason for hiding this comment

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

nice! thanks for providing lots of context too, that really helps.

@Wyverald Wyverald added team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Sep 16, 2024
@github-actions github-actions bot removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Sep 17, 2024
@Wyverald Wyverald deleted the luis/fix-hangs-during-git-worker-execution-when-fetching-lots-of-repos branch September 17, 2024 20:54
bazel-io pushed a commit to bazel-io/bazel that referenced this pull request Sep 17, 2024
…tories

This commit adds the `-c core.fsmonitor=false` arguments to all git commands (excluding `git init`) in `git_worker.bzl`. The purpose of this is to disable git from spawning a file system monitor, when many of these are spawned on a machine that is already under load or out of file descriptors, the git commands will hang indefinitely.

Fixes bazelbuild#21438

Closes bazelbuild#23626.

PiperOrigin-RevId: 675472544
Change-Id: I79e0b6d1e406a9076cdb810747c382de52face1a
github-merge-queue bot pushed a commit that referenced this pull request Sep 17, 2024
…g repositories (#23647)

This commit adds the `-c core.fsmonitor=false` arguments to all git
commands (excluding `git init`) in `git_worker.bzl`. The purpose of this
is to disable git from spawning a file system monitor, when many of
these are spawned on a machine that is already under load or out of file
descriptors, the git commands will hang indefinitely.

Fixes #21438

Closes #23626.

PiperOrigin-RevId: 675472544
Change-Id: I79e0b6d1e406a9076cdb810747c382de52face1a

Commit
bb577b0

Co-authored-by: Luis Padron <lpadron@squareup.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bazel is stuck fetching external repositories
4 participants