Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore(bigtable): adds CloseClient() to test proxy #6861

Merged
merged 8 commits into from Oct 20, 2022
Merged

chore(bigtable): adds CloseClient() to test proxy #6861

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2eb7d80
chore(bigtable): adds CloseClient() to test proxy
telpirion Oct 13, 2022
e3ddd98
fix
telpirion Oct 13, 2022
594d8f9
fix per test suite
telpirion Oct 17, 2022
87cc148
Merge branch 'main' into bigtable-proxy-close
telpirion Oct 17, 2022
b5c9019
test fix
telpirion Oct 17, 2022
47786e6
Merge branch 'main' into bigtable-proxy-close
telpirion Oct 18, 2022
e3a0dca
per reviewer
telpirion Oct 20, 2022
7635616
Merge branch 'main' into bigtable-proxy-close
telpirion Oct 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions bigtable/go.mod
View file
Open in desktop
Expand Up @@ -7,13 +7,13 @@ require (
cloud.google.com/go/iam v0.3.0
github.com/golang/protobuf v1.5.2
github.com/google/btree v1.1.2
github.com/google/go-cmp v0.5.8
github.com/googleapis/cloud-bigtable-clients-test v0.0.0-20220824184156-0ba36e446d93
github.com/google/go-cmp v0.5.9
github.com/googleapis/cloud-bigtable-clients-test v0.0.0-20221012214650-1d7ae69b0110
github.com/googleapis/gax-go/v2 v2.5.1
golang.org/x/oauth2 v0.0.0-20220909003341-f21342109be1
google.golang.org/api v0.98.0
google.golang.org/genproto v0.0.0-20221010155953-15ba04fc1c0e
google.golang.org/grpc v1.49.0
google.golang.org/grpc v1.50.0
google.golang.org/protobuf v1.28.1
rsc.io/binaryregexp v0.2.0
)
Expand Down
11 changes: 6 additions & 5 deletions bigtable/go.sum
View file
Open in desktop
Expand Up @@ -149,8 +149,9 @@ github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/
github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
github.com/google/go-cmp v0.5.7/go.mod h1:n+brtR0CgQNWTVd5ZUFpTBC8YFBDLK/h/bpaJ8/DtOE=
github.com/google/go-cmp v0.5.8 h1:e6P7q2lk1O+qJJb4BtCQXlK8vWEO8V1ZeuEdJNOqZyg=
github.com/google/go-cmp v0.5.8/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
github.com/google/martian v2.1.0+incompatible/go.mod h1:9I4somxYTbIHy5NJKHRl3wXiIaQGbYVAs8BPL6v8lEs=
github.com/google/martian/v3 v3.0.0/go.mod h1:y5Zk1BBys9G+gd6Jrk0W3cC1+ELVxBWuIGO+w/tUAp0=
github.com/google/martian/v3 v3.1.0/go.mod h1:y5Zk1BBys9G+gd6Jrk0W3cC1+ELVxBWuIGO+w/tUAp0=
Expand All @@ -173,8 +174,8 @@ github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm4
github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I=
github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
github.com/googleapis/cloud-bigtable-clients-test v0.0.0-20220824184156-0ba36e446d93 h1:41iSCOKGDBsK3XBfc+78/TTKLMReSGObSzUdVS9OMAo=
github.com/googleapis/cloud-bigtable-clients-test v0.0.0-20220824184156-0ba36e446d93/go.mod h1:1lRdlEG7pxSh809itBaBjEC8eC8sXwdyjgc607IM+es=
github.com/googleapis/cloud-bigtable-clients-test v0.0.0-20221012214650-1d7ae69b0110 h1:7b5OjwaXZXLWlelmqKCp2pBBD0dgXwKawcTD54L1kKY=
github.com/googleapis/cloud-bigtable-clients-test v0.0.0-20221012214650-1d7ae69b0110/go.mod h1:EpIlz+Q8rera5LV8JXtACO1HbHg2W0PxL1wU2tJL0uY=
github.com/googleapis/enterprise-certificate-proxy v0.0.0-20220520183353-fd19c99a87aa/go.mod h1:17drOmN3MwGY7t0e+Ei9b45FFGA3fBs3x36SsCg1hq8=
github.com/googleapis/enterprise-certificate-proxy v0.1.0 h1:zO8WHNx/MYiAKJ3d5spxZXZE6KHmIQGQcAzwUzV7qQw=
github.com/googleapis/enterprise-certificate-proxy v0.1.0/go.mod h1:17drOmN3MwGY7t0e+Ei9b45FFGA3fBs3x36SsCg1hq8=
Expand Down Expand Up @@ -637,8 +638,8 @@ google.golang.org/grpc v1.45.0/go.mod h1:lN7owxKUQEqMfSyQikvvk5tf/6zMPsrK+ONuO11
google.golang.org/grpc v1.46.0/go.mod h1:vN9eftEi1UMyUsIF80+uQXhHjbXYbm0uXoFCACuMGWk=
google.golang.org/grpc v1.46.2/go.mod h1:vN9eftEi1UMyUsIF80+uQXhHjbXYbm0uXoFCACuMGWk=
google.golang.org/grpc v1.47.0/go.mod h1:vN9eftEi1UMyUsIF80+uQXhHjbXYbm0uXoFCACuMGWk=
google.golang.org/grpc v1.49.0 h1:WTLtQzmQori5FUH25Pq4WT22oCsv8USpQ+F6rqtsmxw=
google.golang.org/grpc v1.49.0/go.mod h1:ZgQEeidpAuNRZ8iRrlBKXZQP1ghovWIVhdJRyCDK+GI=
google.golang.org/grpc v1.50.0 h1:fPVVDxY9w++VjTZsYvXWqEf9Rqar/e+9zYfxKK+W+YU=
google.golang.org/grpc v1.50.0/go.mod h1:ZgQEeidpAuNRZ8iRrlBKXZQP1ghovWIVhdJRyCDK+GI=
google.golang.org/grpc/cmd/protoc-gen-go-grpc v1.1.0/go.mod h1:6Kw0yEErY5E/yWrBtf03jp27GLLJujG4z/JK95pnjjw=
google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8=
google.golang.org/protobuf v0.0.0-20200221191635-4d8936d0db64/go.mod h1:kwYJMbMJ01Woi6D6+Kah6886xMZcty6N08ah7+eCXa0=
Expand Down
45 changes: 38 additions & 7 deletions bigtable/internal/testproxy/proxy.go
View file
Open in desktop
Expand Up @@ -353,6 +353,7 @@ type testClient struct {
c *bigtable.Client // c stores the Bigtable client under test
appProfileID string // appProfileID is currently unused
perOperationTimeout *duration.Duration // perOperationTimeout sets a custom timeout for methods calls on this client
isOpen bool // isOpen indicates whether this client is open for new requests
}

