diff --git a/.github/workflows/samba-integration.yml b/.github/workflows/samba-integration.yml new file mode 100644 index 000000000..ab6b7b6f9 --- /dev/null +++ b/.github/workflows/samba-integration.yml @@ -0,0 +1,120 @@ +name: "Samba on FUSE Integration" + +on: + push: + branches: [ master, main ] + paths: + - 'weed/mount/**' + - 'weed/filer/**' + - 'weed/cluster/**' + - 'test/samba/**' + - '.github/workflows/samba-integration.yml' + pull_request: + branches: [ master, main ] + paths: + - 'weed/mount/**' + - 'weed/filer/**' + - 'weed/cluster/**' + - 'test/samba/**' + - '.github/workflows/samba-integration.yml' + workflow_dispatch: + +concurrency: + group: samba-integration/${{ github.ref }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + samba-integration: + name: samba-integration + runs-on: ubuntu-22.04 + timeout-minutes: 45 + + steps: + - name: Checkout code + uses: actions/checkout@v6 + + - name: Set up Go + uses: actions/setup-go@v6 + with: + go-version-file: 'go.mod' + + - name: Start local Docker registry + run: docker run -d --restart=always -p 5000:5000 --name registry registry:2 + + - name: Set up Docker Buildx + uses: docker/setup-buildx-action@v4 + with: + driver-opts: network=host + + - name: Build weed race binary + run: | + cd docker + make binary_race + + - name: Build SeaweedFS e2e image + uses: docker/build-push-action@v7 + with: + context: docker + file: docker/Dockerfile.e2e + tags: localhost:5000/chrislusf/seaweedfs:e2e + push: true + cache-from: type=gha,scope=samba-e2e + cache-to: type=gha,mode=max,scope=samba-e2e + + - name: Tag e2e image for docker compose + run: | + docker pull localhost:5000/chrislusf/seaweedfs:e2e + docker tag localhost:5000/chrislusf/seaweedfs:e2e chrislusf/seaweedfs:e2e + + - name: Build samba image + uses: docker/build-push-action@v7 + with: + context: test/samba + build-contexts: | + chrislusf/seaweedfs:e2e=docker-image://localhost:5000/chrislusf/seaweedfs:e2e + tags: localhost:5000/chrislusf/seaweedfs:samba + push: true + cache-from: type=gha,scope=samba-harness + cache-to: type=gha,mode=max,scope=samba-harness + + - name: Tag samba image for docker compose + run: | + docker pull localhost:5000/chrislusf/seaweedfs:samba + docker tag localhost:5000/chrislusf/seaweedfs:samba chrislusf/seaweedfs:samba + + - name: Start SeaweedFS cluster and Samba + run: | + docker compose -f test/samba/docker-compose.yml up --wait + + - name: Run Samba test battery + run: | + set -o pipefail + docker compose -f test/samba/docker-compose.yml exec -T samba \ + /run_inside_container.sh 2>&1 | tee /tmp/samba-output.log + + - name: Collect logs + if: always() + run: | + mkdir -p /tmp/samba-docker-logs + for svc in master volume filer samba; do + docker compose -f test/samba/docker-compose.yml logs "$svc" \ + > "/tmp/samba-docker-logs/${svc}.log" 2>&1 || true + done + + - name: Tear down + if: always() + run: | + docker compose -f test/samba/docker-compose.yml down -v + + - name: Upload logs + if: always() + uses: actions/upload-artifact@v7 + with: + name: samba-integration-results + path: | + /tmp/samba-output.log + /tmp/samba-docker-logs/ + retention-days: 7 diff --git a/test/samba/Dockerfile b/test/samba/Dockerfile new file mode 100644 index 000000000..e2756a586 --- /dev/null +++ b/test/samba/Dockerfile @@ -0,0 +1,20 @@ +FROM chrislusf/seaweedfs:e2e + +RUN apt-get -o Acquire::Retries=5 -o Acquire::http::Timeout=30 update && \ + DEBIAN_FRONTEND=noninteractive apt-get -o Acquire::Retries=5 -o Acquire::http::Timeout=30 install -y \ + --no-install-recommends \ + --no-install-suggests \ + samba \ + smbclient \ + python3-minimal \ + && apt-get clean \ + && rm -rf /var/lib/apt/lists/* + +COPY smb.conf.template /smb.conf.template +COPY smb_tests.sh /smb_tests.sh +COPY lock_tests.sh /lock_tests.sh +COPY entrypoint.sh /entrypoint.sh +COPY run_inside_container.sh /run_inside_container.sh +RUN chmod +x /smb_tests.sh /lock_tests.sh /entrypoint.sh /run_inside_container.sh + +ENTRYPOINT ["/entrypoint.sh"] diff --git a/test/samba/README.md b/test/samba/README.md new file mode 100644 index 000000000..228db7018 --- /dev/null +++ b/test/samba/README.md @@ -0,0 +1,120 @@ +# Samba on FUSE integration test + +Exports a SeaweedFS FUSE mount over SMB with Samba's `smbd` and drives it with +`smbclient`, verifying that SMB file operations work correctly on top of the +mount and that data stays consistent across both protocols. + +## What it checks + +The functional battery in `smb_tests.sh` covers: + +- connecting to the share and listing the root +- 1 MiB upload/download round-trip with content verification +- subdirectory creation and writes into it +- file rename +- 64 MiB upload/download (exercises SeaweedFS chunk splitting) +- recursive upload of a directory tree +- cross-protocol consistency: files written over SMB appear on the FUSE mount + with identical content, and files written directly on the FUSE mount are + readable over SMB +- deleting files and directory trees + +The locking / concurrency battery in `lock_tests.sh` covers the harder cases a +network-filesystem backend has to get right: + +- **POSIX `fcntl` byte-range locking** on the FUSE mount: a held exclusive lock + denies a conflicting lock, allows a non-overlapping range, and is reacquirable + after release (exercises the mount's `SetLk`/`GetLk`) +- **Distributed locking** (`-dlm`): a file held open for writing on one mount + blocks a writer on a second mount until it is released +- **Distributed-lock integrity**: concurrent writers to the same file from two + mounts leave exactly one intact payload, never a torn mix +- **Concurrency**: parallel writers to distinct files all succeed + +Both FUSE mounts are started with `-dlm` (distributed lock manager). The second +mount (`/mnt/seaweedfs2`) exists only to contend with the smbd-backed mount in +the distributed-locking tests; both see the same filer path, so `.../share` is +the same data on each. + +> Note on DLM semantics: `-dlm` coordinates *write access* (one mount writes a +> file at a time) and guarantees writes are not torn. It does not guarantee +> which concurrent writer wins or instant cross-mount read convergence — the +> holder's buffered data is flushed on close, asynchronously to lock release. + +### Known issue: DLM handoff stalls under same-file contention + +The two handoff checks in test 2 (`blocked SMB write succeeds after the other +mount releases` and `post-release content is the SMB writer's payload`) are +marked **expected-fail** (xfail) — they pin a remaining DLM liveness bug without +failing CI. If the handoff is fixed they flip to `[XPASS]` and turn the suite +red, a reminder to promote them to hard assertions. + +When two mounts contend for the *same* file, the lock handoff does not complete +in a reasonable time because the holder releases the distributed lock only on +the FUSE `Release` op, which the kernel delays by tens of seconds after +`close()` (vs ~12 ms uncontended). The waiting writer's client gives up before +the lock frees. This is a **liveness/latency** problem, not data corruption — +the lock stays over-conservative, so no torn writes occur. + +Two contributing causes have been fixed in the lock client (`weed/cluster/lock_client.go`): + +- the waiter no longer polls with `util.RetryUntil`'s growing backoff; it polls + at a steady cadence so a freed lock is picked up promptly, and +- `Stop()` no longer races the renewal goroutine, which previously could send a + stale unlock token and leave the lock lingering as "owned" at the filer. + +The remaining cause — the holder-side release waiting on FUSE `Release` — needs +the lock released promptly on flush/close (with care for the multi-fd case), and +is left as a follow-up. + +## Layout + +| File | Purpose | +| --- | --- | +| `smb_tests.sh` | SMB functional battery. Shared by both runners. | +| `lock_tests.sh` | SMB locking / concurrency battery. Shared by both runners. | +| `smb.conf.template` | Samba config; placeholders are filled in at run time. | +| `run.sh` | Local runner: `weed mini` + two `-dlm` mounts + `smbd` + both batteries, all as the current user on unprivileged ports. | +| `entrypoint.sh` | Container entrypoint: starts two `-dlm` FUSE mounts and runs `smbd`. | +| `run_inside_container.sh` | Runs both batteries inside the container against the local `smbd`. | +| `Dockerfile` | Adds Samba to the `chrislusf/seaweedfs:e2e` image. | +| `docker-compose.yml` | master + volume + filer + samba services. | + +## Running locally + +Requirements: `weed` on `$PATH`, `fusermount3`, and Samba's `smbd` / +`smbclient` / `smbpasswd` (Debian/Ubuntu: `apt-get install samba smbclient`). + +```sh +test/samba/run.sh +``` + +No `sudo` is needed: `smbd` runs as the current user on port 4450 and all state +lives under a temp work dir that is cleaned up on exit. + +## Running with Docker + +Mirrors the CI job. Requires `/dev/fuse` and `SYS_ADMIN` (provided in the +compose file). + +```sh +# build the base e2e image first (from the repo's docker/ dir) +docker compose -f test/samba/docker-compose.yml up --wait +docker compose -f test/samba/docker-compose.yml exec -T samba /run_inside_container.sh +docker compose -f test/samba/docker-compose.yml down -v +``` + +## CI + +`.github/workflows/samba-integration.yml` runs on changes to `weed/mount/**`, +`weed/filer/**`, or `test/samba/**`. It builds the e2e image, builds the Samba +harness image on top, brings up the cluster, runs the battery, and uploads +server logs as artifacts. + +## Notes + +- The share disables Samba's DOS-attribute / xattr mapping and oplocks. The + SeaweedFS FUSE mount does not implement that surface, and leaving it on + produces `NT_STATUS_NOT_SUPPORTED` errors unrelated to data integrity. +- The share path is a subdirectory of the mount (`.../share`) so the runner can + verify SMB-side operations directly on the FUSE side. diff --git a/test/samba/docker-compose.yml b/test/samba/docker-compose.yml new file mode 100644 index 000000000..1d17d45ad --- /dev/null +++ b/test/samba/docker-compose.yml @@ -0,0 +1,60 @@ +services: + master: + image: chrislusf/seaweedfs:e2e + command: "-v=4 master -ip=master -ip.bind=0.0.0.0 -raftBootstrap" + healthcheck: + test: ["CMD", "curl", "--fail", "-I", "http://localhost:9333/cluster/healthz"] + interval: 2s + timeout: 10s + retries: 30 + start_period: 10s + + volume: + image: chrislusf/seaweedfs:e2e + command: "-v=4 volume -master=master:9333 -ip=volume -ip.bind=0.0.0.0 -preStopSeconds=1" + healthcheck: + test: ["CMD", "curl", "--fail", "-I", "http://localhost:8080/healthz"] + interval: 2s + timeout: 10s + retries: 15 + start_period: 5s + depends_on: + master: + condition: service_healthy + + filer: + image: chrislusf/seaweedfs:e2e + command: "-v=4 filer -master=master:9333 -ip=filer -ip.bind=0.0.0.0" + healthcheck: + test: ["CMD", "curl", "--fail", "-I", "http://localhost:8888/healthz"] + interval: 2s + timeout: 10s + retries: 15 + start_period: 5s + depends_on: + volume: + condition: service_healthy + + samba: + image: chrislusf/seaweedfs:samba + build: + context: . + environment: + FILER: filer:8888 + cap_add: + - SYS_ADMIN + devices: + - /dev/fuse + security_opt: + - apparmor:unconfined + healthcheck: + test: + - "CMD-SHELL" + - "mountpoint -q /mnt/seaweedfs && mountpoint -q /mnt/seaweedfs2 && smbclient -L 127.0.0.1 -p 445 -U smbtest%smbtest -m SMB3 >/dev/null 2>&1" + interval: 3s + timeout: 10s + retries: 20 + start_period: 15s + depends_on: + filer: + condition: service_healthy diff --git a/test/samba/entrypoint.sh b/test/samba/entrypoint.sh new file mode 100755 index 000000000..d5ab257cc --- /dev/null +++ b/test/samba/entrypoint.sh @@ -0,0 +1,76 @@ +#!/usr/bin/env bash +# +# Entrypoint for the samba test container. +# +# Mounts SeaweedFS over FUSE twice, both with distributed locking (-dlm) so the +# locking tests can exercise cross-mount write coordination: +# - MOUNT_DIR (/mnt/seaweedfs) is exported over SMB by smbd +# - MOUNT2_DIR (/mnt/seaweedfs2) is a second, independent mount of the same +# filer used to contend with the SMB writer +# +# Both mounts see the same filer path, so .../share is the same data on each. +# smbd runs in the foreground (as root, which owns the mounts), so the share +# uses "force user = root". +set -euo pipefail + +FILER="${FILER:-filer:8888}" +MOUNT_DIR="${MOUNT_DIR:-/mnt/seaweedfs}" +MOUNT2_DIR="${MOUNT2_DIR:-/mnt/seaweedfs2}" +SHARE_DIR="${MOUNT_DIR}/share" +STATE_DIR="${STATE_DIR:-/var/lib/samba-test}" +SMB_PORT="${SMB_PORT:-445}" +SMB_USER="${SMB_USER:-smbtest}" +SMB_PASS="${SMB_PASS:-smbtest}" + +mkdir -p "${MOUNT_DIR}" "${MOUNT2_DIR}" \ + "${STATE_DIR}/private" "${STATE_DIR}/state" "${STATE_DIR}/cache" \ + "${STATE_DIR}/lock" "${STATE_DIR}/pid" "${STATE_DIR}/ncalrpc" + +# mount_seaweedfs — mount with -dlm and wait for it. +mount_seaweedfs() { + local dir="$1" log="$2" + echo "==> Mounting SeaweedFS (${FILER}) at ${dir} with -dlm" + weed -v=1 mount \ + -filer="${FILER}" \ + -dir="${dir}" \ + -filer.path=/ \ + -dirAutoCreate \ + -allowOthers \ + -dlm \ + >"${log}" 2>&1 & + local pid=$! + for _ in $(seq 1 120); do + if mountpoint -q "${dir}"; then + return 0 + fi + if ! kill -0 "${pid}" 2>/dev/null; then + echo "weed mount (${dir}) exited early; log tail:" >&2 + tail -n 100 "${log}" >&2 || true + exit 1 + fi + sleep 0.5 + done + echo "FUSE mount ${dir} did not come up" >&2 + tail -n 100 "${log}" >&2 || true + exit 1 +} + +mount_seaweedfs "${MOUNT_DIR}" /var/log/weed-mount.log +mount_seaweedfs "${MOUNT2_DIR}" /var/log/weed-mount2.log + +mkdir -p "${SHARE_DIR}" +chmod 0777 "${SHARE_DIR}" + +# --- configure and start smbd ---------------------------------------------- +echo "==> Configuring Samba share on port ${SMB_PORT}" +sed -e "s#@SHARE_PATH@#${SHARE_DIR}#g" \ + -e "s#@STATE_DIR@#${STATE_DIR}#g" \ + -e "s#@SMB_PORT@#${SMB_PORT}#g" \ + -e "s#@FORCE_USER@#root#g" \ + /smb.conf.template >/etc/samba/smb.conf + +id -u "${SMB_USER}" >/dev/null 2>&1 || useradd -M -s /usr/sbin/nologin "${SMB_USER}" +printf '%s\n%s\n' "${SMB_PASS}" "${SMB_PASS}" | smbpasswd -a -s "${SMB_USER}" + +echo "==> Starting smbd" +exec smbd -F --no-process-group -s /etc/samba/smb.conf diff --git a/test/samba/lock_tests.sh b/test/samba/lock_tests.sh new file mode 100755 index 000000000..da7f2e439 --- /dev/null +++ b/test/samba/lock_tests.sh @@ -0,0 +1,233 @@ +#!/usr/bin/env bash +# +# Locking / concurrency test battery for Samba on a SeaweedFS FUSE mount. +# +# Covers the challenges a network-filesystem backend has to get right: +# 1. POSIX fcntl byte-range locking on the FUSE mount (SetLk/GetLk) +# 2. Distributed locking (-dlm): a write held open on one mount blocks a +# writer on another mount until it is released +# 3. Distributed locking integrity: concurrent writers to the same file from +# two mounts produce intact (non-torn) data +# 4. Concurrent writers to distinct files all succeed +# +# Required env: +# SMB_USER, SMB_PASS samba credentials +# MOUNT_SHARE dir on the smbd-backed FUSE mount (mount 1) +# MOUNT2_SHARE dir on the second FUSE mount (mount 2) +# Optional env: +# SMB_HOST (127.0.0.1), SMB_SHARE (seaweedfs), SMB_PORT (445) +set -uo pipefail + +SMB_HOST="${SMB_HOST:-127.0.0.1}" +SMB_SHARE="${SMB_SHARE:-seaweedfs}" +SMB_PORT="${SMB_PORT:-445}" +SMB_USER="${SMB_USER:?SMB_USER is required}" +SMB_PASS="${SMB_PASS:?SMB_PASS is required}" +MOUNT_SHARE="${MOUNT_SHARE:?MOUNT_SHARE is required}" +MOUNT2_SHARE="${MOUNT2_SHARE:?MOUNT2_SHARE is required}" + +WORK="$(mktemp -d /tmp/samba-locktest.XXXXXX)" +trap 'rm -rf "${WORK}"' EXIT + +PASS=0 +FAIL=0 +XFAIL=0 +XPASS=0 +pass() { printf ' [PASS] %s\n' "$1"; PASS=$((PASS + 1)); } +fail() { printf ' [FAIL] %s\n' "$1"; FAIL=$((FAIL + 1)); } +# Expected failure: a known-broken behavior. [XFAIL] does not fail the suite; +# an unexpected pass ([XPASS]) does, so the check gets promoted once it's fixed. +xfail() { printf ' [XFAIL] %s\n' "$1"; XFAIL=$((XFAIL + 1)); } +xpass() { printf ' [XPASS] %s\n' "$1"; XPASS=$((XPASS + 1)); } + +smb() { + smbclient "//${SMB_HOST}/${SMB_SHARE}" -p "${SMB_PORT}" \ + -U "${SMB_USER}%${SMB_PASS}" -m SMB3 -c "$1" +} +md5() { md5sum "$1" | awk '{print $1}'; } + +# 1. POSIX fcntl byte-range locking on the FUSE mount ------------------------ +# Exercises the mount's SetLk/GetLk via two processes contending over fcntl +# (F_SETLK) byte-range locks. python3's fcntl.lockf issues real POSIX locks. +echo "==> 1. POSIX fcntl byte-range locking (FUSE mount SetLk/GetLk)" +lockfile="${MOUNT_SHARE}/fcntl_lock.dat" +: >"${lockfile}" +fcntl_out="$(python3 - "${lockfile}" <<'PY' +import fcntl, os, sys + +path = sys.argv[1] +parent_to_child_r, parent_to_child_w = os.pipe() # release signal +child_to_parent_r, child_to_parent_w = os.pipe() # locked signal + +pid = os.fork() +if pid == 0: # child: hold an exclusive lock on [0,100) + fd = os.open(path, os.O_RDWR | os.O_CREAT, 0o644) + fcntl.lockf(fd, fcntl.LOCK_EX, 100, 0, 0) + os.write(child_to_parent_w, b"L") + os.read(parent_to_child_r, 1) # wait until parent says release + fcntl.lockf(fd, fcntl.LOCK_UN, 100, 0, 0) + os.close(fd) + os._exit(0) + +# parent +os.read(child_to_parent_r, 1) # wait until child holds the lock +fd = os.open(path, os.O_RDWR | os.O_CREAT, 0o644) +results = [] + +# a. a conflicting exclusive lock must be denied while the child holds it +try: + fcntl.lockf(fd, fcntl.LOCK_EX | fcntl.LOCK_NB, 100, 0, 0) + fcntl.lockf(fd, fcntl.LOCK_UN, 100, 0, 0) + results.append(("conflicting exclusive lock denied while held", False)) +except OSError: + results.append(("conflicting exclusive lock denied while held", True)) + +# b. a non-overlapping range must be grantable +try: + fcntl.lockf(fd, fcntl.LOCK_EX | fcntl.LOCK_NB, 100, 200, 0) + fcntl.lockf(fd, fcntl.LOCK_UN, 100, 200, 0) + results.append(("non-overlapping range lock granted", True)) +except OSError: + results.append(("non-overlapping range lock granted", False)) + +# c. after the holder releases, the lock must be acquirable +os.write(parent_to_child_w, b"R") +os.waitpid(pid, 0) +try: + fcntl.lockf(fd, fcntl.LOCK_EX | fcntl.LOCK_NB, 100, 0, 0) + fcntl.lockf(fd, fcntl.LOCK_UN, 100, 0, 0) + results.append(("lock acquirable after holder releases", True)) +except OSError: + results.append(("lock acquirable after holder releases", False)) + +for name, ok in results: + print((" [PASS] " if ok else " [FAIL] ") + name) +sys.exit(0 if all(ok for _, ok in results) else 1) +PY +)" +echo "${fcntl_out}" +PASS=$((PASS + $(grep -c '\[PASS\]' <<<"${fcntl_out}"))) +FAIL=$((FAIL + $(grep -c '\[FAIL\]' <<<"${fcntl_out}"))) + +# 2. Distributed lock blocks a cross-mount writer, then hands it off ---------- +# mount 2 holds a file open for writing (holding the DLM lock on its path). +# An SMB put of the same file goes through mount 1 and must (a) block while +# mount 2 holds it and (b) actually SUCCEED once mount 2 releases, leaving the +# SMB writer's payload on disk. smbclient gets a long client timeout (-t) so we +# are testing the lock handoff itself, not smbclient's own ~20s default timeout. +# +# KNOWN ISSUE (expected failure): the (b) handoff checks are marked xfail. The +# holder releases the distributed lock only on FUSE Release, which the kernel +# delays for tens of seconds under this contention, so the waiting writer does +# not acquire in time. This is a DLM liveness bug, not data corruption. When the +# holder-side release is fixed these flip to [XPASS] and fail the suite, a +# reminder to promote them to hard assertions. +echo "==> 2. distributed lock: cross-mount write coordination" +dlmfile="dlm_coord.bin" +newdata="${WORK}/dlm_new.bin" +head -c 4096 /dev/urandom >"${newdata}" + +# Hold the file open for writing on mount 2 via fd 9 -> holds the DLM lock. +exec 9>"${MOUNT2_SHARE}/${dlmfile}" +printf 'held-by-mount2' >&9 + +# Start the SMB write; record its real exit code when it returns. +rm -f "${WORK}/dlm_put.rc" +( + smbclient "//${SMB_HOST}/${SMB_SHARE}" -p "${SMB_PORT}" \ + -U "${SMB_USER}%${SMB_PASS}" -m SMB3 -t 120 \ + -c "put ${newdata} ${dlmfile}" >/dev/null 2>&1 + echo "$?" >"${WORK}/dlm_put.rc" +) & +smb_bg=$! + +sleep 4 +if [[ ! -f "${WORK}/dlm_put.rc" ]]; then + pass "SMB write blocks while another mount holds the file open" +else + fail "SMB write returned early instead of blocking (rc=$(cat "${WORK}/dlm_put.rc"))" +fi + +# Release mount 2's DLM lock; the blocked SMB write must now complete. +exec 9>&- + +# Wait (bounded) for the SMB put to finish so a stuck handoff fails the test +# instead of hanging the suite. +put_rc="timeout" +for _ in $(seq 1 20); do + if [[ -f "${WORK}/dlm_put.rc" ]]; then + put_rc="$(cat "${WORK}/dlm_put.rc")" + break + fi + sleep 1 +done +kill "${smb_bg}" 2>/dev/null +wait "${smb_bg}" 2>/dev/null + +# xfail: the handoff stalls because the lock is freed only on the delayed FUSE +# Release. A pass here means the holder-side release was fixed. +if [[ "${put_rc}" == "0" ]]; then + xpass "blocked SMB write succeeds after the other mount releases" +else + xfail "blocked SMB write succeeds after the other mount releases (rc=${put_rc})" +fi + +# A correct handoff leaves the SMB writer's payload on disk: mount 1 acquired +# the lock and wrote after mount 2 released. +got="${WORK}/dlm_got.bin" +if smb "get ${dlmfile} ${got}" >/dev/null 2>&1 && [[ "$(md5 "${got}")" == "$(md5 "${newdata}")" ]]; then + xpass "post-release content is the SMB writer's payload (correct handoff)" +else + xfail "post-release content is the SMB writer's payload (correct handoff)" +fi + +# 3. Distributed lock integrity: concurrent writers, same file --------------- +# An SMB writer (mount 1) and a direct writer (mount 2) race on one file. DLM +# serializes them, so the result must be exactly one of the two payloads. +echo "==> 3. distributed lock: concurrent writers produce intact data" +racefile="dlm_race.bin" +payloadA="${WORK}/dlm_raceA.bin" +head -c 1048576 /dev/urandom >"${payloadA}" +payloadB="direct-write-from-mount2-payload" +(smb "put ${payloadA} ${racefile}" >/dev/null 2>&1) & +(printf '%s' "${payloadB}" >"${MOUNT2_SHARE}/${racefile}") & +wait +racegot="${WORK}/dlm_race_got.bin" +if smb "get ${racefile} ${racegot}" >/dev/null 2>&1 && + { [[ "$(md5 "${racegot}")" == "$(md5 "${payloadA}")" ]] || [[ "$(cat "${racegot}")" == "${payloadB}" ]]; }; then + pass "concurrent same-file writers leave one intact payload" +else + fail "concurrent same-file writers leave one intact payload" +fi + +# 4. Concurrent writers to distinct files ------------------------------------ +echo "==> 4. concurrent writers to distinct files" +n=6 +declare -a srcs=() +for i in $(seq 1 "${n}"); do + s="${WORK}/cc_${i}.bin" + head -c 1048576 /dev/urandom >"${s}" + srcs+=("${s}") + (smb "put ${s} concurrent_${i}.bin" >/dev/null 2>&1) & +done +wait +all_ok=true +for i in $(seq 1 "${n}"); do + g="${WORK}/cc_got_${i}.bin" + if ! smb "get concurrent_${i}.bin ${g}" >/dev/null 2>&1 || + [[ "$(md5 "${srcs[$((i - 1))]}")" != "$(md5 "${g}")" ]]; then + all_ok=false + fi +done +if ${all_ok}; then + pass "${n} concurrent distinct-file writes all intact" +else + fail "${n} concurrent distinct-file writes all intact" +fi + +echo +echo "==> Summary: ${PASS} passed, ${FAIL} failed, ${XFAIL} expected-fail" +if [[ "${XPASS}" -gt 0 ]]; then + echo "==> ${XPASS} check(s) unexpectedly passed - the DLM handoff appears fixed; promote them from xfail to assertions" +fi +[[ "${FAIL}" -eq 0 && "${XPASS}" -eq 0 ]] diff --git a/test/samba/run.sh b/test/samba/run.sh new file mode 100755 index 000000000..4d8eb3b41 --- /dev/null +++ b/test/samba/run.sh @@ -0,0 +1,203 @@ +#!/usr/bin/env bash +# +# Run the SMB (Samba) integration test against a SeaweedFS FUSE mount. +# +# Pipeline: +# 1. start a self-contained "weed mini" (master + volume + filer in one) +# 2. mount the filesystem with "weed mount" +# 3. export a subdirectory of the mount over SMB with smbd +# 4. drive the share with smbclient (test/samba/smb_tests.sh) +# +# Everything runs as the current user on unprivileged ports, so no sudo is +# required. State lives under a temp work dir and is removed on exit. +# +# Requirements: weed in $PATH, fusermount3, and Samba's smbd / smbclient / +# smbpasswd (Debian/Ubuntu: apt-get install samba smbclient). +# +# Usage: +# test/samba/run.sh +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +WEED_BIN="${WEED_BIN:-weed}" +WORK_DIR="${WORK_DIR:-$(mktemp -d /tmp/seaweedfs-samba.XXXXXX)}" +MOUNT_DIR="${MOUNT_DIR:-${WORK_DIR}/mnt}" +MOUNT2_DIR="${MOUNT2_DIR:-${WORK_DIR}/mnt2}" +DATA_DIR="${DATA_DIR:-${WORK_DIR}/data}" +LOG_DIR="${LOG_DIR:-${WORK_DIR}/logs}" +STATE_DIR="${WORK_DIR}/samba" +SHARE_DIR="${MOUNT_DIR}/share" +SHARE_DIR2="${MOUNT2_DIR}/share" + +FILER_PORT="${FILER_PORT:-28888}" +FILER_ADDR="127.0.0.1:${FILER_PORT}" +SMB_PORT="${SMB_PORT:-4450}" +SMB_SHARE="seaweedfs" +SMB_USER="${SMB_USER:-$(id -un)}" +SMB_PASS="${SMB_PASS:-seaweedfs}" + +SMBD_BIN="$(command -v smbd || echo /usr/sbin/smbd)" +SMBPASSWD_BIN="$(command -v smbpasswd || echo /usr/bin/smbpasswd)" + +CI_LOG_DIR="/tmp/seaweedfs-samba-logs" + +mini_pid="" +mount_pid="" +mount2_pid="" +smbd_pid="" + +unmount_dir() { + local dir="$1" + if mountpoint -q "${dir}" 2>/dev/null; then + fusermount3 -u "${dir}" 2>/dev/null || + fusermount -u "${dir}" 2>/dev/null || true + fi +} + +cleanup() { + set +e + if [[ -n "${smbd_pid}" ]] && kill -0 "${smbd_pid}" 2>/dev/null; then + kill -TERM "${smbd_pid}" 2>/dev/null || true + wait "${smbd_pid}" 2>/dev/null || true + fi + for p in "${mount_pid}" "${mount2_pid}"; do + if [[ -n "${p}" ]] && kill -0 "${p}" 2>/dev/null; then + kill -TERM "${p}" 2>/dev/null || true + wait "${p}" 2>/dev/null || true + fi + done + unmount_dir "${MOUNT_DIR}" + unmount_dir "${MOUNT2_DIR}" + if [[ -n "${mini_pid}" ]] && kill -0 "${mini_pid}" 2>/dev/null; then + kill -TERM "${mini_pid}" 2>/dev/null || true + wait "${mini_pid}" 2>/dev/null || true + fi + # Copy logs to a fixed path for CI artifact upload. + mkdir -p "${CI_LOG_DIR}" + cp "${LOG_DIR}"/*.log "${LOG_DIR}"/*.out "${STATE_DIR}/smbd.log" "${CI_LOG_DIR}/" 2>/dev/null || true +} +trap cleanup EXIT INT TERM + +mkdir -p "${MOUNT_DIR}" "${MOUNT2_DIR}" "${DATA_DIR}" "${LOG_DIR}" \ + "${STATE_DIR}/private" "${STATE_DIR}/state" "${STATE_DIR}/cache" \ + "${STATE_DIR}/lock" "${STATE_DIR}/pid" "${STATE_DIR}/ncalrpc" + +# --- 1. weed mini ----------------------------------------------------------- +echo "==> Starting weed mini on ${FILER_ADDR}" +"${WEED_BIN}" mini \ + -dir="${DATA_DIR}" \ + -ip=127.0.0.1 \ + -filer.port="${FILER_PORT}" \ + -s3=false \ + -webdav=false \ + -admin.ui=false \ + >"${LOG_DIR}/mini.log" 2>&1 & +mini_pid=$! + +for i in $(seq 1 60); do + if (echo >"/dev/tcp/127.0.0.1/${FILER_PORT}") 2>/dev/null; then + break + fi + if ! kill -0 "${mini_pid}" 2>/dev/null; then + echo "weed mini exited early; log tail:" >&2 + tail -n 100 "${LOG_DIR}/mini.log" >&2 || true + exit 1 + fi + sleep 0.5 +done +if ! (echo >"/dev/tcp/127.0.0.1/${FILER_PORT}") 2>/dev/null; then + echo "weed mini filer did not become reachable within 30s; log tail:" >&2 + tail -n 100 "${LOG_DIR}/mini.log" >&2 || true + exit 1 +fi + +# --- 2. weed mount (two mounts, both with -dlm) ----------------------------- +# mount_with_dlm +mount_with_dlm() { + local dir="$1" log="$2" pidvar="$3" pid + echo "==> Mounting SeaweedFS at ${dir} with -dlm" + "${WEED_BIN}" mount \ + -filer="${FILER_ADDR}" \ + -dir="${dir}" \ + -filer.path=/ \ + -dirAutoCreate \ + -dlm \ + >"${log}" 2>&1 & + pid=$! + printf -v "${pidvar}" '%s' "${pid}" + for _ in $(seq 1 60); do + if mountpoint -q "${dir}"; then + return 0 + fi + if ! kill -0 "${pid}" 2>/dev/null; then + echo "weed mount (${dir}) exited early; log tail:" >&2 + tail -n 100 "${log}" >&2 || true + exit 1 + fi + sleep 0.5 + done + echo "FUSE mount ${dir} did not come up within 30s" >&2 + tail -n 100 "${log}" >&2 || true + exit 1 +} + +mount_with_dlm "${MOUNT_DIR}" "${LOG_DIR}/mount.log" mount_pid +mount_with_dlm "${MOUNT2_DIR}" "${LOG_DIR}/mount2.log" mount2_pid + +mkdir -p "${SHARE_DIR}" + +# --- 3. smbd ---------------------------------------------------------------- +echo "==> Generating smb.conf and starting smbd on port ${SMB_PORT}" +SMB_CONF="${STATE_DIR}/smb.conf" +sed -e "s#@SHARE_PATH@#${SHARE_DIR}#g" \ + -e "s#@STATE_DIR@#${STATE_DIR}#g" \ + -e "s#@SMB_PORT@#${SMB_PORT}#g" \ + -e "s#@FORCE_USER@#${SMB_USER}#g" \ + "${SCRIPT_DIR}/smb.conf.template" >"${SMB_CONF}" + +printf '%s\n%s\n' "${SMB_PASS}" "${SMB_PASS}" | + "${SMBPASSWD_BIN}" -c "${SMB_CONF}" -a -s "${SMB_USER}" + +"${SMBD_BIN}" -F --no-process-group -s "${SMB_CONF}" >"${LOG_DIR}/smbd.out" 2>&1 & +smbd_pid=$! + +for i in $(seq 1 60); do + if (echo >"/dev/tcp/127.0.0.1/${SMB_PORT}") 2>/dev/null; then + break + fi + if ! kill -0 "${smbd_pid}" 2>/dev/null; then + echo "smbd exited early; log tail:" >&2 + tail -n 100 "${LOG_DIR}/smbd.out" "${STATE_DIR}/smbd.log" 2>/dev/null >&2 || true + exit 1 + fi + sleep 0.5 +done +if ! (echo >"/dev/tcp/127.0.0.1/${SMB_PORT}") 2>/dev/null; then + echo "smbd did not become reachable within 30s; log tail:" >&2 + tail -n 100 "${LOG_DIR}/smbd.out" "${STATE_DIR}/smbd.log" 2>/dev/null >&2 || true + exit 1 +fi + +# --- 4. run the test batteries --------------------------------------------- +rc=0 + +echo "==> Running SMB functional test battery" +SMB_HOST=127.0.0.1 \ + SMB_SHARE="${SMB_SHARE}" \ + SMB_PORT="${SMB_PORT}" \ + SMB_USER="${SMB_USER}" \ + SMB_PASS="${SMB_PASS}" \ + SHARE_FS_PATH="${SHARE_DIR}" \ + "${SCRIPT_DIR}/smb_tests.sh" || rc=1 + +echo "==> Running SMB locking / concurrency test battery" +SMB_HOST=127.0.0.1 \ + SMB_SHARE="${SMB_SHARE}" \ + SMB_PORT="${SMB_PORT}" \ + SMB_USER="${SMB_USER}" \ + SMB_PASS="${SMB_PASS}" \ + MOUNT_SHARE="${SHARE_DIR}" \ + MOUNT2_SHARE="${SHARE_DIR2}" \ + "${SCRIPT_DIR}/lock_tests.sh" || rc=1 + +exit "${rc}" diff --git a/test/samba/run_inside_container.sh b/test/samba/run_inside_container.sh new file mode 100755 index 000000000..9452ec153 --- /dev/null +++ b/test/samba/run_inside_container.sh @@ -0,0 +1,24 @@ +#!/usr/bin/env bash +# +# Runs the SMB test batteries inside the samba container against the local smbd, +# which serves /mnt/seaweedfs/share over a SeaweedFS FUSE mount. A second FUSE +# mount (/mnt/seaweedfs2) backs the distributed-locking tests. +# Invoked via: docker compose exec samba /run_inside_container.sh +set -euo pipefail + +export SMB_HOST=127.0.0.1 +export SMB_SHARE=seaweedfs +export SMB_PORT="${SMB_PORT:-445}" +export SMB_USER="${SMB_USER:-smbtest}" +export SMB_PASS="${SMB_PASS:-smbtest}" +export SHARE_FS_PATH="${SHARE_FS_PATH:-/mnt/seaweedfs/share}" +export MOUNT_SHARE="${MOUNT_SHARE:-/mnt/seaweedfs/share}" +export MOUNT2_SHARE="${MOUNT2_SHARE:-/mnt/seaweedfs2/share}" + +rc=0 +echo "############ SMB functional tests ############" +/smb_tests.sh || rc=1 +echo +echo "############ SMB locking / concurrency tests ############" +/lock_tests.sh || rc=1 +exit "${rc}" diff --git a/test/samba/smb.conf.template b/test/samba/smb.conf.template new file mode 100644 index 000000000..0e89f275f --- /dev/null +++ b/test/samba/smb.conf.template @@ -0,0 +1,52 @@ +[global] + server role = standalone server + workgroup = WORKGROUP + server string = SeaweedFS FUSE Samba test + security = user + server min protocol = SMB2 + smb ports = @SMB_PORT@ + bind interfaces only = yes + interfaces = lo 127.0.0.1 + + # Self-contained state so smbd can run rootless and leaves nothing behind + # outside the test work directory. + private dir = @STATE_DIR@/private + state directory = @STATE_DIR@/state + cache directory = @STATE_DIR@/cache + lock directory = @STATE_DIR@/lock + pid directory = @STATE_DIR@/pid + ncalrpc dir = @STATE_DIR@/ncalrpc + log file = @STATE_DIR@/smbd.log + log level = 1 + usershare max shares = 0 + + # No printing subsystem in a file-server test. + load printers = no + printing = bsd + printcap name = /dev/null + disable spoolss = yes + + # The SeaweedFS FUSE mount does not implement the full xattr / DOS-attribute + # surface Samba uses by default. Disabling these avoids spurious + # NT_STATUS_NOT_SUPPORTED / EOPNOTSUPP errors unrelated to data integrity. + ea support = no + store dos attributes = no + map archive = no + map hidden = no + map system = no + map readonly = no + + # A network-filesystem backend should not advertise local oplocks/leases. + oplocks = no + level2 oplocks = no + kernel oplocks = no + posix locking = no + +[seaweedfs] + path = @SHARE_PATH@ + comment = SeaweedFS share backed by a FUSE mount + browseable = yes + read only = no + create mask = 0644 + directory mask = 0755 + force user = @FORCE_USER@ diff --git a/test/samba/smb_tests.sh b/test/samba/smb_tests.sh new file mode 100755 index 000000000..9fca446da --- /dev/null +++ b/test/samba/smb_tests.sh @@ -0,0 +1,172 @@ +#!/usr/bin/env bash +# +# SMB protocol test battery against a Samba share backed by a SeaweedFS FUSE +# mount. Driven both by the local runner (test/samba/run.sh) and by the Docker +# harness (run_inside_container.sh). +# +# Required env: +# SMB_USER samba username +# SMB_PASS samba password +# Optional env: +# SMB_HOST samba host (default 127.0.0.1) +# SMB_SHARE share name (default seaweedfs) +# SMB_PORT smbd port (default 445) +# SHARE_FS_PATH directory on the FUSE mount that backs the share. When set, +# the suite also checks cross-protocol consistency: data written +# over SMB is visible on the FUSE mount, and vice versa. +set -uo pipefail + +SMB_HOST="${SMB_HOST:-127.0.0.1}" +SMB_SHARE="${SMB_SHARE:-seaweedfs}" +SMB_PORT="${SMB_PORT:-445}" +SMB_USER="${SMB_USER:?SMB_USER is required}" +SMB_PASS="${SMB_PASS:?SMB_PASS is required}" +SHARE_FS_PATH="${SHARE_FS_PATH:-}" + +WORK="$(mktemp -d /tmp/samba-smbtest.XXXXXX)" +trap 'rm -rf "${WORK}"' EXIT + +PASS=0 +FAIL=0 +pass() { printf ' [PASS] %s\n' "$1"; PASS=$((PASS + 1)); } +fail() { printf ' [FAIL] %s\n' "$1"; FAIL=$((FAIL + 1)); } + +# Run one or more smbclient commands (separated by ';') against the share. +smb() { + smbclient "//${SMB_HOST}/${SMB_SHARE}" -p "${SMB_PORT}" \ + -U "${SMB_USER}%${SMB_PASS}" -m SMB3 -c "$1" +} + +md5() { md5sum "$1" | awk '{print $1}'; } + +echo "==> Target //${SMB_HOST}/${SMB_SHARE} (port ${SMB_PORT}) as ${SMB_USER}" +[[ -n "${SHARE_FS_PATH}" ]] && echo "==> Cross-protocol checks against ${SHARE_FS_PATH}" + +# 1. Connectivity ------------------------------------------------------------ +echo "==> 1. connectivity" +if smb "ls" >/dev/null 2>&1; then + pass "connect and list share root" +else + fail "connect and list share root" +fi + +# 2. Upload / download round-trip ------------------------------------------- +echo "==> 2. upload / download round-trip" +src="${WORK}/src.bin" +head -c 1048576 /dev/urandom >"${src}" # 1 MiB +if smb "put ${src} roundtrip.bin" >/dev/null 2>&1; then + pass "put 1 MiB file" +else + fail "put 1 MiB file" +fi +got="${WORK}/got.bin" +if smb "get roundtrip.bin ${got}" >/dev/null 2>&1 && [[ "$(md5 "${src}")" == "$(md5 "${got}")" ]]; then + pass "get returns identical content" +else + fail "get returns identical content" +fi +if [[ -n "${SHARE_FS_PATH}" ]]; then + if [[ -f "${SHARE_FS_PATH}/roundtrip.bin" ]] && [[ "$(md5 "${SHARE_FS_PATH}/roundtrip.bin")" == "$(md5 "${src}")" ]]; then + pass "SMB-written file visible on FUSE mount with identical content" + else + fail "SMB-written file visible on FUSE mount with identical content" + fi +fi + +# 3. Directory operations ---------------------------------------------------- +echo "==> 3. directory operations" +if smb "mkdir docs; cd docs; put ${src} nested.bin; ls" >/dev/null 2>&1; then + pass "mkdir + put into subdirectory" +else + fail "mkdir + put into subdirectory" +fi +if [[ -z "${SHARE_FS_PATH}" || -f "${SHARE_FS_PATH}/docs/nested.bin" ]]; then + pass "nested file present" +else + fail "nested file present" +fi + +# 4. Rename ------------------------------------------------------------------ +echo "==> 4. rename" +if smb "rename roundtrip.bin renamed.bin" >/dev/null 2>&1; then + pass "rename file" +else + fail "rename file" +fi +renback="${WORK}/renamed.bin" +if smb "get renamed.bin ${renback}" >/dev/null 2>&1 && [[ "$(md5 "${renback}")" == "$(md5 "${src}")" ]]; then + pass "renamed file readable with original content" +else + fail "renamed file readable with original content" +fi +if [[ -n "${SHARE_FS_PATH}" ]]; then + if [[ -f "${SHARE_FS_PATH}/renamed.bin" && ! -e "${SHARE_FS_PATH}/roundtrip.bin" ]]; then + pass "rename reflected on FUSE mount" + else + fail "rename reflected on FUSE mount" + fi +fi + +# 5. Large file (exercises SeaweedFS chunking) ------------------------------- +echo "==> 5. large file (SeaweedFS chunking)" +big="${WORK}/big.bin" +head -c 67108864 /dev/urandom >"${big}" # 64 MiB +bigback="${WORK}/big.back" +if smb "put ${big} big.bin" >/dev/null 2>&1 && + smb "get big.bin ${bigback}" >/dev/null 2>&1 && + [[ "$(md5 "${big}")" == "$(md5 "${bigback}")" ]]; then + pass "64 MiB put/get round-trip" +else + fail "64 MiB put/get round-trip" +fi + +# 6. Recursive upload -------------------------------------------------------- +echo "==> 6. recursive upload" +tree="${WORK}/tree" +mkdir -p "${tree}/a/b" +echo one >"${tree}/f1.txt" +echo two >"${tree}/a/f2.txt" +echo three >"${tree}/a/b/f3.txt" +if (cd "${WORK}" && smb "recurse ON; prompt OFF; mput tree" >/dev/null 2>&1) && + { [[ -z "${SHARE_FS_PATH}" ]] || [[ -f "${SHARE_FS_PATH}/tree/a/b/f3.txt" ]]; }; then + pass "recursive mput" +else + fail "recursive mput" +fi + +# 7. Cross-protocol read (FUSE writes, SMB reads) ---------------------------- +if [[ -n "${SHARE_FS_PATH}" ]]; then + echo "==> 7. cross-protocol read (FUSE write -> SMB read)" + echo "written-via-fuse" >"${SHARE_FS_PATH}/from_fuse.txt" + cpb="${WORK}/from_fuse.back" + if smb "get from_fuse.txt ${cpb}" >/dev/null 2>&1 && grep -q written-via-fuse "${cpb}"; then + pass "FUSE-written file readable over SMB" + else + fail "FUSE-written file readable over SMB" + fi +fi + +# 8. Delete ------------------------------------------------------------------ +echo "==> 8. delete" +smb "del renamed.bin" >/dev/null 2>&1 +smb "del big.bin" >/dev/null 2>&1 +smb "deltree docs" >/dev/null 2>&1 +smb "deltree tree" >/dev/null 2>&1 +if [[ -n "${SHARE_FS_PATH}" ]]; then + if [[ ! -e "${SHARE_FS_PATH}/renamed.bin" && ! -e "${SHARE_FS_PATH}/big.bin" && + ! -e "${SHARE_FS_PATH}/docs" && ! -e "${SHARE_FS_PATH}/tree" ]]; then + pass "delete files and directory trees" + else + fail "delete files and directory trees" + fi +else + if ! smb "get renamed.bin /dev/null" >/dev/null 2>&1; then + pass "deleted file no longer retrievable" + else + fail "deleted file no longer retrievable" + fi +fi + +echo +echo "==> Summary: ${PASS} passed, ${FAIL} failed" +[[ "${FAIL}" -eq 0 ]] diff --git a/weed/cluster/lock_client.go b/weed/cluster/lock_client.go index d1ce242f6..d84f51bef 100644 --- a/weed/cluster/lock_client.go +++ b/weed/cluster/lock_client.go @@ -11,7 +11,6 @@ import ( "github.com/seaweedfs/seaweedfs/weed/glog" "github.com/seaweedfs/seaweedfs/weed/pb" "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" - "github.com/seaweedfs/seaweedfs/weed/util" "google.golang.org/grpc" ) @@ -37,6 +36,7 @@ type LiveLock struct { expireAtNs int64 hostFiler pb.ServerAddress cancelCh chan struct{} + renewalDone chan struct{} // closed when the renewal goroutine exits; nil if there is none grpcDialOption grpc.DialOption isLocked int32 // 0 = unlocked, 1 = locked; use atomic operations self string @@ -83,7 +83,9 @@ func (lc *LockClient) NewBlockingLongLivedLock(key, owner string, lockTTL time.D // Block until acquired lock.retryUntilLocked(lockTTL) // Start renewal goroutine using a ticker for interruptible sleep + lock.renewalDone = make(chan struct{}) go func() { + defer close(lock.renewalDone) renewInterval := lockTTL / 2 ticker := time.NewTicker(renewInterval) defer ticker.Stop() @@ -119,7 +121,9 @@ func (lc *LockClient) StartLongLivedLock(key string, owner string, onLockOwnerCh if lock.lockTTL == 0 { lock.lockTTL = lock_manager.LiveLockTTL } + lock.renewalDone = make(chan struct{}) go func() { + defer close(lock.renewalDone) renewInterval := lock.lockTTL / 2 isLocked := false lockOwner := "" @@ -149,30 +153,39 @@ func (lc *LockClient) StartLongLivedLock(key string, owner string, onLockOwnerCh onLockOwnerChange(lock.LockOwner()) lockOwner = lock.LockOwner() } + // Sleep until the next attempt, but wake immediately on Stop() so + // the goroutine exits and closes renewalDone before Stop()'s bounded + // wait elapses. An uninterruptible sleep here (up to 5*renewInterval + // when unlocked) can outlast that wait and break the shutdown + // synchronization. + sleepFor := renewInterval + if !isLocked { + sleepFor = 5 * renewInterval + } + timer := time.NewTimer(sleepFor) select { case <-lock.cancelCh: + timer.Stop() return - default: - if isLocked { - time.Sleep(renewInterval) - } else { - time.Sleep(5 * renewInterval) - } + case <-timer.C: } } }() return } +// retryUntilLocked blocks until the lock is acquired, polling at the steady +// short cadence that AttemptToLock already enforces on contention (~1s). It +// deliberately avoids util.RetryUntil's exponential backoff (which grows to +// several seconds): when a holder on another mount releases the lock, the +// waiter must pick it up promptly, otherwise cross-mount write handoff stalls +// long enough to time out clients. func (lock *LiveLock) retryUntilLocked(lockDuration time.Duration) { - util.RetryUntil("create lock:"+lock.key, func() error { - return lock.AttemptToLock(lockDuration) - }, func(err error) (shouldContinue bool) { - if err != nil { - glog.Warningf("create lock %s: %s", lock.key, err) + for lock.renewToken == "" { + if err := lock.AttemptToLock(lockDuration); err != nil { + glog.V(1).Infof("create lock %s: %v", lock.key, err) } - return lock.renewToken == "" - }) + } } func (lock *LiveLock) AttemptToLock(lockDuration time.Duration) error { @@ -226,10 +239,27 @@ func (lock *LiveLock) Stop() error { close(lock.cancelCh) } - // Wait a brief moment for the goroutine to see the closed channel - // This reduces the race condition window where the goroutine might - // attempt one more lock operation after we've released the lock - time.Sleep(10 * time.Millisecond) + // Wait for the renewal goroutine to fully exit before unlocking. A renewal + // in flight when we close cancelCh rotates renewToken on the server; if we + // then unlock with the token we read here, the unlock fails with a token + // mismatch and the lock lingers until its TTL expires — blocking other + // mounts waiting on the same file. Waiting for the goroutine to return also + // makes the renewToken read below race-free (channel close = happens-before). + if lock.renewalDone != nil { + select { + case <-lock.renewalDone: + case <-time.After(lock.lockTTL + 2*time.Second): + // The renewal goroutine is wedged, almost certainly in a stuck + // renewal RPC. Do not unlock here: the renewToken may be rotated + // when that RPC finally returns, so an unlock sent now could race + // it, be rejected on a stale token, and leave the lock lingering + // anyway. cancelCh is closed, so the goroutine stops renewing once + // its in-flight call returns and the lock then expires within its + // TTL on its own. + glog.Warningf("lock %s: renewal goroutine still running at shutdown; letting lock expire via TTL", lock.key) + return nil + } + } // Also release the lock if held // Note: We intentionally don't clear renewToken here because