gharris1727 commented on code in PR #15154: URL: https://github.com/apache/kafka/pull/15154#discussion_r1449313479
########## connect/runtime/src/main/java/org/apache/kafka/connect/runtime/errors/ProcessingContext.java: ########## @@ -17,82 +17,36 @@ package org.apache.kafka.connect.runtime.errors; import org.apache.kafka.clients.consumer.ConsumerRecord; -import org.apache.kafka.clients.producer.RecordMetadata; import org.apache.kafka.common.record.TimestampType; -import org.apache.kafka.connect.errors.ConnectException; -import org.apache.kafka.connect.runtime.errors.WorkerErrantRecordReporter.ErrantRecordFuture; import org.apache.kafka.connect.source.SourceRecord; -import java.util.Collection; -import java.util.Collections; -import java.util.List; -import java.util.Objects; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.Future; -import java.util.stream.Collectors; - /** - * Contains all the metadata related to the currently evaluating operation. Only one instance of this class is meant - * to exist per task in a JVM. + * Contains all the metadata related to the currently evaluating operation, and associated with a particular + * sink or source record from the consumer or task, respectively. This class is not thread safe, and so once an + * instance is passed to a new thread, it should no longer be accessed by the previous thread. Review Comment: 1. I believe so, as long as the "passing between the threads" itself is thread safe. So for example, if one thread writes to context, writes the context to a volatile field, then a second thread reads from that volatile field and then reads from the context, the memory model should ensure that the read sees the writes. Producer::send appears to have the synchronization when allocating the write to a batch. And the InternalSinkRecord#context field is marked final, which according to [this guidance](https://www.cs.umd.edu/~pugh/java/memoryModel/jsr-133-faq.html#finalRight) regarding final fields (emphasis is mine) should be sufficient: > The values for an object's final fields are set in its constructor. Assuming the object is constructed "correctly", once an object is constructed, the values assigned to the final fields in the constructor will be visible to all other threads without synchronization. In addition, the visible values for any other object or array referenced by those final fields **will be at least as up-to-date as the final fields.** 2. The fields themselves are not synchronized, as the object is not thread-safe. I don't think volatile buys any thread safety in this situation. -- 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. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