// credentialsBundle implements credentials.Bundle interface
Expand Down Expand Up @@ -493,7 +494,17 @@ type goTestProxyServer struct {
pb.UnimplementedCloudBigtableV2TestProxyServer
clientsLock sync.RWMutex // clientsLock prevents simultaneous mutation of the clientIDs map
clientIDs map[string]*testClient // clientIDs has all of the bigtable.Client objects under test
}

func (s *goTestProxyServer) client(clientID string) (*testClient, bool) {
telpirion marked this conversation as resolved.
Show resolved Hide resolved
client, ok := s.clientIDs[clientID]
if !ok {
return nil, false
}
if !client.isOpen {
return nil, false
}
return client, true
}

// CreateClient responds to the CreateClient RPC. This method adds a new client
Expand Down Expand Up @@ -536,11 +547,29 @@ func (s *goTestProxyServer) CreateClient(ctx context.Context, req *pb.CreateClie
c: c,
appProfileID: req.AppProfileId,
perOperationTimeout: req.PerOperationTimeout,
isOpen: true,
}

return &pb.CreateClientResponse{}, nil
}

// CloseClient responds to the CloseClient RPC. This method closes an existing
// client, making it inaccessible to new requests.
func (s *goTestProxyServer) CloseClient(ctx context.Context, req *pb.CloseClientRequest) (*pb.CloseClientResponse, error) {
clientID := req.ClientId
s.clientsLock.Lock()
defer s.clientsLock.Unlock()

btc, exists := s.client(clientID)
if !exists {
return nil, stat.Error(codes.InvalidArgument,
fmt.Sprintf("%s: ClientID does not exist", logLabel))
}
btc.isOpen = false

return &pb.CloseClientResponse{}, nil
}

