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

Add a consumer option to rely on users for polling. #108

Merged
merged 1 commit into from
Oct 5, 2019

Conversation

shlevy
Copy link
Contributor

@shlevy shlevy commented Sep 5, 2019

We've been hitting a lot of issues with the background poll thread causing commit errors and large numbers of duplicate messages. I'm not sure how to fix it (see #107 for an attempt), but this change dramatically reduces the number of duplicate messages/commit errors we see in our test harness in a backwards-compatible way.

@AlexeyRaga
Copy link
Member

Does it just "reduces the number of duplicates" or it fixes the issue?
Because if it doesn't fix an issue then it means that we are treating symptoms and that the real issue is still unknown.
And if it does fix an issue then it means that we do something wrong around redirecting/rebalancing.

@@ -36,11 +38,12 @@ data ConsumerProperties = ConsumerProperties
{ cpProps :: Map Text Text
, cpLogLevel :: Maybe KafkaLogLevel
, cpCallbacks :: [KafkaConf -> IO ()]
, cpUserPolls :: Any
Copy link
Member

Choose a reason for hiding this comment

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

Using Any means that once stitched on it can never be switched off. It could be confusing, so can we have just Bool or Last Bool?

-- in a background thread, with this property set you can simplify
-- hw-kafka-client's footprint and have full control over when polling
-- happens at the cost of having to manage this yourself.
userPolls :: ConsumerProperties
Copy link
Member

Choose a reason for hiding this comment

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

The name userPolls kind of suggests that user should somehow poll manually.

Should we instead do something like:

data CallbackMode = CallbackModeAsync | CallbackModeSync

callbackMode :: CallbackMode -> ConsumerProperties

It looks a bit more explicit to me, do you agree?

@AlexeyRaga AlexeyRaga merged commit fb36815 into haskell-works:master Oct 5, 2019
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