[GitHub] [kafka] jolshan commented on a change in pull request #10590: KAFKA-5761: support ByteBuffer as value in ProducerRecord and avoid redundant serialization when it's used

[GitBox <gi...@apache.org>]

jolshan commented on a change in pull request #10590:
URL: https://github.com/apache/kafka/pull/10590#discussion_r633111374



##########
File path: clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java
##########
@@ -920,23 +923,28 @@ private void throwIfProducerClosed() {
                         " to class " + producerConfig.getClass(ProducerConfig.KEY_SERIALIZER_CLASS_CONFIG).getName() +
                         " specified in key.serializer", cce);
             }
-            byte[] serializedValue;
-            try {
-                serializedValue = valueSerializer.serialize(record.topic(), record.headers(), record.value());
-            } catch (ClassCastException cce) {
-                throw new SerializationException("Can't convert value of class " + record.value().getClass().getName() +
+
+            setReadOnly(record.headers());

Review comment:
       I think this line is what is causing the test to fail. It sets the header as read only, but in the test, we try to write to it.
   From the stack trace,
   
   ```
   at org.apache.kafka.common.header.internals.RecordHeaders.canWrite(RecordHeaders.java:122)
   	at org.apache.kafka.common.header.internals.RecordHeaders.add(RecordHeaders.java:61)
   	at org.apache.kafka.common.header.internals.RecordHeaders.add(RecordHeaders.java:68)
   	at kafka.api.PlaintextConsumerTest$SerializerImpl.serialize(PlaintextConsumerTest.scala:76)
   	at kafka.api.PlaintextConsumerTest$SerializerImpl.serialize$(PlaintextConsumerTest.scala:75)
   	at kafka.api.PlaintextConsumerTest$$anon$1.serialize(PlaintextConsumerTest.scala:142)
   	at kafka.api.PlaintextConsumerTest$$anon$1.serialize(PlaintextConsumerTest.scala:142)
   	at org.apache.kafka.clients.producer.KafkaProducer.doSend(KafkaProducer.java:937)```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org