BewareMyPower commented on code in PR #139:
URL: https://github.com/apache/pulsar-client-cpp/pull/139#discussion_r1062271300


##########
lib/ConsumerImpl.cc:
##########
@@ -1526,4 +1577,96 @@ void ConsumerImpl::cancelTimers() noexcept {
     checkExpiredChunkedTimer_->cancel(ec);
 }
 
+void ConsumerImpl::processPossibleToDLQ(const MessageId& messageId, 
ProcessDLQCallBack cb) {
+    auto messages = possibleSendToDeadLetterTopicMessages_.find(messageId);
+    if (!messages) {
+        cb(false);
+        return;
+    }
+
+    // Initialize deadLetterProducer_
+    if (!deadLetterProducer_) {
+        Lock createLock(createProducerLock_);
+        if (!deadLetterProducer_) {
+            deadLetterProducer_ = std::make_shared<Promise<Result, 
Producer>>();
+            ProducerConfiguration producerConfiguration;
+            producerConfiguration.setSchema(config_.getSchema());
+            producerConfiguration.setBlockIfQueueFull(false);
+            if (!deadLetterPolicy_.getInitialSubscriptionName().empty()) {
+                producerConfiguration.setInitialSubscriptionName(
+                    deadLetterPolicy_.getInitialSubscriptionName());
+            }
+            ClientImplPtr client = client_.lock();
+            if (client) {
+                client->createProducerAsync(
+                    deadLetterPolicy_.getDeadLetterTopic(), 
producerConfiguration,
+                    [this](Result res, Producer producer) {

Review Comment:
   We should catch `shared_from_this()` to avoid `this` pointer is expired.



##########
lib/ConsumerImpl.cc:
##########
@@ -1526,4 +1577,96 @@ void ConsumerImpl::cancelTimers() noexcept {
     checkExpiredChunkedTimer_->cancel(ec);
 }
 
+void ConsumerImpl::processPossibleToDLQ(const MessageId& messageId, 
ProcessDLQCallBack cb) {
+    auto messages = possibleSendToDeadLetterTopicMessages_.find(messageId);
+    if (!messages) {
+        cb(false);
+        return;
+    }
+
+    // Initialize deadLetterProducer_
+    if (!deadLetterProducer_) {
+        Lock createLock(createProducerLock_);
+        if (!deadLetterProducer_) {
+            deadLetterProducer_ = std::make_shared<Promise<Result, 
Producer>>();
+            ProducerConfiguration producerConfiguration;
+            producerConfiguration.setSchema(config_.getSchema());
+            producerConfiguration.setBlockIfQueueFull(false);
+            if (!deadLetterPolicy_.getInitialSubscriptionName().empty()) {
+                producerConfiguration.setInitialSubscriptionName(
+                    deadLetterPolicy_.getInitialSubscriptionName());
+            }
+            ClientImplPtr client = client_.lock();
+            if (client) {
+                client->createProducerAsync(
+                    deadLetterPolicy_.getDeadLetterTopic(), 
producerConfiguration,
+                    [this](Result res, Producer producer) {
+                        if (res == ResultOk) {
+                            deadLetterProducer_->setValue(producer);
+                        } else {
+                            LOG_ERROR("Dead letter producer create exception 
with topic: "
+                                      << 
deadLetterPolicy_.getDeadLetterTopic() << " ex: " << res);
+                            deadLetterProducer_.reset();
+                        }
+                    });
+            } else {
+                LOG_WARN(getName() << "Client is destroyed and cannot create 
dead letter producer.");
+            }
+        }
+        createLock.unlock();

Review Comment:
   ```suggestion
   ```
   
   `createLock.unlock()` will be automatically called. And since you don't need 
to unlock before `createLock` goes out of scope, you can change its type to 
`std::lock_guard<std::mutex>` instead of the default `Lock` 
(`std::unique_lock`).



##########
lib/ConsumerImpl.cc:
##########
@@ -1526,4 +1577,96 @@ void ConsumerImpl::cancelTimers() noexcept {
     checkExpiredChunkedTimer_->cancel(ec);
 }
 
+void ConsumerImpl::processPossibleToDLQ(const MessageId& messageId, 
ProcessDLQCallBack cb) {
+    auto messages = possibleSendToDeadLetterTopicMessages_.find(messageId);
+    if (!messages) {
+        cb(false);
+        return;
+    }
+
+    // Initialize deadLetterProducer_
+    if (!deadLetterProducer_) {
+        Lock createLock(createProducerLock_);
+        if (!deadLetterProducer_) {
+            deadLetterProducer_ = std::make_shared<Promise<Result, 
Producer>>();
+            ProducerConfiguration producerConfiguration;
+            producerConfiguration.setSchema(config_.getSchema());
+            producerConfiguration.setBlockIfQueueFull(false);
+            if (!deadLetterPolicy_.getInitialSubscriptionName().empty()) {
+                producerConfiguration.setInitialSubscriptionName(
+                    deadLetterPolicy_.getInitialSubscriptionName());
+            }
+            ClientImplPtr client = client_.lock();
+            if (client) {
+                client->createProducerAsync(
+                    deadLetterPolicy_.getDeadLetterTopic(), 
producerConfiguration,
+                    [this](Result res, Producer producer) {
+                        if (res == ResultOk) {
+                            deadLetterProducer_->setValue(producer);
+                        } else {
+                            LOG_ERROR("Dead letter producer create exception 
with topic: "
+                                      << 
deadLetterPolicy_.getDeadLetterTopic() << " ex: " << res);
+                            deadLetterProducer_.reset();
+                        }
+                    });
+            } else {
+                LOG_WARN(getName() << "Client is destroyed and cannot create 
dead letter producer.");
+            }
+        }
+        createLock.unlock();
+    }
+
+    for (const auto& message : messages.value()) {
+        auto self = get_shared_this_ptr();
+        deadLetterProducer_->getFuture().addListener([self, message, 
cb](Result res, Producer producer) {
+            auto originMessageId = message.getMessageId();
+            std::stringstream originMessageIdStr;
+            originMessageIdStr << originMessageId;
+            MessageBuilder msgBuilder;
+            
msgBuilder.setAllocatedContent(const_cast<void*>(message.getData()), 
message.getLength())
+                .setProperties(message.getProperties())
+                .setProperty(PROPERTY_ORIGIN_MESSAGE_ID, 
originMessageIdStr.str())
+                .setProperty(SYSTEM_PROPERTY_REAL_TOPIC, 
message.getTopicName());
+            if (message.hasPartitionKey()) {
+                msgBuilder.setPartitionKey(message.getPartitionKey());
+            }
+            if (message.hasOrderingKey()) {
+                msgBuilder.setOrderingKey(message.getOrderingKey());
+            }
+            producer.sendAsync(msgBuilder.build(), [self, originMessageId, 
cb](Result res,

Review Comment:
   I prefer catching a `weak_ptr` instead of a `shared_ptr` here because the 
pending sends could extend the lifetime of the consumer. If the consumer closed 
before the send is completed, the callback should not be called.



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

Reply via email to