Skip to content
This repository has been archived by the owner on Dec 8, 2021. It is now read-only.

maintain desired SessionPool size #1172

Closed
mr-salty opened this issue Jan 8, 2020 · 5 comments
Closed

maintain desired SessionPool size #1172

mr-salty opened this issue Jan 8, 2020 · 5 comments
Assignees
@mr-salty
Labels
api: spanner Issues related to the googleapis/google-cloud-cpp-spanner API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Milestone
Spanner: GA

Comments

@mr-salty
Copy link
Contributor

mr-salty commented Jan 8, 2020

There are 2 parameters that specify the desired SessionPool size (plus max_sessions_per_channel which establishes an upper bound). The SessionPool should create or delete Sessions in the background to maintain the desired size.

  • min_sessions specifies the minimum number of Sessions associated with the pool, whether idle or in-use. Currently we create min_sessions Sessions at pool creation time but do not replenish Sessions if the number drops below that
  • max_idle_sessions specified the maximum number of idle Sessions to keep in the pool; any Sessions in excess of this number should be periodically deleted.
@mr-salty mr-salty added the type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. label Jan 8, 2020
@mr-salty mr-salty added this to the Spanner: GA milestone Jan 8, 2020
@mr-salty mr-salty mentioned this issue Jan 8, 2020
Closed
@mr-salty mr-salty self-assigned this Jan 23, 2020
@google-cloud-label-sync google-cloud-label-sync bot added the api: spanner Issues related to the googleapis/google-cloud-cpp-spanner API. label Jan 29, 2020
@mr-salty mr-salty mentioned this issue Feb 18, 2020
Merged
mr-salty added a commit that referenced this issue Feb 19, 2020
@mr-salty
chore: add some async methods to SpannerStub (#1270)
0e443a3 
* chore: add some async methods to SpannerStub

These are only for use by `SessionPool`; we are not exposing any async
APIs to users at this time.

Part of #1171 and #1172

* ABI/API update

the "break" is adding pure virtual methods to an internal object
(`SpannerStub`), so it's not actually a break.
@mr-salty mr-salty mentioned this issue Mar 9, 2020
Merged
mr-salty added a commit that referenced this issue Mar 10, 2020
@mr-salty
feat: refactor and enhance session creation logic (#1343)
2d10fc2 
* feat: refactor and enhance session creation logic

Centralize all Session creation in the `Grow()` method, which computes
the number of Sessions to create on each channel to balance the load
across channels. Also introduces an async mode (not implemented here)
which will eventually become the only mode.

Also fixes a bug where the per-channel session counts were not being
updated properly when `Sessions` were marked bad or disassociated.
Maybe this part should be its own PR, and I don't really like how it
obfuscates the locking requirements so I'm open to suggestions on how
to do this in a cleaner way.

Part of #1172

* address review comments

* address review comments
@mr-salty mr-salty mentioned this issue Mar 13, 2020
Closed
@mr-salty
Copy link
Contributor Author

Still TODO here:

  • Use the async BatchCreateSessions call once Support asynchronous operations in SessionPool #1365 is done.
  • Determine when sessions need to be deleted (by comparing the pool size vs. options_, fairly trivial) and use async DeleteSession to delete them.
  • Get rid of uses of non-async BatchCreateSessions (this could probably be done after GA)

@coryan
Copy link
Contributor

coryan commented Mar 19, 2020

@mr-salty I think we should close this now?

@mr-salty
Copy link
Contributor Author

I still have the "clean up old sessions" PR pending, I think it's done but I really need to do the fake clock before I can write tests for it. I can file another issue for the "delete" part though.

@coryan
Copy link
Contributor

coryan commented Mar 19, 2020

I can file another issue for the "delete" part though.

That sounds like the way to go.

@mr-salty
Copy link
Contributor Author

done (#1435)

devjgm pushed a commit to devjgm/google-cloud-cpp that referenced this issue May 7, 2020
@mr-salty
chore: add some async methods to SpannerStub (googleapis/google-clo…
a922ce5 
…ud-cpp-spanner#1270)

* chore: add some async methods to SpannerStub

These are only for use by `SessionPool`; we are not exposing any async
APIs to users at this time.

Part of googleapis/google-cloud-cpp-spanner#1171 and googleapis/google-cloud-cpp-spanner#1172

* ABI/API update

the "break" is adding pure virtual methods to an internal object
(`SpannerStub`), so it's not actually a break.
devjgm pushed a commit to devjgm/google-cloud-cpp that referenced this issue May 7, 2020
@mr-salty
feat: refactor and enhance session creation logic (googleapis/google-…
d1e2b1f 
…cloud-cpp-spanner#1343)

* feat: refactor and enhance session creation logic

Centralize all Session creation in the `Grow()` method, which computes
the number of Sessions to create on each channel to balance the load
across channels. Also introduces an async mode (not implemented here)
which will eventually become the only mode.

Also fixes a bug where the per-channel session counts were not being
updated properly when `Sessions` were marked bad or disassociated.
Maybe this part should be its own PR, and I don't really like how it
obfuscates the locking requirements so I'm open to suggestions on how
to do this in a cleaner way.

Part of googleapis/google-cloud-cpp-spanner#1172

* address review comments

* address review comments
@mr-salty mr-salty mentioned this issue May 7, 2020
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@mr-salty mr-salty

Labels
api: spanner Issues related to the googleapis/google-cloud-cpp-spanner API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Milestone
Spanner: GA
Development

No branches or pull requests
2 participants
@coryan @mr-salty