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

pubsub: enable streaming pull #2212

Merged
merged 1 commit into from
Jul 10, 2017
Merged

pubsub: enable streaming pull #2212

merged 1 commit into from
Jul 10, 2017

Conversation

pongad
Copy link
Contributor

@pongad pongad commented Jul 3, 2017

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.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 3, 2017
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.
@pongad pongad changed the title are treated at lower severity and don't spam the console. pubsub: enable streaming pull Jul 3, 2017
@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 81.197% when pulling 315e58a on pongad:streaming-pull into 2a92207 on GoogleCloudPlatform:pubsub-streaming-pull.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 81.197% when pulling 315e58a on pongad:streaming-pull into 2a92207 on GoogleCloudPlatform:pubsub-streaming-pull.

Copy link
Member

@garrettjonesgoogle garrettjonesgoogle left a comment

Choose a reason for hiding this comment

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

@davidtorres can you take a look too?

}

@Override
protected void doStop() {
// stopAllStreamingConnections();
stopAllStreamingConnections();
stopAllPollingConnections();

This comment was marked as spam.

This comment was marked as spam.


ackDeadlineUpdater =
executor.scheduleAtFixedRate(
new Runnable() {

This comment was marked as spam.

This comment was marked as spam.

@pongad pongad merged commit 315e58a into googleapis:pubsub-streaming-pull Jul 10, 2017
@pongad pongad deleted the streaming-pull branch July 10, 2017 04:12
@pongad
Copy link
Contributor Author

pongad commented Jul 10, 2017

Sorry I accidentally merged this. I've reset the branch and re-posted this at #2223

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.

4 participants