From fcc7bd04c66aa56bf6de9c6ffa4aad42e3f8ec5f Mon Sep 17 00:00:00 2001 From: "user.email" Date: Mon, 26 Oct 2026 17:38:19 -0700 Subject: [PATCH] cluster: bound each DistributedLock attempt A lock-ring member reachable from the masters but not from this filer makes one DistributedLock attempt wait out the TCP connect timeout, so a write whose key hashes to it stalls far past noLockServerRetryPeriod before failing. Give the attempt a 10s context deadline so the retry window can take effect; a timed-out attempt keeps no renew token, so the lock server still grants the key to at most one owner. --- weed/cluster/lock_client.go | 14 +++++++++- weed/cluster/lock_client_test.go | 47 ++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/weed/cluster/lock_client.go b/weed/cluster/lock_client.go index 9d14f383f2..7d3aa6d811 100644 --- a/weed/cluster/lock_client.go +++ b/weed/cluster/lock_client.go @@ -45,6 +45,12 @@ type LockClient struct { // snapshot) but short enough that a write fails instead of hanging // forever. noLockServerRetryPeriod time.Duration + + // lockRpcTimeout bounds a single DistributedLock attempt: a ring member + // reachable from the masters but not from this filer would otherwise + // block an attempt on the TCP connect timeout, far past + // noLockServerRetryPeriod. + lockRpcTimeout time.Duration } func NewLockClient(grpcDialOption grpc.DialOption, seedFiler pb.ServerAddress) *LockClient { @@ -55,6 +61,7 @@ func NewLockClient(grpcDialOption grpc.DialOption, seedFiler pb.ServerAddress) * seedFiler: seedFiler, priorWindow: 5 * time.Second, noLockServerRetryPeriod: 15 * time.Second, + lockRpcTimeout: 10 * time.Second, } } @@ -417,7 +424,12 @@ func (lock *LiveLock) doLock(lockDuration time.Duration) (errorMessage string, e previousOwner := lock.owner err = pb.WithFilerClient(false, 0, lock.hostFiler, lock.grpcDialOption, func(client filer_pb.SeaweedFilerClient) error { - resp, err := client.DistributedLock(context.Background(), &filer_pb.LockRequest{ + // Bound the attempt so an unreachable owner fails within the retry + // window. A timed-out attempt keeps no renew token, so the lock + // server still grants the key to at most one owner. + ctx, cancel := context.WithTimeout(context.Background(), lock.lc.lockRpcTimeout) + defer cancel() + resp, err := client.DistributedLock(ctx, &filer_pb.LockRequest{ Name: lock.key, SecondsToLock: int64(lockDuration.Seconds()), RenewToken: lock.renewToken, diff --git a/weed/cluster/lock_client_test.go b/weed/cluster/lock_client_test.go index 7a6a89950f..5843cce0b5 100644 --- a/weed/cluster/lock_client_test.go +++ b/weed/cluster/lock_client_test.go @@ -286,6 +286,53 @@ func TestNewShortLivedLockFailsFastOnUnreachableFiler(t *testing.T) { } } +type hangingLockFiler struct { + filer_pb.UnimplementedSeaweedFilerServer +} + +func (s *hangingLockFiler) DistributedLock(ctx context.Context, req *filer_pb.LockRequest) (*filer_pb.LockResponse, error) { + select { + case <-ctx.Done(): + return nil, ctx.Err() + case <-time.After(30 * time.Second): + return nil, fmt.Errorf("test: server never answered") + } +} + +// A ring member that accepts connections but never answers stalls the caller +// the same way a partially partitioned peer does: each attempt must fail +// within the per-attempt bound so the retry window can give up. +func TestNewShortLivedLockBoundsHangingAttempt(t *testing.T) { + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + grpcServer := grpc.NewServer() + filer_pb.RegisterSeaweedFilerServer(grpcServer, &hangingLockFiler{}) + go grpcServer.Serve(listener) + defer grpcServer.Stop() + + dialOption := grpc.WithTransportCredentials(insecure.NewCredentials()) + host, port, err := net.SplitHostPort(listener.Addr().String()) + if err != nil { + t.Fatalf("split host port: %v", err) + } + lc := NewLockClient(dialOption, pb.ServerAddress(fmt.Sprintf("%s:0.%s", host, port))) + lc.noLockServerRetryPeriod = 200 * time.Millisecond + lc.lockRpcTimeout = 50 * time.Millisecond + + start := time.Now() + lock := lc.NewShortLivedLock("test-key", "test-owner") + elapsed := time.Since(start) + + if lock != nil { + t.Fatal("expected nil lock when the ring member never answers") + } + if elapsed > 5*time.Second { + t.Fatalf("lock acquisition took %v, expected it bounded by the per-attempt timeout", elapsed) + } +} + // LiveLock.generation is a fencing token written and read with 64-bit atomic // operations. On 32-bit platforms (GOARCH=386 and GOARCH=arm) a 64-bit atomic // op requires an 8-byte-aligned address, which Go only guarantees for the