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

[INGEST-76] Updating to librdkafka 1.8.2 #274

Closed
wants to merge 3 commits into from

Conversation

darena-zendesk
Copy link
Contributor

Why?

My team (@zendesk/ingest) maintains the support views pipeline that uses Racecar. Our consumers hang often and never recover. As a first step, we would like to upgrade Racecar to use the latest for rdkafka-ruby (0.11.0) which upgrades librdkafka to 1.8.2. Librdkafka 1.8.0 change log contains changes in this area.

References:

How?

I made the following changes:

  • Updated racecar.gemspec to require rdkafka 0.11.0
  • Committed the changes to Gemfile.lock that were generated by running bin/setup

Testing

I followed the instructions for setting up the Racecar development environment in the README and confirmed (dockerized) tests pass locally by running docker-compose run --rm tests rspec.

Considerations

  • I noticed that spec/integration/consumer_spec.rb tests only passed when ran in isolation. I observed this on both master and my branch.

Thanks in advance for your time! 🙇‍♀️

@darena-zendesk darena-zendesk force-pushed the da/INGEST-76/kafka_consumer_heartbeats branch 4 times, most recently from d6cd614 to 7fcfdb8 Compare January 26, 2022 22:04
@darena-zendesk darena-zendesk force-pushed the da/INGEST-76/kafka_consumer_heartbeats branch from 7fcfdb8 to 0eb9029 Compare January 26, 2022 22:11
Copy link
Contributor

@dasch dasch left a comment

Choose a reason for hiding this comment

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

I'm 👍 if we can either avoid the bump to the required Ruby version or have a good reason for it.

@@ -12,7 +12,7 @@ jobs:

strategy:
matrix:
ruby-version: ["2.4", "2.5", "2.6", "3.0"]
ruby-version: ["2.6", "3.0"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this change? That's breaking any existing client on an older version of Ruby, so we'd need a reason, and an entry in the changelog.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i need to add into the change log that rdkafka 0.11.0 requires ruby 2.6+ (https://rubygems.org/gems/rdkafka/versions/0.11.0). i was trying to get CI to pass, so i updated that collection, but couldn't find the definition that ensure the project passed on ruby 2.5.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 can you also rebase on latest master?

@leonmaia
Copy link
Contributor

Hey @darena-zendesk this is just a gentle bump on this PR 😅, could you rebase this with master? This change is very helpful and I know of a couple teams including mine that would love to have this merged as soon as possible 😄.

@leonmaia leonmaia mentioned this pull request Feb 15, 2022
@leonmaia leonmaia closed this Feb 16, 2022
@leonmaia
Copy link
Contributor

Merged on #283

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.

3 participants