test(spanner): unflake ReadBenchmark by isolating mock server and fixing heap leak - #14417
Conversation
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the benchmarking setup for the Spanner client by isolating the mock Spanner and monitoring gRPC servers into a separate JVM process (StandaloneBenchmarkServer) to prevent server threads and GC activity from interfering with the JMH benchmark client. Additionally, the benchmark validation logic was updated to allow improvements in latency to pass without throwing validation errors. Feedback on these changes includes: simplifying the validation check by removing a redundant Math.ceil call, clearing modified system properties in ReadBenchmark's tearDown to avoid test pollution, and adding try-catch error handling during StandaloneBenchmarkServer startup to prevent resource leaks on failure.
d1a15f6 to
d333deb
Compare
…ing heap leak - Add StandaloneBenchmarkServer and update ReadBenchmark to connect to an external gRPC mock server via SPANNER_PORT and MONITORING_PORT environment variables, throwing an IllegalStateException if SPANNER_PORT is missing. This allows running the mock server and JMH client in separate JVMs pinned to disjoint CPU cores. - Add MockSpannerServiceImpl.setRecordRequests(boolean) to return early from maybeFreezeAndRecordRequest when disabled, bypassing global lock contention and preventing unbounded retention of 20+ million request protobufs (~21.5 GB Old-Gen heap leak) during 20-minute benchmark runs. - Fix BenchmarkValidator so latency improvements (actual < baseline) do not fail validation.
d333deb to
574a011
Compare
sakthivelmanii
added
the
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
3 participants
StandaloneBenchmarkServerand updateReadBenchmark(along with the Maven benchmark profile) to connect to an external gRPC mock server whenjmh.spanner.server.portorSPANNER_PORTis specified. This allows running the mock server and JMH client in separate JVMs pinned to disjoint CPU cores.MockSpannerServiceImpl.setRecordRequests(boolean)to bypass global synchronized (lock) contention and prevent unbounded retention of 20+ million request protobufs (~21.5 GB Old-Gen heap leak) during 20-minute benchmark runs.BenchmarkValidatorso latency improvements (actual < baseline) do not fail validation, and include explicit units (us/op), allowed ceiling, percentage delta, and host SAR CPU telemetry (avg_busy, avg_steal, max_steal) in failure messages.