BewareMyPower commented on PR #110:
URL:
https://github.com/apache/pulsar-client-cpp/pull/110#issuecomment-1308994748
> In the current logic, there are two putIfAbsent operations here, and they
are confusing
The original logic should have been like the following diff based on your
fix:
```diff
diff --git a/lib/ConsumerImpl.cc b/lib/ConsumerImpl.cc
index 8658e08..f103144 100644
--- a/lib/ConsumerImpl.cc
+++ b/lib/ConsumerImpl.cc
@@ -392,11 +392,8 @@ Optional<SharedBuffer>
ConsumerImpl::processMessageChunk(const SharedBuffer& pay
auto it = chunkedMessageCache_.find(uuid);
if (chunkId == 0) {
- if (it == chunkedMessageCache_.end()) {
- it = chunkedMessageCache_.putIfAbsent(
- uuid, ChunkedMessageCtx{metadata.num_chunks_from_msg(),
metadata.total_chunk_msg_size()});
- }
- if (maxPendingChunkedMessage_ > 0 && chunkedMessageCache_.size() >
maxPendingChunkedMessage_) {
+ if (it == chunkedMessageCache_.end() && maxPendingChunkedMessage_ >
0 &&
+ chunkedMessageCache_.size() >= maxPendingChunkedMessage_) {
chunkedMessageCache_.removeOldestValues(
chunkedMessageCache_.size() - maxPendingChunkedMessage_,
[this](const std::string& uuid, const ChunkedMessageCtx&
ctx) {
@@ -404,7 +401,10 @@ Optional<SharedBuffer>
ConsumerImpl::processMessageChunk(const SharedBuffer& pay
discardChunkMessages(uuid, msgId,
autoAckOldestChunkedMessageOnQueueFull_);
}
});
- it = chunkedMessageCache_.find(uuid); // Need to reset the
iterator after changing the cache.
+ }
+ if (it == chunkedMessageCache_.end()) {
+ it = chunkedMessageCache_.putIfAbsent(
+ uuid, ChunkedMessageCtx{metadata.num_chunks_from_msg(),
metadata.total_chunk_msg_size()});
}
}
```
I intended to check the size first, then remove the oldest values if
necessary. After the oldest values are removed, insert the chunk message
context.
But I forgot to remove the first `putIfAbsent`.
--
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]
