Skip to content

Binary producer/consumer for kafka#1440

Merged
jeremydmiller merged 1 commit intoJasperFx:mainfrom
dominikjeske:features/binary_producer
Jun 16, 2025
Merged

Binary producer/consumer for kafka#1440
jeremydmiller merged 1 commit intoJasperFx:mainfrom
dominikjeske:features/binary_producer

Conversation

@dominikjeske
Copy link
Contributor

@dominikjeske dominikjeske commented May 21, 2025

Why we need this change - in current implementation Wolverine is serializing message using IMessageSerializer to bytes but in kafka ProducerBuilder<string, string> is used to we are transforming this to string with Encoding.Default.GetString and inside IProducer<string, string>.ProduceAsync() another method is used to change this to bytes again with Encoding.Default.GetBytes (https://github.com/confluentinc/confluent-kafka-dotnet/blob/3f10fc8f829e7540e135fe8fe343371f1d4f5f2b/src/Confluent.Kafka/Serializers.cs#L42). So there is additional overhead that could be simplified by using ProducerBuilder<string, byte[]>.
Additionally in our case we implemented custom serializer using confluent avro and during tests we saw that Encoding.Default is not enough and byte array we get when deserializing are different. After investigation we find out that bytes produced by avro can't be change to UTF8 - it works only when we changed woverine code to Encoding.Unicode.


if (envelope.Data != null && envelope.Data.Any())
{
outgoing.Value = Encoding.Default.GetString(envelope.Data);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can skip this conversion

@jeremydmiller jeremydmiller added this to the 4.0 milestone May 27, 2025
@dominikjeske dominikjeske force-pushed the features/binary_producer branch from 1e853a9 to 485f1c2 Compare May 28, 2025 16:35
@jeremydmiller jeremydmiller removed this from the 4.0 milestone May 29, 2025
@dominikjeske dominikjeske marked this pull request as ready for review June 10, 2025 19:47
@jeremydmiller
Copy link
Member

Dammit, I should have gotten this in before 4.0. This is a breaking change, but I'm going to cheat and get it in anyway

@jeremydmiller jeremydmiller merged commit 39dd7d1 into JasperFx:main Jun 16, 2025
1 check passed
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.

2 participants