Merge our changes into updated master branch#2
Conversation
This reverts commit aa43ff4.
yboulkaid
changed the title
| raise ArgumentError unless delivery_threshold >= 0 | ||
| raise ArgumentError unless delivery_interval >= 0 | ||
| raise ArgumentError unless max_retries >= 0 | ||
| raise ArgumentError unless retry_backoff >= 0 |
There was a problem hiding this comment.
These checks were in our fork but not in the official version. We backport them to keep existing behavior.
|
This is tested and verified working on preview 👌 |
| # We use our object ID here to avoid conflicting with other instances | ||
| thread_key = @thread_key ||= "kafka_tagged_logging_tags:#{object_id}".freeze | ||
| Thread.current[thread_key] ||= [] | ||
| Thread.current[thread_key] ||= ["ruby-kafka"] |
There was a problem hiding this comment.
The tagged logging in action:
|
I dug up the exact diffs introducing the retry logic in our fork and in the upstream fork to make sure the logic is the same: 
The only difference is that our fork sleeps a constant duration ( |
yboulkaid
changed the title
yboulkaid
changed the title
yboulkaid
changed the title
|
regarding #2 (comment) @yboulkaid thank you for comparison. Could we keep our logic instead of upstream's? I believe it has better configurability. Moreover if upstream is not supported anymore it doesn't make sense to send PR to upstream |
| )) | ||
|
|
||
| if message.bytesize >= @max_buffer_bytesize | ||
| buffer_overflow topic, "Message is too big: message_bytesize=#{message.bytesize}" |
There was a problem hiding this comment.
@yboulkaid where is buffer_overflow function defined?
There was a problem hiding this comment.
It's defined in the AsyncProducer:
ruby-kafka/lib/kafka/async_producer.rb
Line 182 in b9b1cdf
This is the helper method that's used by the rest of the class to raise BufferOverflow errors
Sure! I'll make the change |
I used our logic instead of upstream in 23a8f75 |
yboulkaid
changed the title
4 participants
Our fork of
ruby-kafkahas fallen behind the gem'smaster, which made us miss the improvements and bugfixes as well as made it hard to add code changes to our fork because the specs were failing (because of for example this)This PR ports the changes we made to our fork to the latest master. Note that the diff of the custom code removal can be seen in this commit
The main changes we had made in this fork are:
BufferOverflow. Similar changes were also implemented upsteam, so we don't need to have our custom version anymore. See the merge commit for the removal diff.ruby-kafka:as a prefix to our log outputs so it's easier to filter the log lines coming from ruby-kafka.This has been replaced by adding
ruby-kafkaas a default log tag in theKafka::TaggedLoggerclass, so that all logging coming from this gem automatically has[ruby-kafka]appended. See the merge commit for the removal diff.