Max ack extension by davidtorres · Pull Request #1898 · googleapis/google-cloud-java

davidtorres · 2017-04-11T13:37:02Z

Inadvertently (and my because my lack of knowledge of github PRs) I reset a change in PR #1869 and that caused it to drop it.

@pongad @garrettjonesgoogle

disable it. googleapis#1639

…on to (googleapis#1645)". Compression is not fully supported in gRPC, can't have it in the library yet. This reverts commit a599972.

…Consumer interface, it really is no useful to be able to set an exception as ack reply, since the result is the same as nack, if we ever require another result then we can just add one more value to the AckReply enum. Also adding a fail-safe catch so if the receiver ever throws an exception we will interpret that as a nack and keep going.

This prevents the library extending messages infinitevely if the user code has lost track of the ack consumer handle.

the next extension surpasses it.

respected.

coveralls · 2017-04-11T13:53:25Z

Changes Unknown when pulling 4883e6f on davidtorres:max-ack-ext into ** on GoogleCloudPlatform:master**.

Sign in to view

+            return new ModifyAckDeadline(ack, 2); // 2 seconds is the initial mod ack deadline
+          }
+        });
+


Sign in to view

+        });
+
+    int timeIncrement = 4; // Second time increment
+


Sign in to view

+
+    while (fakeExecutor.getClock().millisTime() + (timeIncrement * 1000) < 1000 * 60 * 60) {
+      fakeExecutor.advanceTime(Duration.standardSeconds(timeIncrement));
+


* Adding units to new tests variables * Making a constant for the first time modify ack deadline extension

garrettjonesgoogle · 2017-04-11T17:12:01Z

LGTM after tests pass

garrettjonesgoogle · 2017-04-11T18:06:00Z

@davidtorres I have a concern that the new test might be flaky - could you check the Travis failures?

manifest only certain github testing environments.

davidtorres · 2017-04-11T19:49:20Z

Yes I'm pushing a change now to address the issue, the tests should/might have been flaky before as well (or the expectations too lax), this is an old bug in the tests because of thread scheduling and the latest changes to the tests just exposed the bug.

coveralls · 2017-04-11T20:26:43Z

Changes Unknown when pulling 09172a6 on davidtorres:max-ack-ext into ** on GoogleCloudPlatform:master**.

coveralls · 2017-04-11T21:14:22Z

Changes Unknown when pulling 6a21a54 on davidtorres:max-ack-ext into ** on GoogleCloudPlatform:master**.

davidtorres · 2017-04-11T21:28:56Z

Tests are now passing PTAL

Sign in to view


+  private Deque<Duration> expectedWorkQueue = new LinkedList<>();
+
+  public void setupScheduleExpectation(Duration delay) {


Sign in to view

    // Send messages to be acked
    List<String> testAckIdsBatch = ImmutableList.of("A", "B", "C");
    testReceiver.setExplicitAck(true);
+    // A modify ack deadline should be schedule for the next 9s


Sign in to view

      pendingCallables.add(callable);
    }
    work();
+    synchronized (expectedWorkQueue) {


Sign in to view

    }
    work();
+    synchronized (expectedWorkQueue) {
+      if (!expectedWorkQueue.isEmpty() && expectedWorkQueue.peek().equals(callable.delay)) {


pongad · 2017-04-12T00:01:15Z

Aside from @garrettjonesgoogle's comments, I'm good with this. Thank you for the PR!

FakeScheduledExecutor that allow you to set expectations on scheduled work.

coveralls · 2017-04-12T14:48:57Z

Changes Unknown when pulling 204438c on davidtorres:max-ack-ext into ** on GoogleCloudPlatform:master**.

…is#1898)

…37.1 (#1898)

…37.1 (#1898)

… v3.5.0 (#1898)

davidtorres added 14 commits February 22, 2017 15:39

Enabling client-side compression in the library, with an option to

c4c6174

disable it. googleapis#1639

Merge remote-tracking branch 'upstream/master'

27338a4

Revert "Enabling client-side compression in the library, with an opti…

d6a1043

…on to (googleapis#1645)". Compression is not fully supported in gRPC, can't have it in the library yet. This reverts commit a599972.

Merge remote-tracking branch 'upstream/master'

823ad09

Merge remote-tracking branch 'upstream/master'

652894a

Example fixes to comply with changes to the AckReplyConsumer interface.

3e906e6

Allowing for setting a maximum for message lease extensions.

32af932

This prevents the library extending messages infinitevely if the user code has lost track of the ack consumer handle.

Setting so the max ack deadline duration is always respected even when

7779e37

the next extension surpasses it.

Merge branch 'master' into max-ack-ext

45248f0

Merge remote-tracking branch 'upstream/master' into max-ack-ext

d4f0048

Merge branch 'master' into max-ack-ext

140b6bc

Align with the changes to ApiClock

4debcbb

Adding a test for testing the default max ack extension duration is

4883e6f

respected.

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

davidtorres mentioned this pull request Apr 11, 2017

Setting a maximum period for ack extensions. #1869

Closed

garrettjonesgoogle reviewed Apr 11, 2017

View reviewed changes

Addressing feedback:

347c661

* Adding units to new tests variables * Making a constant for the first time modify ack deadline extension

Fixing race conditions in tests that for thread scheduling reason

09172a6

manifest only certain github testing environments.

Addressing codacy warnings

6a21a54

garrettjonesgoogle reviewed Apr 11, 2017

View reviewed changes

Adding documentation to the new added method in the

204438c

FakeScheduledExecutor that allow you to set expectations on scheduled work.

garrettjonesgoogle merged commit 95946ef into googleapis:master Apr 12, 2017

This was referenced Apr 12, 2017

pubsub test flakes #1626

Closed

Fix Pub/Sub DEADLINE_EXCEEDED issue during build #1877

Closed

meltsufin pushed a commit that referenced this pull request Dec 22, 2025

deps: update googleapis/sdk-platform-java action to v2.64.2 (#1898)

5860f51

chingor13 pushed a commit that referenced this pull request Jan 22, 2026

deps: update googleapis/sdk-platform-java action to v2.60.0 (#1898)

0921f86

suztomo pushed a commit to suztomo/google-cloud-java that referenced this pull request Mar 11, 2026

deps: update googleapis/sdk-platform-java action to v2.64.2 (googleap…

3cbf629

…is#1898)

lqiu96 pushed a commit that referenced this pull request Mar 20, 2026

deps: update googleapis/sdk-platform-java action to v2.60.0 (#1898)

f84f65c

meltsufin pushed a commit that referenced this pull request Apr 29, 2026

deps: update dependency com.google.cloud:google-cloud-bigquery to v2.…

fc0dc96

…37.1 (#1898)

meltsufin pushed a commit that referenced this pull request May 1, 2026

deps: update dependency com.google.cloud:google-cloud-bigquery to v2.…

5602698

…37.1 (#1898)

meltsufin pushed a commit that referenced this pull request May 2, 2026

build(deps): update dependency org.codehaus.mojo:exec-maven-plugin to…

b9df8d5

… v3.5.0 (#1898)


		while (fakeExecutor.getClock().millisTime() + (timeIncrement * 1000) < 1000 * 60 * 60) {
		fakeExecutor.advanceTime(Duration.standardSeconds(timeIncrement));


		private Deque<Duration> expectedWorkQueue = new LinkedList<>();

		public void setupScheduleExpectation(Duration delay) {

Conversation

davidtorres commented Apr 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Apr 11, 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!

garrettjonesgoogle commented Apr 11, 2017

Uh oh!

garrettjonesgoogle commented Apr 11, 2017

Uh oh!

davidtorres commented Apr 11, 2017

Uh oh!

coveralls commented Apr 11, 2017

Uh oh!

coveralls commented Apr 11, 2017

Uh oh!

davidtorres commented Apr 11, 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!

This comment was marked as spam.

Uh oh!

pongad commented Apr 12, 2017

Uh oh!

coveralls commented Apr 12, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

davidtorres commented Apr 11, 2017 •

edited

Loading