Pubsub hp by davidtorres · Pull Request #1483 · googleapis/google-cloud-java

davidtorres · 2016-12-17T05:03:52Z

No description provided.

googlebot · 2016-12-17T05:03:54Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.

davidtorres · 2016-12-17T05:04:58Z

@pongad please take a look, this adds support for falling back to pull and ack when no streaming pull support and fixes the flakiness on the tests

coveralls · 2016-12-17T06:07:10Z

Changes Unknown when pulling 68d79ad on davidtorres:pubsub-hp into ** on GoogleCloudPlatform:pubsub-hp**.

pongad

Travis and AppVeyor seems to still be flaking but we are moving in the right direction. Thank you for working on this! I have a few comments, mostly stylistic.

Sign in to view

 void extendExpiration() {
 expiration = new Instant(clock.millis()).plus(Duration.standardSeconds(nextExtensionSeconds));
 nextExtensionSeconds = 2 * nextExtensionSeconds;
+ if (nextExtensionSeconds > MAX_ACK_DEADLINE_EXTENSION_SECS) {


Sign in to view

+ }
+ Instant now = new Instant(clock.millis());
+ int totalByteCount = 0;
+ final List<AckHandler> ackHandlers = new ArrayList<>(responseMessages.size());


Sign in to view

+
+ List<List<String>> ackChunks = Lists.partition(acksToSend, MAX_PER_REQUEST_CHANGES);
+ Iterator<List<String>> ackChunksIt = ackChunks.iterator();
+ while (ackChunksIt.hasNext()) {


Sign in to view

- clock);
- }
+ streamingSubscriberConnections = new ArrayList<StreamingSubscriberConnection>(numChannels);
+ pollingSubscriberConnections = new ArrayList<PollingSubscriberConnection>(numChannels);


Sign in to view

-ImmutableList<String> receivedAcksCopy = ImmutableList.copyOf(acks);
- acks.clear();
+List<String> receivedAcksCopy = ImmutableList.copyOf(acks.subList(0, expectedCount));
+ acks.removeAll(receivedAcksCopy);


Sign in to view

- modAckDeadlines.clear();
+ List<ModifyAckDeadline> modAckDeadlinesCopy =
+ ImmutableList.copyOf(modAckDeadlines.subList(0, expectedCount));
+ modAckDeadlines.removeAll(modAckDeadlinesCopy);


pongad · 2016-12-19T02:48:13Z

@davidtorres Could you change the commit email? The one you're using, while undoubtedly yours, seems to be running into cla troubles.

pongad · 2016-12-19T02:49:08Z

Also cc @garrettjonesgoogle

googlebot · 2016-12-19T22:02:57Z

CLAs look good, thanks!

davidtorres · 2016-12-19T22:07:19Z

@pongad comments addressed please take a look

davidtorres · 2016-12-19T22:11:34Z

@pongad Not sure about the flakyness, can you point me to where you see this?

coveralls · 2016-12-19T22:36:49Z

Changes Unknown when pulling 0d82897 on davidtorres:pubsub-hp into ** on GoogleCloudPlatform:pubsub-hp**.

Sign in to view

