spanner: ReadWriteTransaction reties on io.ErrUnexpectedEOF

[ashi009]

ReadWriteTransaction function returned io.ErrUnexpectedEOF will cause spanner client to retry the transaction repeatedly.

_, err := client.ReadWriteTransaction(ctx, func(ctx context.Context, txn *spanner.ReadWriteTransaction) error {
    var b []byte
    row, err := txn.ReadRow(ctx, "Accounts", spanner.Key{"alice"}, []string{"proto"})
    if err != nil {
        return err
    }
    if err := row.Column(0, &b); err != nil {
        return err
    }
 	var msg pb.Message
	if err := proto.Unmarshal(b, &msg); err != nil {
        return err // which might be io.ErrUnexpectedEOF
    }
	...
    return nil
})

Even wrapping the returned error will cause it to retry, due to the way isRetryable works:

google-cloud-go/spanner/retry.go

Lines 77 to 94 in deb0013

	func isErrorUnexpectedEOF(err error) bool {
	if err == nil {
	return false
	}
	// Unexpected EOF is a transport layer issue that could be recovered by
	// retries. The most likely scenario is a flaky RecvMsg() call due to
	// network issues.
	// For grpc version >= 1.14.0, the error code is Internal.
	// (https://github.com/grpc/grpc-go/releases/tag/v1.14.0)
	if ErrCode(err) == codes.Internal && strings.Contains(ErrDesc(err), "unexpected EOF") {
	return true
	}
	// For grpc version < 1.14.0, the error code in Unknown.
	if ErrCode(err) == codes.Unknown && strings.Contains(ErrDesc(err), "unexpected EOF") {
	return true
	}
	return false
	}

[jeanbza]

Could you provide a little more information? Which method returned io.ErrUnexpectedEOF, and why should this not be retried?

[ashi009]

porto.Unmarshal might. It's necessary to do so within an RWT to manipulate it atomically if the data stored in spanner is a proto blob.

[olavloite]

@ashi009 Wrapping the error in a custom error with a custom error message should work. Running the following test works for me, in the sense that the transaction does not retry but fails with the custom error:

func TestReadWriteTransactionCustomError(t *testing.T) {
// setup test client
....

  _, err = client.ReadWriteTransaction(ctx,
      func(ctx context.Context, transaction *spanner.ReadWriteTransaction) error {
    _, err = unmarshal()
    if err == io.ErrUnexpectedEOF {
      err = &CustomError{
        Description:"Custom error",
        Err:io.ErrUnexpectedEOF,
      }
    }
    if err != nil {
      return err
    }
    return nil
  })
  if err == nil {
    t.Fatalf("missing exception")
  }
  if err.Error() != "Custom error" {
    t.Fatalf("unexpected exception: %s, expected %s", err.Error(), "Custom error")
  }
  e, ok := err.(*CustomError)
  if !ok {
    t.Fatalf("expected CustomError")
  }
  if e.Err != io.ErrUnexpectedEOF {
    t.Fatalf("expected io.ErrUnexpectedEOF as underlying error")
  }
}

type CustomError struct {
  Description string
  Err error
}

func (e *CustomError) Error() string {
  return e.Description
}

func unmarshal() (int, error) {
  return 0, io.ErrUnexpectedEOF
}

Running the same test without wrapping io.ErrUnexpectedEOF with a custom error message does indeed cause the transaction to retry indefinitely. That is however as expected, as this specific error is treated by the Spanner library as a retryable error code because it is an error that might happen at the transport level of gRPC.

[ashi009]

@olavloite This is indeed a working solution. However, it's unintuitive, as the convention for wrapping an error is to prepend the context. In that case, the "unexpected EOF" message will always present in the final error message and trigger the retry.

[broady]

Might want to remove the bottom part of this retry logic, which seems obsolete:

google-cloud-go/spanner/retry.go

Lines 87 to 95 in 844a547

	// For grpc version >= 1.14.0, the error code is Internal.
	// (https://github.com/grpc/grpc-go/releases/tag/v1.14.0)
	if ErrCode(err) == codes.Internal && strings.Contains(ErrDesc(err), "unexpected EOF") {
	return true
	}
	// For grpc version < 1.14.0, the error code in Unknown.
	if ErrCode(err) == codes.Unknown && strings.Contains(ErrDesc(err), "unexpected EOF") {
	return true
	}

[ashi009]

1.14 was released last August, any prior version is more than 6 months old. If this project is subject to the build horizon policy (maybe not for an open source project?), removing it SGTM. Chris Broadfoot <notifications@github.com>于2019年5月3日 周五上午1:00写道：

Might want to remove this retry logic, which seems obsolete: https://github.com/googleapis/google-cloud-go/blob/844a54768fdb2daa81cfafe91355d68439698b3d/spanner/retry.go#L92-L95 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1384 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAFH2B6KXI647SRE3R32AHTPTMM2RANCNFSM4HEPTX4A> .

-- Sent from Gmail Mobile

[olavloite]

This issue was also solved with the change mentioned above.