-
Notifications
You must be signed in to change notification settings - Fork 375
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
doc(storage): describe connection pool #7637
doc(storage): describe connection pool #7637
Conversation
Google Cloud Build Logs
ℹ️ NOTE: Kokoro logs are linked from "Details" below. |
Codecov Report
@@ Coverage Diff @@
## main #7637 +/- ##
==========================================
- Coverage 95.28% 95.27% -0.01%
==========================================
Files 1251 1251
Lines 112992 112992
==========================================
- Hits 107659 107658 -1
- Misses 5333 5334 +1
Continue to review full report at Codecov.
|
google/cloud/storage/client.h
Outdated
* new HTTPS session is relatively expensive, as it must go through the TCP/IP | ||
* and SSL handshakes. To minimize this overhead the class maintains a | ||
* connection pool to the service. After each request completes the connection | ||
* is returned to the pool, and reuse in future requests. Note that for |
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.
s/reuse/reused/
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.
Fixed.
* | ||
* The application can limit the maximum size of this connection pool using | ||
* `storage::ConnectionPoolSizeOption`. If returning a connection to the pool | ||
* would make the pool larger than this limit then the oldest connection in the |
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: Would "oldest inactive connection" be more accurate?
The paragraph below suggests that an active download will not be closed.
Your call on whether to update the text or not.
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.
All connections in the pool are inactive ... and yes, active downloads are not closed (not while in progress).
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.
hmm ok.
s/connecitons in the pool are inactivate/connections in the pool are inactive/
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.
Fixed.
Google Cloud Build Logs
ℹ️ NOTE: Kokoro logs are linked from "Details" below. |
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.
Reviewable status: 0 of 1 files reviewed, 3 unresolved discussions (waiting on @coryan, @dbolduc, and @scotthart)
google/cloud/storage/client.h, line 84 at r2 (raw file):
inactivate
inactive
35b9db0
to
4caf7fb
Compare
Google Cloud Build Logs
ℹ️ NOTE: Kokoro logs are linked from "Details" below. |
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.
Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on @dbolduc and @scotthart)
google/cloud/storage/client.h, line 84 at r2 (raw file):
Previously, scotthart (Scott Hart) wrote…
inactivate
inactive
Done.
Fixes #1554
This change is