fix: init mp pool & grpc client once, use os.sched_setaffinity#1751
Conversation
product-auto-label
Bot
added
size: l
![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 multiprocessing setup in the write benchmarks to initialize the process pool and clients only once per run, optimizing benchmark results by preventing setup overhead. From a security standpoint, the analysis could not be fully completed as several files listed in SECURITY_ANALYSIS_TODO.md were unreadable, meaning no vulnerabilities were found in the accessible files, but a comprehensive review was hindered. For code improvements, consider making the os.sched_setaffinity CPU affinity setting more robust and portable, and ensuring graceful cleanup of resources in worker processes.
| def _worker_init(bucket_type): | ||
| """Initializes a persistent event loop and client for each worker process.""" | ||
| os.sched_setaffinity(0, {i for i in range(20, 190)}) # Pin to cores 20-189 | ||
| global worker_loop, worker_client, worker_json_client | ||
| if bucket_type == "zonal": | ||
| worker_loop = asyncio.new_event_loop() | ||
| asyncio.set_event_loop(worker_loop) | ||
| worker_client = worker_loop.run_until_complete(create_client()) | ||
| else: # regional | ||
| from google.cloud import storage | ||
|
|
||
| worker_json_client = storage.Client() |
There was a problem hiding this comment.
The _worker_init function initializes an event loop and clients for each worker process, but there's no corresponding cleanup logic. This can lead to resource leaks. While process termination will clean up resources, a graceful shutdown is better practice. Consider using atexit.register() in _worker_init to call a cleanup function that closes the clients and the event loop before the worker process exits.
There was a problem hiding this comment.
@chandra-siri, can you check if a cleanup is necessary? If not, let's resolve the comment. I think the client doesn't need to be closed, so we should be fine.
There was a problem hiding this comment.
yes close is possible in both json and grpc clients, but it's not urgent item. so created a b/479135274 few days earlier. Will fix it soon
…storage into writes_fixes
product-auto-label
Bot
added
size: m
|
/gcbrun(6bf0860) |
|
Added some comments, Please take a look. |
Addressed them |
|
@chandra-siri presubmits are failing, please fix it. |
chandra-siri
dismissed
googlyrahman’s stale review
The base branch was changed.
PR created by the Librarian CLI to initialize a release. Merging this PR will auto trigger a release. Librarian Version: v1.0.2-0.20251119154421-36c3e21ad3ac Language Image: us-central1-docker.pkg.dev/cloud-sdk-librarian-prod/images-prod/python-librarian-generator@sha256:8e2c32496077054105bd06c54a59d6a6694287bc053588e24debe6da6920ad91 <details><summary>google-cloud-storage: 3.10.0</summary> ## [3.10.0](v3.9.0...v3.10.0) (2026-03-18) ### Features * [Bucket Encryption Enforcement] add support for bucket encryption enforcement config (#1742) ([2a6e8b0](2a6e8b0)) ### Perf Improvments * [Rapid Buckets Reads] Use raw proto access for read resumption strategy (#1764) ([14cfd61](14cfd61)) * [Rapid Buckets Benchmarks] init mp pool & grpc client once, use os.sched_setaffinity (#1751) ([a9eb82c](a9eb82c)) * [Rapid Buckets Writes] don't flush at every append, results in bad perf (#1746) ([ab62d72](ab62d72)) ### Bug Fixes * [Windows] skip downloading blobs whose name contain `":" ` eg: `C:` `D:` etc when application runs in Windows. (#1774) ([5581988](5581988)) * [Path Traversal] Prevent path traversal in `download_many_to_path` (#1768) ([700fec3](700fec3)) * [Rapid Buckets] pass token correctly, '&' instead of ',' (#1756) ([d8dd1e0](d8dd1e0)) </details>
4 participants
fix: init mp pool & grpc client once, use os.sched_setaffinity