+ final StreamObserver<StreamingPullResponse> responseObserver) {
+ final Stream stream = new Stream();
+ stream.requestObserver =
+ new StreamObserver<StreamingPullRequest>() {


Sign in to view

+ @Override
+ void initialize() {
+ final SettableFuture<Void> errorFuture = SettableFuture.create();
+ final ClientResponseObserver<StreamingPullRequest, StreamingPullResponse> responseObserver =


Sign in to view

+ * Abstract base implementation class of a subscriber connection in charge of receiving subscription
+ * messages.
+ */
+abstract class AbstractSubscriberConnection extends AbstractService {


pongad · 2016-12-19T23:30:38Z

@davidtorres I'll approve the change from my end, please also address Garrett's comments

pongad · 2016-12-19T23:38:35Z

@davidtorres The flake I thought I saw seems to have disappeared 😄

…ss to use containment in favor of inheritance

coveralls · 2016-12-20T22:10:17Z

Changes Unknown when pulling 6ef9df7 on davidtorres:pubsub-hp into ** on GoogleCloudPlatform:pubsub-hp**.

Sign in to view


- @Override
- protected void doStop() {
+ protected void stop() {


Sign in to view

+ * Abstract base implementation class of a subscriber connection in charge of receiving subscription
+ * messages.
+ */
+abstract class AbstractSubscriberConnection extends AbstractService {


garrettjonesgoogle · 2016-12-21T00:18:58Z

LGTM, I don't want to block on more refactoring right now.

coveralls · 2016-12-21T02:33:42Z

Changes Unknown when pulling f8c8dc6 on davidtorres:pubsub-hp into ** on GoogleCloudPlatform:pubsub-hp**.

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

) * add api key credential * add no op * formatting fix * added tests for ApiKeyCredentials * formatting fix * added java docs * formatting fix * added error checking * removed PreCondition dependency * fixed * import * updated authenticationType to API-Key * updated to use assertThrows + expanded java docs

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

…eapis#1483)

…48.0 (#1483)

…o v1.123.2 (#1483) * chore(deps): update dependency com.google.cloud:google-cloud-pubsub to v1.123.2 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

… to v2.15.0 (googleapis#1483) * chore(deps): update dependency com.google.cloud:google-cloud-bigtable to v2.15.0 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

googlebot added the cla: no This human has *not* signed the Contributor License Agreement. label Dec 17, 2016

pongad suggested changes Dec 19, 2016
View reviewed changes

pongad assigned pongad and garrettjonesgoogle Dec 19, 2016

davidtorres added 2 commits December 19, 2016 17:01

Update
eb693bd

Addressing feedback
0d82897

davidtorres force-pushed the pubsub-hp branch from 68d79ad to 0d82897Compare December 19, 2016 22:02

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 Dec 19, 2016

garrettjonesgoogle reviewed Dec 19, 2016
View reviewed changes

pongad approved these changes Dec 19, 2016
View reviewed changes

davidtorres added 2 commits December 20, 2016 16:16

Addressing the latest comments: mainly refactoring the connection cla…
6ef9df7
…ss to use containment in favor of inheritance

Fixing a race condition on shutdown
6b3c738

garrettjonesgoogle reviewed Dec 21, 2016
View reviewed changes

Addressing last comments of feedback
f8c8dc6

pongad merged commit 61687f0 into googleapis:pubsub-hpDec 21, 2016

theacodes unassigned pongad Sep 28, 2018

chingor13 pushed a commit that referenced this pull request Jan 6, 2026
chore(main): release 0.132.9-alpha (#1483)
4dd1491
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>

lqiu96 pushed a commit that referenced this pull request Mar 20, 2026
chore(main): release 0.132.9-alpha (#1483)
aa6b7ec
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>

suztomo pushed a commit to suztomo/google-cloud-java that referenced this pull request Mar 23, 2026
test(deps): update dependency com.google.truth:truth to v1.4.0 (googl…
971dc41
…eapis#1483)

suztomo pushed a commit that referenced this pull request Mar 30, 2026
chore(deps): update dependency com.google.cloud:libraries-bom to v26.…
3c4a4a8
…48.0 (#1483)

lqiu96 pushed a commit that referenced this pull request Apr 1, 2026
chore(deps): update dependency com.google.cloud:libraries-bom to v26.…
b6fa650
…48.0 (#1483)

Uh oh!

Conversation

davidtorres commented Dec 17, 2016

Uh oh!

googlebot commented Dec 17, 2016

Uh oh!

davidtorres commented Dec 17, 2016

Uh oh!

coveralls commented Dec 17, 2016

Uh oh!

pongad left a comment

Choose a reason for hiding this comment

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!

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!

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!

pongad commented Dec 19, 2016

Uh oh!

pongad commented Dec 19, 2016

Uh oh!

googlebot commented Dec 19, 2016

Uh oh!

davidtorres commented Dec 19, 2016

Uh oh!

davidtorres commented Dec 19, 2016

Uh oh!

coveralls commented Dec 19, 2016

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!

This comment was marked as spam.

Uh oh!

pongad commented Dec 19, 2016

Uh oh!

pongad commented Dec 19, 2016

Uh oh!

coveralls commented Dec 20, 2016

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.