CCOL-2441: Allow custom publishing batch size per producer #216
Conversation
| @@ -197,6 +197,7 @@ producers.schema_namespace|nil|Default namespace for all producers. Can remain n | |||
| producers.topic_prefix|nil|Add a prefix to all topic names. This can be useful if you're using the same Kafka broker for different environments that are producing the same topics. | |||
| producers.disabled|false|Disable all actual message producing. Generally more useful to use the `disable_producers` method instead. | |||
| producers.backend|`:kafka_async`|Currently can be set to `:db`, `:kafka`, or `:kafka_async`. If using Kafka directly, a good pattern is to set to async in your user-facing app, and sync in your consumers or delayed workers. | |||
| producers.max_batch_size|500|Maximum batch size for publishing. Individual producers can override. | |||
There was a problem hiding this comment.
Missing the doc for the new per-producer setting?
| @@ -92,6 +90,12 @@ def partition_key(_payload) | |||
| nil | |||
| end | |||
|
|
|||
| # @param size [Integer] Override the default batch size for publishing. | |||
| # @return [void] | |||
| def max_batch_size(size) | |||
There was a problem hiding this comment.
We probably don't need an actual method here, we can just assign it directly to the config inside configuration.rb.
There was a problem hiding this comment.
I'm not 100% sure what's going on but it seems like producer specs are not working as intended in general. As you suggested I added a line in configuration.rb to allow per-producer level batch size setting like so:
if kafka_config.respond_to?(:bulk_import_id_column) # consumer
...
else # new code here for producer
klass.config[:max_batch_size] = kafka_config.max_batch_size || Deimos.config.producers.max_batch_size
endI tried setting max_batch_size for a producer in item-feeds and it is working from there. However, self.configure_producer_or_consumer method is never reached for stubbed producers in producer_spec.rb so this config is not being set.
Also, now that I removed max_batch_size method from producer.rb, I'm getting this error when running producer specs:
Deimos::Producer should produce a message
Failure/Error: max_batch_size 1
NoMethodError:
undefined method `max_batch_size'
There was a problem hiding this comment.
Can you push your branch? I can take a look.
There was a problem hiding this comment.
Yeah... we're going to change this entirely in V2 🙃 I forgot that the stubbed producers don't go through the normal process. OK - you can put it back the way it was!
This reverts commit 0999eea.
|
LGTM! |
Pull Request Template
Description
Enabled the ability to configure
max_batch_sizeon a per-producer level instead of hard-coded limit of 500. The global setting is still set to 500 by default.Fixes CCOL-2441
Type of change
How Has This Been Tested?
Added specs
Checklist: