Skip to content
This repository has been archived by the owner on Dec 3, 2019. It is now read-only.

add pause/resume APIs to publisher and consumer #24

Merged
merged 6 commits into from
May 23, 2017
Merged

add pause/resume APIs to publisher and consumer #24

merged 6 commits into from
May 23, 2017

Conversation

datoug
Copy link
Contributor

@datoug datoug commented May 23, 2017

This patch adds pause/resume APIs to publisher and consumer. When paused, we'll just return an empty list of input/output hosts in the reconfiguration thread.

@datoug datoug requested review from aravindvs and hiboyang May 23, 2017 21:05
@coveralls
Copy link

coveralls commented May 23, 2017

Coverage Status

Coverage remained the same at 80.903% when pulling 5fef1a0 on pause into ccfe5e5 on master.

@coveralls
Copy link

coveralls commented May 23, 2017

Coverage Status

Coverage remained the same at 80.903% when pulling d0fedb3 on pause into ccfe5e5 on master.

} else {
// This is a potentially a transient error.
// Retry on next reconfigure
return
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible here getting infinite loop with high cpu usage if c.paused is zero and error is not EntityNotExistsError?

consumerOptions := &cherami.ReadConsumerGroupHostsResult_{}
if atomic.LoadUint32(&c.paused) == 0 {
consumerOptions, err = c.client.ReadConsumerGroupHosts(c.path, c.consumerGroupName)
if err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious, how we pause consuming here when c.paused is zero? It seems it will continue executing when error is nil.

// For non-streaming publishers, Pause/Resume APIs are no-op.
Pause()
// Resume publishing.
Resume()
Copy link

@abhinav abhinav May 25, 2017

Choose a reason for hiding this comment

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

@datoug, Adding methods to an interface is a breaking change. (See #25.)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants