Skip to content

Spanner client session pool: timeout for creating sessions, more visibility #2688

Closed

Description

@maxqch

Hi,

I've noticed that sometimes com.google.cloud.spanner.DatabaseClient#readWriteTransaction takes a long time (~5 seconds) to return the transaction runner.

We have setFailIfPoolExhausted set to true, so I believe this is the result of a CreateSession request taking a long time, possibly due to failing + retrying the rpc, though we have no visibility into this.

The library does not allow us to set a timeout for attempts to create sessions which can result in our threads being hung up for several seconds waiting for a session. This is problematic when clients retry and exhausting our server's thread pool since all the threads are blocked waiting for sessions.

It would be great if there was the option (either set in SessionPoolOption or as a parameter on the DatabaseClient functions) that dictates the maximum amount of time to wait trying to get a session.

Thanks!

Activity

  1. self-assigned this
    on Dec 7, 2017
  2. vkedia commented on Dec 7, 2017

    @vkedia

    Thanks for filing this. I think what we need to do is to respect the deadline specified in grpc Context while waiting for a session to be available.
    As for visibility I am working on adding Tracing ( #2677 ) support into client library which should make it easier to debug such issues.

  3. added
    api: spannerIssues related to the Spanner API.
    type: questionRequest for information or clarification. Not an issue.
    on Dec 11, 2017
  4. vchudnov-g commented on Dec 11, 2017

    @vchudnov-g

    Does this address your question, @maxqch ?

  5. maxqch commented on Dec 11, 2017

    @maxqch
    Author

    Tracing sounds great, I'm looking forward to it :)

    I'm not sure I understand the grpc context deadline. How will this be exposed through the API? Also is there a ticket tracking the issue so it can get prioritized and worked on?

  6. vkedia commented on Dec 11, 2017

    @vkedia

    We can use this issue to prioritize this.
    We already use deadline specified in grpc Context elsewhere in the spanner client library. We just need to change the place where we block for a session to be created to timeout if the deadline has elapsed. There will be no change in the API as far as I can see. You would do something like this:

    Context.CancellableContext context =
            Context.current().withDeadlineAfter(10, TimeUnit.SECONDS, executor);
        Runnable work =
            context.wrap(
                new Runnable() {
                  @Override
                  public void run() {
                    dbClient.readWriteTransaction().run(transactionCallable);
                  }
                });
        work.run();
    

    You can do this even now and we would respect the deadline and fail the operation if transaction took a time longer than the deadline except that we do not respect this while waiting for a session.

  7. maxqch commented on Dec 11, 2017

    @maxqch
    Author

    Ah, I see, that makes sense. It would probably be helpful to add that to the API documentations. Maybe it's just me but I wasn't aware that an existing grpc context was being used for timeouts.

    Sounds good to me.

  8. added
    type: feature request‘Nice-to-have’ improvement, new feature or different behavior or design.
    and removed
    type: questionRequest for information or clarification. Not an issue.
    on Jan 24, 2018
  9. added
    type: cleanupAn internal cleanup or hygiene concern.
    and removed
    type: feature request‘Nice-to-have’ improvement, new feature or different behavior or design.
    on Jun 6, 2018
  10. added
    type: docsImprovement to the documentation for an API.
    and removed
    type: bugError or flaw in code with unintended results or allowing sub-optimal usage patterns.
    type: docsImprovement to the documentation for an API.
    on Oct 9, 2018
  11. JasonRosenberg commented on Dec 12, 2018

    @JasonRosenberg

    Any updates on this? We are running into this issue as well.

  12. ajaaym commented on Jan 11, 2019

    @ajaaym
    Contributor

    Change to use grpc context for timeouts while creating session is done and released.

  13. ajaaym commented on Jan 11, 2019

    @ajaaym
    Contributor

    PR is not merged yet, will track this issue in #3616

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

api: spannerIssues related to the Spanner API.type: docsImprovement to the documentation for an API.

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions