pubsub: enable streaming pull by pongad · Pull Request #2223 · googleapis/google-cloud-java

pongad · 2017-07-10T04:11:43Z

Moving this to a branch.
If nothing else, we should perf test this before
declaring it release-ready.

This commit re-enables streaming pull.
Unlike previous implementation,
it does not fall back to polling if streaming is unavailable,
since the streaming pull endpoint should be working
by the time this is released.

This commit fixes a bug deadline modification code.
Previously we record an Instant we receive a message,
then call Instant::getNano to get the time in nanoseconds.
This is incorrect since getNano returns the nanosecond
from the beginning of that second, not since an epoch.
The fix is to simply save the time since epoch instead of Instant.

This commit also slightly changes how errors are logged,
so that errors that occur after the service is being shut down
don't spam the console.

cc @davidtorres

Moving this to a branch. If nothing else, we should perf test this before declaring it release-ready. This commit re-enables streaming pull. Unlike previous implementation, it does not fall back to polling if streaming is unavailable, since the streaming pull endpoint should be working by the time this is released. This commit fixes a bug deadline modification code. Previously we record an Instant we receive a message, then call Instant::getNano to get the time in nanoseconds. This is incorrect since getNano returns the nanosecond from the beginning of that second, not since an epoch. The fix is to simply save the time since epoch instead of Instant. This commit also slightly changes how errors are logged, so that errors that occur after the service is being shut down don't spam the console.

coveralls · 2017-07-10T05:04:36Z

Coverage increased (+0.05%) to 81.22% when pulling 3475c5e on pongad:pubsub-streaming-pull into 19262b7 on GoogleCloudPlatform:pubsub-streaming-pull.

garrettjonesgoogle · 2017-07-13T03:36:05Z

LGTM

Sign in to view

+ @Override
+ public void run() {
+ try {
+ // startPollingConnections();


Sign in to view

-// stopAllStreamingConnections();
- stopAllPollingConnections();
+ stopAllStreamingConnections();
+// stopAllPollingConnections();


Sign in to view

+ }
+
 // Starts polling connections. Blocks until all connections declare themselves running.
 private void startPollingConnections() throws IOException {


Sign in to view


 @Test
 public void testGetSubscriptionOnce() throws Exception {
+ if (isStreamingTest) {


googlebot · 2017-07-27T07:46:47Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

googlebot · 2017-07-27T07:51:02Z

CLAs look good, thanks!

pongad · 2017-07-27T08:05:35Z

@davidtorres Sorry for the delay. PTAL

coveralls · 2017-07-27T08:37:36Z

Changes Unknown when pulling 4541df2 on pongad:pubsub-streaming-pull into ** on GoogleCloudPlatform:pubsub-streaming-pull**.

coveralls · 2017-07-27T08:41:45Z

Changes Unknown when pulling 4541df2 on pongad:pubsub-streaming-pull into ** on GoogleCloudPlatform:pubsub-streaming-pull**.

garrettjonesgoogle · 2017-07-28T15:27:25Z

Actually I don't want to nuke polling just yet... could we keep the code for the moment?

pongad · 2017-07-31T02:01:44Z

@garrettjonesgoogle Do you mean just keeping the Polling class around? I was thinking about the same thing, but then if we need to restore polling, we'll probably revert anyway right? This is also in a branch so we can test to make sure before releasing? Please let me know if you feel strongly about this.

garrettjonesgoogle · 2017-07-31T15:00:06Z

@pongad I want to see if there is demand for Pub/Sub on http/json first - my request isn't really about reverting.

pongad · 2017-08-01T02:14:26Z

@garrettjonesgoogle I'm not sure exactly what you want. I added back Polling class, but it's not tested as we removed the fallback logic. Is this good enough for you?

garrettjonesgoogle · 2017-08-01T14:24:10Z

The diff is now showing all of the diffs from master since the head of your branch doesn't have those commits, making it very hard to read.

Could we add a package-private thing on the builder to trigger polling, and run all the relevant tests on both polling and streaming?

pongad · 2017-08-02T02:07:15Z

@garrettjonesgoogle PTAL

garrettjonesgoogle · 2017-08-02T18:35:04Z

re-LGTM

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>

…v1 to v2.75.0 (googleapis#2223) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [com.google.api.grpc:proto-google-cloud-trace-v1](https://redirect.github.com/googleapis/google-cloud-java) | `2.74.0` -> `2.75.0` | [![age](https://developer.mend.io/api/mc/badges/age/maven/com.google.api.grpc:proto-google-cloud-trace-v1/2.75.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/maven/com.google.api.grpc:proto-google-cloud-trace-v1/2.75.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/maven/com.google.api.grpc:proto-google-cloud-trace-v1/2.74.0/2.75.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/maven/com.google.api.grpc:proto-google-cloud-trace-v1/2.74.0/2.75.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://mend.io/renovate/). View the [repository job log](https://developer.mend.io/github/googleapis/java-spanner-jdbc).

…2223)

…lose or executor shutdown (#2223)

🤖 I have created a release *beep* *boop* --- ### Updating meta-information for bleeding-edge SNAPSHOT release. --- This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).

pongad added 2 commits July 3, 2017 18:00

Merge branch 'master' into pubsub-streaming-pull
bb73a70

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 10, 2017

pongad requested a review from garrettjonesgoogle July 10, 2017 04:11

pongad mentioned this pull request Jul 10, 2017
pubsub: enable streaming pull #2212
Merged

pr comment
3475c5e

davidtorres reviewed Jul 13, 2017
View reviewed changes

pongad added 3 commits July 27, 2017 17:24

pr comment
c9b50a6

Merge branch 'master' into pubsub-streaming-pull
bd6b54a

delete polling
2016878

googlebot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Jul 27, 2017

format code
4541df2

googlebot added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Jul 27, 2017

pongad added 2 commits August 1, 2017 12:08

Merge branch 'master' into pubsub-streaming-pull
ad030ed

add back polling connection
f005740

add polling back
4c7d3a8

pongad merged commit e681e9a into googleapis:pubsub-streaming-pullAug 3, 2017

pongad deleted the pubsub-streaming-pull branch August 3, 2017 00:09

chingor13 pushed a commit that referenced this pull request Feb 20, 2026
chore(main): release 2.41.2-SNAPSHOT (#2223)
76fac08
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>

meltsufin pushed a commit that referenced this pull request Apr 29, 2026
deps: update dependency org.apache.avro:avro to v1.11.4 [security] (#…
31f276b
…2223)

meltsufin pushed a commit that referenced this pull request May 1, 2026
deps: update dependency org.apache.avro:avro to v1.11.4 [security] (#…
f4bc0ed
…2223)

meltsufin pushed a commit that referenced this pull request May 2, 2026
fix: Resolve RejectedExecutionException that occurred on BulkWriter c…
14beba8
…lose or executor shutdown (#2223)

Uh oh!

Conversation

pongad commented Jul 10, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Jul 10, 2017

Uh oh!

garrettjonesgoogle commented Jul 13, 2017

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

googlebot commented Jul 27, 2017

Uh oh!

googlebot commented Jul 27, 2017

Uh oh!

pongad commented Jul 27, 2017

Uh oh!

coveralls commented Jul 27, 2017

Uh oh!

coveralls commented Jul 27, 2017

Uh oh!

garrettjonesgoogle commented Jul 28, 2017

Uh oh!

pongad commented Jul 31, 2017

Uh oh!

garrettjonesgoogle commented Jul 31, 2017

Uh oh!

pongad commented Aug 1, 2017

Uh oh!

garrettjonesgoogle commented Aug 1, 2017

Uh oh!

pongad commented Aug 2, 2017

Uh oh!

garrettjonesgoogle commented Aug 2, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

pongad commented Jul 10, 2017•
edited
Loading