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

Beetle 5.0 #115

Merged
merged 30 commits into from
Feb 20, 2025
Merged

Beetle 5.0 #115

merged 30 commits into from
Feb 20, 2025

Conversation

david-krentzlin
Copy link
Contributor

@david-krentzlin david-krentzlin commented Feb 18, 2025

This now brings the new beetle major version 5.0.
It incorporates a couple of changes, which may be breaking existing applications, hence a new major version.

Note for potential upstream users

As was already the case for 4.x, there will be no public release of this version on rubygems.org.
If you're brave you can use this version from the github and reference the appropriate tag.
However, we currently can not support those versions outside of the use of our organization.
If you need stable operations, its best to stick with the 3.x versions for now.

Changes

  • [breaking] Upgrade bunny from 0.7.13 to 2.23.0 (this includes a major change from a single threaded approach to multithreading)
  • [breaking] Enable heartbeats by default if the server offers heartbeats
  • [breaking] Enable publishing timeouts by default
  • Allow to enable TLS for redis connection (tested and verified to work with elasticache)

david-krentzlin and others added 9 commits February 10, 2025 15:37
Disable heartbeats by default.
* updated bunny to the latest version

The tests are green, but we need to study the interplay between the
various new timeout options in the multi threaded bunny implementation.

This is still WIP!!!

* turn on debug logging for the echo RPC

* enable auto recovery to see whether this fixes the CI failures

* Revert "enable auto recovery to see whether this fixes the CI failures"

This reverts commit c7b4b0e.

* don't wait when forcing a connection to close

* try killing the reader loop hard

* try to collect docker logs

* learned how to ignore scenarios

* improved logging

* colored output

* removed require that doesn't work anymore

* upgraded bunny

* Remove superfluous configuration

* Run actions on PRs against every base branch

* Update expectation to match new options

* Expect logger correctly

* Match only a subset of options we're interested in

* Update to bunny 2.23

This brings updates to the heartbeat implementation which we should get
in.

---------

Co-authored-by: David Krentzlin <[email protected]>
@david-krentzlin david-krentzlin mentioned this pull request Feb 18, 2025
1 task
@david-krentzlin david-krentzlin marked this pull request as ready for review February 18, 2025 13:40
@@ -40,7 +39,7 @@ jobs:
run: bundle install && bundle exec appraisal generate

- name: Compile and test beetle go binary
run: make && make test
run: make # && make test
Copy link
Contributor

Choose a reason for hiding this comment

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

Was that make test commented out for a reason?

Copy link
Contributor Author

@david-krentzlin david-krentzlin Feb 20, 2025

Choose a reason for hiding this comment

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

Yepp, this is to test the gobeetle, which created problems and I don't want to invest the time to fix it.

class NoMessageSent < Error; end
# logged when an RPC call timed outdated
class RPCTimedOut < Error; end
Copy link
Contributor

Choose a reason for hiding this comment

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

where is this class used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. I also don't find a reference anywhere for this.

@@ -123,7 +132,10 @@ class Configuration
# Write timeout for http requests to RabbitMQ HTTP API
attr_accessor :rabbitmq_api_write_timeout

# the socket timeout in seconds for message publishing (defaults to <tt>0</tt>).
# Returns the port on which the Rabbit API is hosted
attr_accessor :api_port
Copy link
Contributor

Choose a reason for hiding this comment

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

where is this used? could only find the release note for 4.0, that said it's dropped

Copy link
Contributor Author

@david-krentzlin david-krentzlin Feb 20, 2025

Choose a reason for hiding this comment

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

it's not used anymore, and could potentially be removed. We decided in the 4.0 to leave it in to not break any code that sets it. However now this looks a bit different and xing-amqp is the only library we really care about that uses beetle, which is fully in our hands. I'll ponder removing it.

@@ -0,0 +1,9 @@
require 'beetle'
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this file just for manual testing or should it be part of the github workflow?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just a manual test that I conducted.

@david-krentzlin david-krentzlin merged commit 147a26d into master Feb 20, 2025
24 checks passed
@david-krentzlin david-krentzlin deleted the v5.x branch February 20, 2025 13:38
@david-krentzlin david-krentzlin restored the v5.x branch February 20, 2025 13:42
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