From 71cc0f167537667a878ec5995cf8167c69c362df Mon Sep 17 00:00:00 2001 From: Matthew Stevenson <52979934+matthewstevenson88@users.noreply.github.com> Date: Thu, 28 Dec 2023 14:33:59 -0800 Subject: [PATCH] Revert "alts: Queue ALTS handshakes once limit is reached rather than dropping. (#6884)" (#6903) This reverts commit adc76852e0971c76dd470e5e629360aeea412d55. --- credentials/alts/alts_test.go | 2 +- credentials/alts/internal/handshaker/handshaker.go | 10 ++++++---- .../alts/internal/handshaker/handshaker_test.go | 12 ++++++------ 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/credentials/alts/alts_test.go b/credentials/alts/alts_test.go index d693541f15e3..20062fe77539 100644 --- a/credentials/alts/alts_test.go +++ b/credentials/alts/alts_test.go @@ -397,7 +397,7 @@ func establishAltsConnection(t *testing.T, handshakerAddress, serverAddress stri if err == nil { break } - if code := status.Code(err); code == codes.Unavailable || code == codes.DeadlineExceeded { + if code := status.Code(err); code == codes.Unavailable { // The server is not ready yet. Try again. continue } diff --git a/credentials/alts/internal/handshaker/handshaker.go b/credentials/alts/internal/handshaker/handshaker.go index 6c867dd85015..64890797f30d 100644 --- a/credentials/alts/internal/handshaker/handshaker.go +++ b/credentials/alts/internal/handshaker/handshaker.go @@ -61,6 +61,8 @@ var ( // control number of concurrent created (but not closed) handshakes. clientHandshakes = semaphore.NewWeighted(int64(envconfig.ALTSMaxConcurrentHandshakes)) serverHandshakes = semaphore.NewWeighted(int64(envconfig.ALTSMaxConcurrentHandshakes)) + // errDropped occurs when maxPendingHandshakes is reached. + errDropped = errors.New("maximum number of concurrent ALTS handshakes is reached") // errOutOfBound occurs when the handshake service returns a consumed // bytes value larger than the buffer that was passed to it originally. errOutOfBound = errors.New("handshaker service consumed bytes value is out-of-bound") @@ -154,8 +156,8 @@ func NewServerHandshaker(ctx context.Context, conn *grpc.ClientConn, c net.Conn, // ClientHandshake starts and completes a client ALTS handshake for GCP. Once // done, ClientHandshake returns a secure connection. func (h *altsHandshaker) ClientHandshake(ctx context.Context) (net.Conn, credentials.AuthInfo, error) { - if err := clientHandshakes.Acquire(ctx, 1); err != nil { - return nil, nil, err + if !clientHandshakes.TryAcquire(1) { + return nil, nil, errDropped } defer clientHandshakes.Release(1) @@ -207,8 +209,8 @@ func (h *altsHandshaker) ClientHandshake(ctx context.Context) (net.Conn, credent // ServerHandshake starts and completes a server ALTS handshake for GCP. Once // done, ServerHandshake returns a secure connection. func (h *altsHandshaker) ServerHandshake(ctx context.Context) (net.Conn, credentials.AuthInfo, error) { - if err := serverHandshakes.Acquire(ctx, 1); err != nil { - return nil, nil, err + if !serverHandshakes.TryAcquire(1) { + return nil, nil, errDropped } defer serverHandshakes.Release(1) diff --git a/credentials/alts/internal/handshaker/handshaker_test.go b/credentials/alts/internal/handshaker/handshaker_test.go index 9f877c48dcf3..956e0fbd1c20 100644 --- a/credentials/alts/internal/handshaker/handshaker_test.go +++ b/credentials/alts/internal/handshaker/handshaker_test.go @@ -193,10 +193,10 @@ func (s) TestClientHandshake(t *testing.T) { }() } - // Ensure that there are no errors. + // Ensure all errors are expected. for i := 0; i < testCase.numberOfHandshakes; i++ { - if err := <-errc; err != nil { - t.Errorf("ClientHandshake() = _, %v, want _, ", err) + if err := <-errc; err != nil && err != errDropped { + t.Errorf("ClientHandshake() = _, %v, want _, or %v", err, errDropped) } } @@ -250,10 +250,10 @@ func (s) TestServerHandshake(t *testing.T) { }() } - // Ensure that there are no errors. + // Ensure all errors are expected. for i := 0; i < testCase.numberOfHandshakes; i++ { - if err := <-errc; err != nil { - t.Errorf("ServerHandshake() = _, %v, want _, ", err) + if err := <-errc; err != nil && err != errDropped { + t.Errorf("ServerHandshake() = _, %v, want _, or %v", err, errDropped) } }