Skip to content

Commit

Permalink
Revert "alts: Queue ALTS handshakes once limit is reached rather than…
Browse files Browse the repository at this point in the history
… dropping. (#6884)" (#6903)

This reverts commit adc7685.
  • Loading branch information
matthewstevenson88 committed Dec 28, 2023
1 parent 4f03f3f commit 71cc0f1
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 11 deletions.
2 changes: 1 addition & 1 deletion credentials/alts/alts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
10 changes: 6 additions & 4 deletions credentials/alts/internal/handshaker/handshaker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down
12 changes: 6 additions & 6 deletions credentials/alts/internal/handshaker/handshaker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 _, <nil>", err)
if err := <-errc; err != nil && err != errDropped {
t.Errorf("ClientHandshake() = _, %v, want _, <nil> or %v", err, errDropped)
}
}

Expand Down Expand Up @@ -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 _, <nil>", err)
if err := <-errc; err != nil && err != errDropped {
t.Errorf("ServerHandshake() = _, %v, want _, <nil> or %v", err, errDropped)
}
}

Expand Down

0 comments on commit 71cc0f1

Please sign in to comment.