-
Notifications
You must be signed in to change notification settings - Fork 103
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.
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
feat(spanner): add support for Multiplexed Session for Read Only Tran… #2214
base: main
Are you sure you want to change the base?
Conversation
* chore(main): release 7.17.0 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --------- Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: surbhigarg92 <[email protected]> Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
🤖 I have created a release *beep* *boop* --- ## [7.17.1](https://togithub.com/googleapis/nodejs-spanner/compare/v7.17.0...v7.17.1) (2025-01-03) ### Bug Fixes * Remove default global trace context propagator ([#2209](https://togithub.com/googleapis/nodejs-spanner/issues/2209)) ([7898e0c](https://togithub.com/googleapis/nodejs-spanner/commit/7898e0ce0477e2d4327822ac26a2674203b47a64)), closes [#2208](https://togithub.com/googleapis/nodejs-spanner/issues/2208) --- This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).
8a256e0
to
7cf1d60
Compare
7cf1d60
to
9a85b93
Compare
860bf28
to
47a9219
Compare
47a9219
to
3a3d8c7
Compare
3a3d8c7
to
bc56399
Compare
bc56399
to
f477940
Compare
…b.com/googleapis/nodejs-spanner into multiplexed-session-support-read-only
…b.com/googleapis/nodejs-spanner into multiplexed-session-support-read-only
000e144
to
fa438fe
Compare
fa438fe
to
270cae3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please share benchmarking results before merging
6fe8379
to
7c4f371
Compare
|
||
'use strict'; | ||
|
||
const muxEnabledTrue = []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I don't think you need to maintain two arrays here. You will either call this script with multiplexedEnabled as true or false. So only one will be used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, updated
} | ||
|
||
async function runQueries() { | ||
multiplexedEnabled === 'true' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Every thread is creating new Spanner instance, that would mean it will create a new mux session. So eventually we are running only one query at a time for one spanner client.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for pointing this out, updated the client creation
7c4f371
to
de2c7ce
Compare
48814d8
to
995b31d
Compare
This PR contains support for Multiplexed Session in all the RO methods, along with their tests.
The methods includes are:
getSnapshot
runStream
writeAtLeastOnce