Skip to content

Add support for cascading retries - #35

Open
derrickburns wants to merge 14 commits into
masterfrom
cascading-topics
Open

Add support for cascading retries#35
derrickburns wants to merge 14 commits into
masterfrom
cascading-topics

Conversation

@derrickburns

Copy link
Copy Markdown
Contributor

@todd I have not tested this yet. I just want to get your eyes on it to see if you a better approach.

Comment thread events/consumer.go
select {
case <-timer.C:
break
case <-session.Context().Done():

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is the context terminated when a rebalance happens?

Comment thread events/config.go Outdated
KafkaUsername string `envconfig:"KAFKA_USERNAME" required:"false"`
KafkaPassword string `envconfig:"KAFKA_PASSWORD" required:"false"`
KafkaDelay int `envconfig:"KAFKA_DELAY" default:"0"`
CascadeDelays []int `envconfig:"KAFKA_CASCADE_DELAYS" default:""`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think envocnfig supports parsing time.Duration out of the box, so it should be possible to use it directly, instead of int.

Should we provide sensible defaults?

@toddkazakov toddkazakov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good - just make sure rebalancing is handled correctly

@derrickburns

Copy link
Copy Markdown
Contributor Author

I added support for exponential backoff retries.

toddkazakov
toddkazakov previously approved these changes Nov 7, 2020

@toddkazakov toddkazakov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code looks good to me. Can you please test this before merging?

@derrickburns

derrickburns commented Nov 7, 2020 via email

Copy link
Copy Markdown
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants