-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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): implement generation and propagation of "x-goog-spanner-request-id" Header #11048
Merged
olavloite
merged 14 commits into
googleapis:main
from
orijtech:spanner-request-id-header
Dec 18, 2024
Merged
feat(spanner): implement generation and propagation of "x-goog-spanner-request-id" Header #11048
olavloite
merged 14 commits into
googleapis:main
from
orijtech:spanner-request-id-header
Dec 18, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
odeke-em
force-pushed
the
spanner-request-id-header
branch
5 times, most recently
from
November 1, 2024 13:39
1938e60
to
463cd50
Compare
5 tasks
odeke-em
changed the title
spanner: prototype and lay down foundation for x-spanner-request-id header
spanner: implement generation and propagation of "x-spanner-request-id" Header
Nov 1, 2024
odeke-em
force-pushed
the
spanner-request-id-header
branch
from
November 1, 2024 14:08
7785a60
to
921c239
Compare
Kindly cc-ing @olavloite @tharoldD @willpoint |
odeke-em
force-pushed
the
spanner-request-id-header
branch
from
November 5, 2024 01:55
3d50ba7
to
3c79e77
Compare
odeke-em
changed the title
spanner: implement generation and propagation of "x-spanner-request-id" Header
feat(spanner): implement generation and propagation of "x-spanner-request-id" Header
Nov 5, 2024
odeke-em
force-pushed
the
spanner-request-id-header
branch
2 times, most recently
from
November 5, 2024 01:59
066caaa
to
ef110da
Compare
Kindly cc-ing you @harshachinta. |
odeke-em
changed the title
feat(spanner): implement generation and propagation of "x-spanner-request-id" Header
feat(spanner): implement generation and propagation of "x-goog-spanner-request-id" Header
Nov 5, 2024
odeke-em
force-pushed
the
spanner-request-id-header
branch
from
November 5, 2024 04:22
e9edb17
to
18cd8d4
Compare
@odeke-em can you please fix the vet and tests issues here? |
rahul2393
added
the
kokoro:force-run
Add this label to force Kokoro to re-run the tests.
label
Nov 8, 2024
kokoro-team
removed
the
kokoro:force-run
Add this label to force Kokoro to re-run the tests.
label
Nov 8, 2024
olavloite
reviewed
Nov 8, 2024
odeke-em
force-pushed
the
spanner-request-id-header
branch
6 times, most recently
from
November 20, 2024 03:36
9a9f77c
to
a35c97c
Compare
olavloite
reviewed
Dec 16, 2024
olavloite
added
the
kokoro:force-run
Add this label to force Kokoro to re-run the tests.
label
Dec 17, 2024
kokoro-team
removed
the
kokoro:force-run
Add this label to force Kokoro to re-run the tests.
label
Dec 17, 2024
olavloite
approved these changes
Dec 17, 2024
odeke-em
force-pushed
the
spanner-request-id-header
branch
from
December 17, 2024 15:19
cf9dfea
to
367ecbd
Compare
olavloite
added
the
kokoro:force-run
Add this label to force Kokoro to re-run the tests.
label
Dec 18, 2024
kokoro-team
removed
the
kokoro:force-run
Add this label to force Kokoro to re-run the tests.
label
Dec 18, 2024
…est-id" Header In tandem with the specification: https://orijtech.notion.site/x-goog-spanner-request-id-always-on-gRPC-header-to-aid-in-quick-debugging-of-errors-14aba6bc91348091a58fca7a505c9827 this change adds sending over the "x-goog-spanner-request-id" header for every unary and streaming call, in the form: <version>.<processId>.<clientId>.<channelId>.<requestCountForClient>.<rpcCountForRequest> where: * version is the version of the specification * processId is a randomly generated uint64 singleton for the lifetime of a process * clientId is the monotonically increasing id/number of gRPC Spanner clients created * requestCountForClient is the monotonically increasing number of requests made by the client * channelId currently at 1 is the Id of the client for Go * rpcCountForRequest is the number of RPCs/retries within a specific request This header is to be sent on both unary and streaming calls and it'll help debug latencies for customers. On an error, customers can assert against .Error and retrieve the associated .RequestID and log it, or even better it'll be printed out whenever errors are logged. Importantly making randIdForProcess to be a uint6 which is 64bits and not a UUID4 which is 128bits which surely massively reduces the possibility of collisions to ensure that high QPS applications can function and accept bursts of traffic without worry, as the prior design used uint32 aka 32 bits for which just 50,000 new processes being created could get the probability of collisions to 25%, with this new change a company would have to create 82 million QPS every second for 1,000 years for a 1% collision with 2.6e18 for which the collision would be 1%. Using 64-bits still provides really good protection whereby for a 1% chance of collision, we would need 810 million objects, so we have good protection. However, Google Cloud Spanner's backend has to store every one of the always on headers for a desired retention period hence 64-bits is a great balance between collision protection vs storage. Fixes googleapis#11073
…nnelID is derived from sessionClient.connPool
We have to re-insert the request-id even after gax.Invoke->grpc internals clear it. Added test to validate retries.
…to be wrapped with request-id
…eStreamingSql request
This change accounts for logic graciously raised by Knut along with his test contribution.
auto-merge was automatically disabled
December 18, 2024 07:07
Head branch was pushed to by a user without write access
odeke-em
force-pushed
the
spanner-request-id-header
branch
from
December 18, 2024 07:07
367ecbd
to
4cf4e11
Compare
olavloite
added
the
kokoro:force-run
Add this label to force Kokoro to re-run the tests.
label
Dec 18, 2024
kokoro-team
removed
the
kokoro:force-run
Add this label to force Kokoro to re-run the tests.
label
Dec 18, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In tandem this specification
https://orijtech.notion.site/x-goog-spanner-request-id-always-on-gRPC-header-to-aid-in-quick-debugging-of-errors-14aba6bc91348091a58fca7a505c9827
This change adds sending over the "x-goog-spanner-request-id" header
for every unary and streaming call, in the form:
where:
This header is to be sent on both unary and streaming calls and it'll
help debug latencies for customers. On an error, customers can assert against
.Error and retrieve the associated .RequestID and log it, or even better
it'll be printed out whenever errors are logged.
Importantly making randIdForProcess to be a uint6 which is 64bits and not
a UUID4 which is 128bits which surely massively reduces the possibility of collisions
to ensure that high QPS applications can function and accept bursts of traffic
without worry, as the prior design used uint32 aka 32 bits for
which just 50,000 new processes being created could get the probability
of collisions to 25%, with this new change a company would have to
create 82 million QPS every second for 1,000 years for a 1% collision
with 2.6e18 for which the collision would be 1%.
Using 64-bits still provides really good protection whereby for a 1% chance of collision,
we would need 810 million objects, so we have good protection.
However, Google Cloud Spanner's backend has to store every one of the always on
headers for a desired retention period hence 64-bits is a great balance between collision
protection vs storage.
Fixes #11073