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

Bigtable latency spikes every hour due to grpc reconnects #4996

Closed
jc2729 opened this issue Sep 1, 2020 · 2 comments · Fixed by #5550
Closed

Bigtable latency spikes every hour due to grpc reconnects #4996

jc2729 opened this issue Sep 1, 2020 · 2 comments · Fixed by #5550
Assignees
@dopiera
Labels
api: bigtable Issues related to the Bigtable API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
Customer Issues

Comments

@jc2729
Copy link

jc2729 commented Sep 1, 2020

Is it possible to implement something similar to the ChannelPrimer in the Bigtable Java client to mitigate hourly spikes in latency due to underlying grpc connection shutdowns & reconnects

Java client reference: googleapis/java-bigtable#115
Related Github issue for Java client: googleapis/java-bigtable-hbase#1463
@mr-salty mr-salty added api: bigtable Issues related to the Bigtable API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. labels Sep 1, 2020
@mr-salty
Copy link
Contributor

mr-salty commented Sep 1, 2020

@coryan you seem like the most likely person to have insight on this, if not assign it back and i'll dig into it.

@coryan coryan removed their assignment Sep 1, 2020
@coryan coryan removed the priority: p2 Moderately-important priority. Fix may not be included in next release. label Sep 1, 2020
@coryan coryan added this to Needs triage in Customer Issues via automation Sep 1, 2020
@coryan
Copy link
Member

coryan commented Sep 1, 2020

A change similar to googleapis/java-bigtable#115 seems possible, but requires a number of changes:

  • The Bigtable classes do not have a set of background threads and/or completion queues with them, we would need to add such background threads to Table, TableAdmin, etc.

  • Then we would need to use something like this grpc::ChannelInterface::NotifyOnStateChange, to query the state of each channel and get notified when it changes:

https://grpc.github.io/grpc/cpp/classgrpc_1_1_channel_interface.html#a2acd32839ec4e5e92fe4b796a913623f

Note that we should avoid calling GetCurrentState(true) as that might block.

  • Alternatively, we could simply make a "ping" call, but there is no equivalent to SELECT 1; in Bigtable-land, that would require some thinking...

So, this is a valid feature request, I will put it in the backlog, no promises on when it will be done.

@coryan coryan moved this from Needs triage to High priority in Customer Issues Sep 2, 2020
dopiera added a commit to dopiera/google-cloud-cpp that referenced this issue Nov 25, 2020
@dopiera
feat: implement Bigtable connection refresh
5a317f1 
This fixes googleapis#4996.

This works, however it is not in its final form. Currently, background
threads running `CompletionQueue` are created underhandedly, without
users' control in every Bigtable `{Admin,InstanceAdmin,Data}Client`.

The choice of the `{Admin,InstanceAdmin,Data}Client` is consistent with
Spanner and PubSub, where counterparts of those clients are called
Connections.

The next steps will contain removing the `CompletionQueue` from both the
underlying clients and the user-facing API and instead providing
`BackgroundThreads` to the ctors. I didn't decide to do this in chunks
because otherwise it would either be confusing, which CompletionQueues
are used or we'd have to duplicate the whole API to use the
CompletionQueue provided in the ctors.

More tests will be possible when `BackgroundThreadsFactory` will be
passed in the `ClientOptions`
@dopiera dopiera mentioned this issue Nov 25, 2020
Merged
dopiera added a commit to dopiera/google-cloud-cpp that referenced this issue Nov 25, 2020
@dopiera
feat: implement Bigtable connection refresh
625fb0f 
This fixes googleapis#4996.

This works, however it is not in its final form. Currently, background
threads running `CompletionQueue` are created underhandedly, without
users' control in every Bigtable `{Admin,InstanceAdmin,Data}Client`.

The choice of the `{Admin,InstanceAdmin,Data}Client` is consistent with
Spanner and PubSub, where counterparts of those clients are called
Connections.

The next steps will contain removing the `CompletionQueue` from both the
underlying clients and the user-facing API and instead providing
`BackgroundThreads` to the ctors. I didn't decide to do this in chunks
because otherwise it would either be confusing, which CompletionQueues
are used or we'd have to duplicate the whole API to use the
CompletionQueue provided in the ctors.

More tests will be possible when `BackgroundThreadsFactory` will be
passed in the `ClientOptions`
dopiera added a commit to dopiera/google-cloud-cpp that referenced this issue Dec 2, 2020
@dopiera
feat: implement Bigtable connection refresh
2059514 
This fixes googleapis#4996.

This works, however it is not in its final form. Currently, background
threads running `CompletionQueue` are created underhandedly, without
users' control in every Bigtable `{Admin,InstanceAdmin,Data}Client`.

The choice of the `{Admin,InstanceAdmin,Data}Client` rather than e.g.
`Table` is consistent with Spanner and PubSub, where counterparts of
those clients are called Connections.

The next steps will contain removing the `CompletionQueue` from both the
underlying clients and the user-facing API and instead providing
`BackgroundThreads` to the ctors. I didn't decide to do this in chunks
because otherwise it would either be confusing, which CompletionQueues
are used or we'd have to duplicate the whole API to use the
CompletionQueue provided in the ctors.

More tests will be possible when `BackgroundThreadsFactory` will be
passed in the `ClientOptions`
dopiera added a commit to dopiera/google-cloud-cpp that referenced this issue Dec 2, 2020
@dopiera
feat: implement Bigtable connection refresh
deec432 
This fixes googleapis#4996.

This works, however it is not in its final form. Currently, background
threads running `CompletionQueue` are created underhandedly, without
users' control in every Bigtable `{Admin,InstanceAdmin,Data}Client`.

The choice of the `{Admin,InstanceAdmin,Data}Client` rather than e.g.
`Table` is consistent with Spanner and PubSub, where counterparts of
those clients are called Connections.

The next steps will contain removing the `CompletionQueue` from both the
underlying clients and the user-facing API and instead providing
`BackgroundThreads` to the ctors. I didn't decide to do this in chunks
because otherwise it would either be confusing, which CompletionQueues
are used or we'd have to duplicate the whole API to use the
CompletionQueue provided in the ctors.

More tests will be possible when `BackgroundThreadsFactory` will be
passed in the `ClientOptions`
dopiera added a commit to dopiera/google-cloud-cpp that referenced this issue Dec 3, 2020
@dopiera
feat: implement Bigtable connection refresh
4c9bc50 
This fixes googleapis#4996.

This works, however it is not in its final form. Currently, background
threads running `CompletionQueue` are created underhandedly, without
users' control in every Bigtable `{Admin,InstanceAdmin,Data}Client`.

The choice of the `{Admin,InstanceAdmin,Data}Client` rather than e.g.
`Table` is consistent with Spanner and PubSub, where counterparts of
those clients are called Connections.

The next steps will contain removing the `CompletionQueue` from both the
underlying clients and the user-facing API and instead providing
`BackgroundThreads` to the ctors. I didn't decide to do this in chunks
because otherwise it would either be confusing, which CompletionQueues
are used or we'd have to duplicate the whole API to use the
CompletionQueue provided in the ctors.

More tests will be possible when `BackgroundThreadsFactory` will be
passed in the `ClientOptions`
dopiera added a commit to dopiera/google-cloud-cpp that referenced this issue Dec 3, 2020
@dopiera
feat: implement Bigtable connection refresh
21cbdeb 
This fixes googleapis#4996.

This works, however it is not in its final form. Currently, background
threads running `CompletionQueue` are created underhandedly, without
users' control in every Bigtable `{Admin,InstanceAdmin,Data}Client`.

The choice of the `{Admin,InstanceAdmin,Data}Client` rather than e.g.
`Table` is consistent with Spanner and PubSub, where counterparts of
those clients are called Connections.

The next steps will contain removing the `CompletionQueue` from both the
underlying clients and the user-facing API and instead providing
`BackgroundThreads` to the ctors. I didn't decide to do this in chunks
because otherwise it would either be confusing, which CompletionQueues
are used or we'd have to duplicate the whole API to use the
CompletionQueue provided in the ctors.

More tests will be possible when `BackgroundThreadsFactory` will be
passed in the `ClientOptions`
dopiera added a commit to dopiera/google-cloud-cpp that referenced this issue Dec 3, 2020
@dopiera
feat: implement Bigtable connection refresh
4d8ac33 
This fixes googleapis#4996.

This works, however it is not in its final form. Currently, background
threads running `CompletionQueue` are created underhandedly, without
users' control in every Bigtable `{Admin,InstanceAdmin,Data}Client`.

The choice of the `{Admin,InstanceAdmin,Data}Client` rather than e.g.
`Table` is consistent with Spanner and PubSub, where counterparts of
those clients are called Connections.

The next steps will contain removing the `CompletionQueue` from both the
underlying clients and the user-facing API and instead providing
`BackgroundThreads` to the ctors. I didn't decide to do this in chunks
because otherwise it would either be confusing, which CompletionQueues
are used or we'd have to duplicate the whole API to use the
CompletionQueue provided in the ctors.

More tests will be possible when `BackgroundThreadsFactory` will be
passed in the `ClientOptions`
dopiera added a commit to dopiera/google-cloud-cpp that referenced this issue Dec 3, 2020
@dopiera
feat: implement Bigtable connection refresh
5386f75 
This fixes googleapis#4996.

This works, however it is not in its final form. Currently, background
threads running `CompletionQueue` are created underhandedly, without
users' control in every Bigtable `{Admin,InstanceAdmin,Data}Client`.

The choice of the `{Admin,InstanceAdmin,Data}Client` rather than e.g.
`Table` is consistent with Spanner and PubSub, where counterparts of
those clients are called Connections.

The next steps will contain removing the `CompletionQueue` from both the
underlying clients and the user-facing API and instead providing
`BackgroundThreads` to the ctors. I didn't decide to do this in chunks
because otherwise it would either be confusing, which CompletionQueues
are used or we'd have to duplicate the whole API to use the
CompletionQueue provided in the ctors.

More tests will be possible when `BackgroundThreadsFactory` will be
passed in the `ClientOptions`
Customer Issues automation moved this from High priority to Closed Dec 9, 2020
dopiera added a commit that referenced this issue Dec 9, 2020
@dopiera
feat: implement Bigtable connection refresh (#5550)
4759120 
This fixes #4996.

This works, however it is not in its final form. Currently, background
threads running `CompletionQueue` are created underhandedly, without
users' control in every Bigtable `{Admin,InstanceAdmin,Data}Client`.

The choice of the `{Admin,InstanceAdmin,Data}Client` rather than e.g.
`Table` is consistent with Spanner and PubSub, where counterparts of
those clients are called Connections.

The next steps will contain removing the `CompletionQueue` from both the
underlying clients and the user-facing API and instead providing
`BackgroundThreads` to the ctors. I didn't decide to do this in chunks
because otherwise it would either be confusing, which CompletionQueues
are used or we'd have to duplicate the whole API to use the
CompletionQueue provided in the ctors.

More tests will be possible when `BackgroundThreadsFactory` will be
passed in the `ClientOptions`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dopiera dopiera

Labels
api: bigtable Issues related to the Bigtable API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
No open projects
Customer Issues
  
Closed
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: implement Bigtable connection refresh dopiera/google-cloud-cpp
4 participants
@dopiera @coryan @mr-salty @jc2729