-
Notifications
You must be signed in to change notification settings - Fork 451
Choose partition by key for several brokers #208
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a great idea but we do need to have tests here, can you please add them (at least a functional one)?
src/Producer/SyncProcess.php
Outdated
} else { | ||
$partId = $value['partId']; | ||
shuffle($partNums); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't the shuffle needed only for when you're using $partsNum
?
src/Producer/SyncProcess.php
Outdated
$partId = 0; | ||
if (! isset($value['partId']) || ! isset($topicMeta[$value['partId']])) { | ||
$partId = $partNums[0]; | ||
if (isset($value['key']) && trim($value['key'])) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extracting this logic to a method and using early returns will make things way easier to understand, could you please do that?
I moved the logic of the choice of partitions to the broker method and add a test. |
@demyanovs awesome, thanks! Can you please address the issues pointed out by phpstan? I'll take a look at your code when the build is green 👍 |
Also, please use |
Hi,
I have three brokers and when I, for example, write 5 messages in a cycle, they come in random order.
Before choosing a broker we need to select partition by key, so that the messages were written consistently in correct order.