spanner: remove the support for resource-based routing
The Spanner team has decided to implement this functionality on the
server side so we no longer need to add this support on the client
side.
Change-Id: I7d8665710252ff5002c1dc8bae0e787a1ccdc66a
Reviewed-on: https://code-review.googlesource.com/c/gocloud/+/55238
Reviewed-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Cody Oss <codyoss@google.com>
Reviewed-by: Knut Olav Løite <koloite@gmail.com>
diff --git a/spanner/client.go b/spanner/client.go
index 7ce2f25..6095741 100644
--- a/spanner/client.go
+++ b/spanner/client.go
@@ -25,16 +25,12 @@
"time"
"cloud.google.com/go/internal/trace"
- instance "cloud.google.com/go/spanner/admin/instance/apiv1"
"google.golang.org/api/option"
gtransport "google.golang.org/api/transport/grpc"
- instancepb "google.golang.org/genproto/googleapis/spanner/admin/instance/v1"
sppb "google.golang.org/genproto/googleapis/spanner/v1"
- field_mask "google.golang.org/genproto/protobuf/field_mask"
"google.golang.org/grpc"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/metadata"
- "google.golang.org/grpc/status"
)
const (
@@ -54,8 +50,7 @@
)
var (
- validDBPattern = regexp.MustCompile("^projects/(?P<project>[^/]+)/instances/(?P<instance>[^/]+)/databases/(?P<database>[^/]+)$")
- validInstancePattern = regexp.MustCompile("^projects/(?P<project>[^/]+)/instances/(?P<instance>[^/]+)")
+ validDBPattern = regexp.MustCompile("^projects/(?P<project>[^/]+)/instances/(?P<instance>[^/]+)/databases/(?P<database>[^/]+)$")
)
func validDatabaseName(db string) error {
@@ -66,15 +61,6 @@
return nil
}
-func getInstanceName(db string) (string, error) {
- matches := validInstancePattern.FindStringSubmatch(db)
- if len(matches) == 0 {
- return "", fmt.Errorf("Failed to retrieve instance name from %q according to pattern %q",
- db, validInstancePattern.String())
- }
- return matches[0], nil
-}
-
func parseDatabaseName(db string) (project, instance, database string, err error) {
matches := validDBPattern.FindStringSubmatch(db)
if len(matches) == 0 {
@@ -135,42 +121,6 @@
return metadata.NewOutgoingContext(ctx, md)
}
-// getInstanceEndpoint returns an instance-specific endpoint if one exists. If
-// multiple endpoints exist, it returns the first one.
-func getInstanceEndpoint(ctx context.Context, database string, opts ...option.ClientOption) (string, error) {
- instanceName, err := getInstanceName(database)
- if err != nil {
- return "", fmt.Errorf("Failed to resolve endpoint: %v", err)
- }
-
- c, err := instance.NewInstanceAdminClient(ctx, opts...)
- if err != nil {
- return "", err
- }
- defer c.Close()
-
- req := &instancepb.GetInstanceRequest{
- Name: instanceName,
- FieldMask: &field_mask.FieldMask{
- Paths: []string{"endpoint_uris"},
- },
- }
-
- resp, err := c.GetInstance(ctx, req)
- if err != nil {
- return "", err
- }
-
- endpointURIs := resp.GetEndpointUris()
-
- if len(endpointURIs) > 0 {
- return endpointURIs[0], nil
- }
-
- // Return empty string when no endpoints exist.
- return "", nil
-}
-
// NewClient creates a client to a database. A valid database name has the
// form projects/PROJECT_ID/instances/INSTANCE_ID/databases/DATABASE_ID. It uses
// a default configuration.
@@ -197,32 +147,6 @@
option.WithoutAuthentication(),
}
opts = append(emulatorOpts, opts...)
- } else if os.Getenv("GOOGLE_CLOUD_SPANNER_ENABLE_RESOURCE_BASED_ROUTING") == "true" {
- // Fetch the instance-specific endpoint.
- reqOpts := []option.ClientOption{option.WithEndpoint(endpoint)}
- reqOpts = append(reqOpts, opts...)
- instanceEndpoint, err := getInstanceEndpoint(ctx, database, reqOpts...)
-
- if err != nil {
- // If there is a PermissionDenied error, fall back to use the global endpoint
- // or the user-specified endpoint.
- if status.Code(err) == codes.PermissionDenied {
- logf(config.logger, `
-Warning: The client library attempted to connect to an endpoint closer to your
-Cloud Spanner data but was unable to do so. The client library will fall back
-and route requests to the endpoint given in the client options, which may
-result in increased latency. We recommend including the scope
-https://www.googleapis.com/auth/spanner.admin so that the client library can
-get an instance-specific endpoint and efficiently route requests.
-`)
- } else {
- return nil, err
- }
- }
-
- if instanceEndpoint != "" {
- opts = append(opts, option.WithEndpoint(instanceEndpoint))
- }
}
// Prepare gRPC channels.
diff --git a/spanner/client_test.go b/spanner/client_test.go
index 613d443..1c9fed2 100644
--- a/spanner/client_test.go
+++ b/spanner/client_test.go
@@ -20,8 +20,6 @@
"context"
"fmt"
"io"
- "io/ioutil"
- "log"
"os"
"strings"
"testing"
@@ -30,11 +28,9 @@
"cloud.google.com/go/civil"
itestutil "cloud.google.com/go/internal/testutil"
. "cloud.google.com/go/spanner/internal/testutil"
- "github.com/golang/protobuf/proto"
structpb "github.com/golang/protobuf/ptypes/struct"
"google.golang.org/api/iterator"
"google.golang.org/api/option"
- instancepb "google.golang.org/genproto/googleapis/spanner/admin/instance/v1"
sppb "google.golang.org/genproto/googleapis/spanner/v1"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
@@ -102,34 +98,6 @@
}
}
-// Test getInstanceName()
-func TestGetInstanceName(t *testing.T) {
- validDbURI := "projects/spanner-cloud-test/instances/foo/databases/foodb"
- invalidDbUris := []string{
- // Completely wrong DB URI.
- "foobarDB",
- // Project ID contains "/".
- "projects/spanner-cloud/test/instances/foo/databases/foodb",
- // No instance ID.
- "projects/spanner-cloud-test/instances//databases/foodb",
- }
- want := "projects/spanner-cloud-test/instances/foo"
- got, err := getInstanceName(validDbURI)
- if err != nil {
- t.Errorf("getInstanceName(%q) has an error: %q, want nil", validDbURI, err)
- }
- if got != want {
- t.Errorf("getInstanceName(%q) = %q, want %q", validDbURI, got, want)
- }
- for _, d := range invalidDbUris {
- wantErr := "Failed to retrieve instance name"
- _, err = getInstanceName(d)
- if !strings.Contains(err.Error(), wantErr) {
- t.Errorf("getInstanceName(%q) has an error: %q, want error pattern %q", validDbURI, err, wantErr)
- }
- }
-}
-
func TestReadOnlyTransactionClose(t *testing.T) {
// Closing a ReadOnlyTransaction shouldn't panic.
c := &Client{}
@@ -353,157 +321,6 @@
}
}
-func TestClient_ResourceBasedRouting_WithEndpointsReturned(t *testing.T) {
- os.Setenv("GOOGLE_CLOUD_SPANNER_ENABLE_RESOURCE_BASED_ROUTING", "true")
- defer os.Setenv("GOOGLE_CLOUD_SPANNER_ENABLE_RESOURCE_BASED_ROUTING", "")
-
- // Create two servers. The base server receives the GetInstance request and
- // returns the instance endpoint of the target server. The client should contact
- // the target server after getting the instance endpoint.
- serverBase, optsBase, serverTeardownBase := NewMockedSpannerInMemTestServerWithAddr(t, "localhost:8081")
- defer serverTeardownBase()
- serverTarget, optsTarget, serverTeardownTarget := NewMockedSpannerInMemTestServerWithAddr(t, "localhost:8082")
- defer serverTeardownTarget()
-
- // Return the instance endpoint.
- instanceEndpoint := fmt.Sprintf("%s", optsTarget[0])
- resps := []proto.Message{&instancepb.Instance{
- EndpointUris: []string{instanceEndpoint},
- }}
- serverBase.TestInstanceAdmin.SetResps(resps)
-
- ctx := context.Background()
- formattedDatabase := fmt.Sprintf("projects/%s/instances/%s/databases/%s", "some-project", "some-instance", "some-database")
- client, err := NewClientWithConfig(ctx, formattedDatabase, ClientConfig{}, optsBase...)
- if err != nil {
- t.Fatal(err)
- }
-
- if err := executeSingerQuery(ctx, client.Single()); err != nil {
- t.Fatal(err)
- }
-
- // The base server should not receive any requests.
- if _, err := shouldHaveReceived(serverBase.TestSpanner, []interface{}{}); err != nil {
- t.Fatal(err)
- }
-
- // The target server should receive requests.
- if _, err = shouldHaveReceived(serverTarget.TestSpanner, []interface{}{
- &sppb.BatchCreateSessionsRequest{},
- &sppb.ExecuteSqlRequest{},
- }); err != nil {
- t.Fatal(err)
- }
-}
-
-func TestClient_ResourceBasedRouting_WithoutEndpointsReturned(t *testing.T) {
- os.Setenv("GOOGLE_CLOUD_SPANNER_ENABLE_RESOURCE_BASED_ROUTING", "true")
- defer os.Setenv("GOOGLE_CLOUD_SPANNER_ENABLE_RESOURCE_BASED_ROUTING", "")
-
- server, opts, serverTeardown := NewMockedSpannerInMemTestServer(t)
- defer serverTeardown()
-
- // Return an empty list of endpoints.
- resps := []proto.Message{&instancepb.Instance{
- EndpointUris: []string{},
- }}
- server.TestInstanceAdmin.SetResps(resps)
-
- ctx := context.Background()
- formattedDatabase := fmt.Sprintf("projects/%s/instances/%s/databases/%s", "some-project", "some-instance", "some-database")
- client, err := NewClientWithConfig(ctx, formattedDatabase, ClientConfig{}, opts...)
- if err != nil {
- t.Fatal(err)
- }
-
- if err := executeSingerQuery(ctx, client.Single()); err != nil {
- t.Fatal(err)
- }
-
- // Check if the request goes to the default endpoint.
- if _, err := shouldHaveReceived(server.TestSpanner, []interface{}{
- &sppb.BatchCreateSessionsRequest{},
- &sppb.ExecuteSqlRequest{},
- }); err != nil {
- t.Fatal(err)
- }
-}
-
-func TestClient_ResourceBasedRouting_WithPermissionDeniedError(t *testing.T) {
- os.Setenv("GOOGLE_CLOUD_SPANNER_ENABLE_RESOURCE_BASED_ROUTING", "true")
- defer os.Setenv("GOOGLE_CLOUD_SPANNER_ENABLE_RESOURCE_BASED_ROUTING", "")
-
- server, opts, serverTeardown := NewMockedSpannerInMemTestServer(t)
- defer serverTeardown()
-
- server.TestInstanceAdmin.SetErr(status.Error(codes.PermissionDenied, "Permission Denied"))
-
- ctx := context.Background()
- formattedDatabase := fmt.Sprintf("projects/%s/instances/%s/databases/%s", "some-project", "some-instance", "some-database")
- // `PermissionDeniedError` causes a warning message to be logged, which is expected.
- // We set the output to be discarded to avoid spamming the log.
- logger := log.New(ioutil.Discard, "", log.LstdFlags)
- client, err := NewClientWithConfig(ctx, formattedDatabase, ClientConfig{logger: logger}, opts...)
- if err != nil {
- t.Fatal(err)
- }
-
- if err := executeSingerQuery(ctx, client.Single()); err != nil {
- t.Fatal(err)
- }
-
- // Fallback to use the default endpoint when calling GetInstance() returns
- // a PermissionDenied error.
- if _, err := shouldHaveReceived(server.TestSpanner, []interface{}{
- &sppb.BatchCreateSessionsRequest{},
- &sppb.ExecuteSqlRequest{},
- }); err != nil {
- t.Fatal(err)
- }
-}
-
-func TestClient_ResourceBasedRouting_WithUnavailableError(t *testing.T) {
- os.Setenv("GOOGLE_CLOUD_SPANNER_ENABLE_RESOURCE_BASED_ROUTING", "true")
- defer os.Setenv("GOOGLE_CLOUD_SPANNER_ENABLE_RESOURCE_BASED_ROUTING", "")
-
- server, opts, serverTeardown := NewMockedSpannerInMemTestServer(t)
- defer serverTeardown()
-
- resps := []proto.Message{&instancepb.Instance{
- EndpointUris: []string{},
- }}
- server.TestInstanceAdmin.SetResps(resps)
- server.TestInstanceAdmin.SetErr(status.Error(codes.Unavailable, "Temporary unavailable"))
-
- ctx := context.Background()
- formattedDatabase := fmt.Sprintf("projects/%s/instances/%s/databases/%s", "some-project", "some-instance", "some-database")
- _, err := NewClientWithConfig(ctx, formattedDatabase, ClientConfig{}, opts...)
- // The first request will get an error and the server resets the error to nil,
- // so the next request will be fine. Due to retrying, there is no errors.
- if err != nil {
- t.Fatal(err)
- }
-}
-
-func TestClient_ResourceBasedRouting_WithInvalidArgumentError(t *testing.T) {
- os.Setenv("GOOGLE_CLOUD_SPANNER_ENABLE_RESOURCE_BASED_ROUTING", "true")
- defer os.Setenv("GOOGLE_CLOUD_SPANNER_ENABLE_RESOURCE_BASED_ROUTING", "")
-
- server, opts, serverTeardown := NewMockedSpannerInMemTestServer(t)
- defer serverTeardown()
-
- server.TestInstanceAdmin.SetErr(status.Error(codes.InvalidArgument, "Invalid argument"))
-
- ctx := context.Background()
- formattedDatabase := fmt.Sprintf("projects/%s/instances/%s/databases/%s", "some-project", "some-instance", "some-database")
- _, err := NewClientWithConfig(ctx, formattedDatabase, ClientConfig{}, opts...)
-
- if status.Code(err) != codes.InvalidArgument {
- t.Fatalf("got unexpected exception %v, expected InvalidArgument", err)
- }
-}
-
func TestClient_Single_QueryOptions(t *testing.T) {
for _, tt := range queryOptionsTestCases() {
t.Run(tt.name, func(t *testing.T) {
diff --git a/spanner/integration_test.go b/spanner/integration_test.go
index 77f22a4..3a647b5 100644
--- a/spanner/integration_test.go
+++ b/spanner/integration_test.go
@@ -492,50 +492,6 @@
}
}
-// Test resource-based routing enabled.
-func TestIntegration_SingleUse_WithResourceBasedRouting(t *testing.T) {
- os.Setenv("GOOGLE_CLOUD_SPANNER_ENABLE_RESOURCE_BASED_ROUTING", "true")
- defer os.Setenv("GOOGLE_CLOUD_SPANNER_ENABLE_RESOURCE_BASED_ROUTING", "")
-
- ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
- defer cancel()
- // Set up testing environment.
- client, _, cleanup := prepareIntegrationTest(ctx, t, DefaultSessionPoolConfig, singerDBStatements)
- defer cleanup()
-
- writes := []struct {
- row []interface{}
- ts time.Time
- }{
- {row: []interface{}{1, "Marc", "Foo"}},
- {row: []interface{}{2, "Tars", "Bar"}},
- {row: []interface{}{3, "Alpha", "Beta"}},
- {row: []interface{}{4, "Last", "End"}},
- }
- // Try to write four rows through the Apply API.
- for i, w := range writes {
- var err error
- m := InsertOrUpdate("Singers",
- []string{"SingerId", "FirstName", "LastName"},
- w.row)
- if writes[i].ts, err = client.Apply(ctx, []*Mutation{m}, ApplyAtLeastOnce()); err != nil {
- t.Fatal(err)
- }
- }
-
- row, err := client.Single().ReadRow(ctx, "Singers", Key{3}, []string{"FirstName"})
- if err != nil {
- t.Errorf("SingleUse.ReadRow returns error %v, want nil", err)
- }
- var got string
- if err := row.Column(0, &got); err != nil {
- t.Errorf("row.Column returns error %v, want nil", err)
- }
- if want := "Alpha"; got != want {
- t.Errorf("got %q, want %q", got, want)
- }
-}
-
// Test custom query options provided on query-level configuration.
func TestIntegration_SingleUse_WithQueryOptions(t *testing.T) {
skipEmulatorTest(t)
