[GitHub] [nifi-minifi-cpp] bakaid commented on a change in pull request #651: MINIFICPP-1033 - PublishKafka sends success flow even though kafka se…

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

bakaid commented on a change in pull request #651: MINIFICPP-1033 - PublishKafka sends success flow even though kafka se…
URL: https://github.com/apache/nifi-minifi-cpp/pull/651#discussion_r326604923
 
 

 ##########
 File path: extensions/librdkafka/PublishKafka.cpp
 ##########
 @@ -345,6 +345,21 @@ void PublishKafka::onTrigger(const std::shared_ptr<core::ProcessContext> &contex
       }
     }
 
+    // Check if brokers are down, if yes then yield.
+    const struct rd_kafka_metadata *metadata;
+    /* Fetch metadata */
+    // TODO: What is the time complexity of this??
+    rd_kafka_resp_err_t err = rd_kafka_metadata(conn->getConnection(), 0, nullptr,
+                            &metadata, 500);
+    if (err != RD_KAFKA_RESP_ERR_NO_ERROR) {
+      logger_->log_error("Failed to acquire metadata: %s\n", rd_kafka_err2str(err));
+      session->transfer(flowFile, Failure);
 
 Review comment:
   @aboda `rd_kafka_metadata` is not a very good way to the determine whether a particular message actually got delivered to kafka. It might offer some protection in the negative case - if we don't have any brokers, we don't even try to send the message -, but in the positive case, even if we pass this check, librdkafka is asynchronous and there is a network call after that, meaning that the fact that we have brokers at this point does not at all mean that the message will be delivered successfully. If this is not a too expensive call, it might offer some benefits (in the negative case), but then it is definitely a yield case in my view.
   @nghiaxlee is working on a `rd_kafka_conf_set_dr_msg_cb` based solution, which provides per-message delivery info. If it works the way we figured it works, it will add some complexity to the code, but it's not too bad, and it is the way to go - as I said, I think giving no information is better than giving bad information resulting in data loss or duplication.

----------------------------------------------------------------
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


With regards,
Apache Git Services