Skip to content
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: wrap GZIPInputStream for connection reuse #840

Merged
merged 3 commits into from Oct 18, 2019

Conversation

codyoss
Copy link
Member

@codyoss codyoss commented Oct 14, 2019

If a connection is closed and there are some bytes that have not
been read that connection can't be reused. Now GZIPInputStream
will have all of its bytes read on close automatically to promote
connection reuse.

Cherry-picked: #749
Fixes: #367

chingor13 and others added 2 commits October 14, 2019 09:52
If a connection is closed and there are some bytes that have not
been read that connection can't be reused. Now GZIPInputStream
will have all of its bytes read on close automatically to promote
connection reuse.

Cherry-picked: googleapis#749
Fixes: googleapis#367
@codyoss codyoss requested a review from a team as a code owner October 14, 2019 18:33
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Oct 14, 2019
@codyoss
Copy link
Member Author

codyoss commented Oct 14, 2019

Noticed that #749 had not been updated in a while so I cherry picked and fixed the PR comments.


/**
* This class in meant to wrap an {@link InputStream} so that all bytes in the steam are read and
* discarded on {@link InputStream#close()}. This ensures that the underlying connection has the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if the stream never ends?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a very good question. I am not sure.

public void close() throws IOException {
if (!closed && inputStream != null) {
try {
ByteStreams.exhaust(this);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we could have an infinite loop here. See internal discussion.

@codyoss
Copy link
Member Author

codyoss commented Oct 17, 2019

@elharo I fixed all of the comments. Do you still have reservations around a possible infinite stream? If so, any thought on how we should move forward with this?(Maybe we just don't if we think the infinite stream is too big of a risk as this feature requires exhausting all of the extra bytes for it to work.)

Copy link
Contributor

@elharo elharo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

given how this is used and that it's non-public the infinite steam issue probably won't come up

@codyoss codyoss merged commit 087a428 into googleapis:master Oct 18, 2019
@codyoss codyoss deleted the chuncked-keepalive branch October 18, 2019 17:36
clundin25 pushed a commit to clundin25/google-http-java-client that referenced this pull request Aug 11, 2022
…-sdk to v2.0.3 (googleapis#840)

[![WhiteSource Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [com.google.appengine:appengine-api-1.0-sdk](https://togithub.com/GoogleCloudPlatform/appengine-java-standard) | `2.0.2` -> `2.0.3` | [![age](https://badges.renovateapi.com/packages/maven/com.google.appengine:appengine-api-1.0-sdk/2.0.3/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/maven/com.google.appengine:appengine-api-1.0-sdk/2.0.3/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/maven/com.google.appengine:appengine-api-1.0-sdk/2.0.3/compatibility-slim/2.0.2)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/maven/com.google.appengine:appengine-api-1.0-sdk/2.0.3/confidence-slim/2.0.2)](https://docs.renovatebot.com/merge-confidence/) |

---

### Release Notes

<details>
<summary>GoogleCloudPlatform/appengine-java-standard</summary>

### [`v2.0.3`](https://togithub.com/GoogleCloudPlatform/appengine-java-standard/releases/v2.0.3)

Release 2.0.3 from github open source code.

</details>

---

### Configuration

📅 **Schedule**: At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Renovate will not automatically rebase this PR, because other commits have been found.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.

---

This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/google-auth-library-java).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

chunked encoding + GZIP prevents keep-alive
4 participants