// RemoveClient responds to the RemoveClient RPC. This method removes an
// existing client from the goTestProxyServer
func (s *goTestProxyServer) RemoveClient(ctx context.Context, req *pb.RemoveClientRequest) (*pb.RemoveClientResponse, error) {
Expand All @@ -549,13 +578,15 @@ func (s *goTestProxyServer) RemoveClient(ctx context.Context, req *pb.RemoveClie
s.clientsLock.Lock()
defer s.clientsLock.Unlock()

// RemoveClient can ignore whether the client accepts new requests
btc, exists := s.clientIDs[clientID]
if !exists {
return nil, stat.Error(codes.InvalidArgument,
fmt.Sprintf("%s: ClientID does not exist", logLabel))
}

// this closes every ClientConn in the pool.
btc.isOpen = false
btc.c.Close()
delete(s.clientIDs, clientID)

Expand All @@ -566,7 +597,7 @@ func (s *goTestProxyServer) RemoveClient(ctx context.Context, req *pb.RemoveClie
// data for a single row in the Table.
func (s *goTestProxyServer) ReadRow(ctx context.Context, req *pb.ReadRowRequest) (*pb.RowResult, error) {
s.clientsLock.RLock()
btc, exists := s.clientIDs[req.ClientId]
btc, exists := s.client(req.ClientId)
s.clientsLock.RUnlock()

if !exists {
Expand Down Expand Up @@ -612,7 +643,7 @@ func (s *goTestProxyServer) ReadRow(ctx context.Context, req *pb.ReadRowRequest)
// data for a set of rows, a range of rows, or the entire table.
func (s *goTestProxyServer) ReadRows(ctx context.Context, req *pb.ReadRowsRequest) (*pb.RowsResult, error) {
s.clientsLock.RLock()
btc, exists := s.clientIDs[req.ClientId]
btc, exists := s.client(req.ClientId)
s.clientsLock.RUnlock()

if !exists {
Expand Down Expand Up @@ -685,7 +716,7 @@ func (s *goTestProxyServer) ReadRows(ctx context.Context, req *pb.ReadRowsReques
// changes (or deletions) to a single row in a table.
func (s *goTestProxyServer) MutateRow(ctx context.Context, req *pb.MutateRowRequest) (*pb.MutateRowResult, error) {
s.clientsLock.RLock()
btc, exists := s.clientIDs[req.ClientId]
btc, exists := s.client(req.ClientId)
s.clientsLock.RUnlock()

if !exists {
Expand Down Expand Up @@ -723,7 +754,7 @@ func (s *goTestProxyServer) MutateRow(ctx context.Context, req *pb.MutateRowRequ
// series of changes or deletions to multiple rows in a single call.
func (s *goTestProxyServer) BulkMutateRows(ctx context.Context, req *pb.MutateRowsRequest) (*pb.MutateRowsResult, error) {
s.clientsLock.RLock()
btc, exists := s.clientIDs[req.ClientId]
btc, exists := s.client(req.ClientId)
s.clientsLock.RUnlock()

if !exists {
Expand Down Expand Up @@ -791,7 +822,7 @@ func (s *goTestProxyServer) BulkMutateRows(ctx context.Context, req *pb.MutateRo
// one mutation if a condition is true and another mutation if it is false.
func (s *goTestProxyServer) CheckAndMutateRow(ctx context.Context, req *pb.CheckAndMutateRowRequest) (*pb.CheckAndMutateRowResult, error) {
s.clientsLock.RLock()
btc, exists := s.clientIDs[req.ClientId]
btc, exists := s.client(req.ClientId)
s.clientsLock.RUnlock()

if !exists {
Expand Down Expand Up @@ -848,7 +879,7 @@ func (s *goTestProxyServer) CheckAndMutateRow(ctx context.Context, req *pb.Check
// of the keys available in a table.
func (s *goTestProxyServer) SampleRowKeys(ctx context.Context, req *pb.SampleRowKeysRequest) (*pb.SampleRowKeysResult, error) {
s.clientsLock.RLock()
btc, exists := s.clientIDs[req.ClientId]
btc, exists := s.client(req.ClientId)
s.clientsLock.RUnlock()

if !exists {
Expand Down Expand Up @@ -894,7 +925,7 @@ func (s *goTestProxyServer) SampleRowKeys(ctx context.Context, req *pb.SampleRow
// applies a non-idempotent change to a row.
func (s *goTestProxyServer) ReadModifyWriteRow(ctx context.Context, req *pb.ReadModifyWriteRowRequest) (*pb.RowResult, error) {
s.clientsLock.RLock()
btc, exists := s.clientIDs[req.ClientId]
btc, exists := s.client(req.ClientId)
s.clientsLock.RUnlock()

if !exists {
Expand Down
3 changes: 1 addition & 2 deletions bigtable/internal/testproxy/proxy_test.go
View file
Open in desktop
Expand Up @@ -193,8 +193,7 @@ func TestCreateAndRemoveClient(t *testing.T) {
t.Log("testproxy test: client created successfully in test proxy")

_, err = client.RemoveClient(ctx, &pb.RemoveClientRequest{
ClientId: cid,
CancelAll: true,
ClientId: cid,
})

if err != nil {
Expand Down