gharris1727 commented on code in PR #12984:
URL: https://github.com/apache/kafka/pull/12984#discussion_r1062744806
##########
connect/runtime/src/main/java/org/apache/kafka/connect/storage/KafkaConfigBackingStore.java:
##########
@@ -723,7 +752,11 @@ private void sendPrivileged(String key, byte[] value) {
try {
fencableProducer.beginTransaction();
- fencableProducer.send(new ProducerRecord<>(topic, key, value));
+ fencableProducer.send(new ProducerRecord<>(topic, key, value),
(metadata, exception) -> {
Review Comment:
> should already be the case with the current changes
It may be the case but not for the reason you specified. The guarantee from
the producer is that the record callbacks are completed before the
commitTransaction _returns_, while I was suggesting a control flow where we
ensure that record callbacks are completed before the commitTransaction is
_called_. If you still call commitTransaction, you will get both the callback's
error and the synchronous error from commitTransaction, and would still need to
make sure that the correct exception is shadowed. The control over this
shadowing is handled by this code and whoever is terminating the callback in
the REST layer.
> we could simply reword the existing exception message
This is certainly the easy way out, but I don't think we should settle for
that. Vague error messages indicate that they aren't capturing and reporting
the exception properly, and making them more vague to compensate doesn't help
anyone diagnose the underlying issue. If we can make a control flow change that
reports the errors faithfully, that is going to help someone down the line
debugging it.
> the worker logs will reveal the underlying TopicAuthorizationException
Is this the case? The worker is using _this_ code path to write the session
key; If we're hiding the result from the REST calls, are we not also hiding the
error from the herder tick thread?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
