Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new utility for generating and formatting Spanner request IDs, including its implementation, unit tests, and build configuration updates. The review feedback recommends addressing a potential fork deadlock risk and performance bottleneck in ProcessRandomId by replacing the std::mutex with a lock-free atomic pattern. Additionally, it suggests capturing and verifying the return value of the write system call in the unit tests to prevent compiler warnings under strict build settings.
| EXPECT_THAT(c1, Gt(0ULL)); | ||
| EXPECT_THAT(c2, Eq(c1 + 1)); | ||
| EXPECT_THAT(c3, Eq(c2 + 1)); |
There was a problem hiding this comment.
nit: why not check for Eq(1), Eq(2), Eq(3) ?
and monotonic would be more relaxed like:
EXPECT_THAT(c2, Gt(c1));
EXPECT_THAT(c3, Gt(c2));
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16465 +/- ##
==========================================
+ Coverage 92.31% 92.34% +0.02%
==========================================
Files 2246 2248 +2
Lines 214568 214645 +77
==========================================
+ Hits 198086 198220 +134
+ Misses 16482 16425 -57 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This PR adds inert code (to be used later) to generate and format Request ID headers for Spanner. Per the process specific requirements, there is some additional logic on non-Windows platforms to handle
forking of processes.