Re: [PR] [fix][broker] Fix issue with GetMessageIdByTimestamp can't find match messageId from compacted ledger [pulsar]

["poorbarcode (via GitHub)" <gi...@apache.org>]

poorbarcode commented on code in PR #21600:
URL: https://github.com/apache/pulsar/pull/21600#discussion_r1449897315


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java:
##########
@@ -2882,28 +2882,47 @@ protected CompletableFuture<MessageId> internalGetMessageIdByTimestampAsync(long
                     throw new RestException(Status.METHOD_NOT_ALLOWED,
                         "Get message ID by timestamp on a non-persistent topic is not allowed");
                 }
-                ManagedLedger ledger = ((PersistentTopic) topic).getManagedLedger();
-                return ledger.asyncFindPosition(entry -> {
-                    try {
-                        long entryTimestamp = Commands.getEntryTimestamp(entry.getDataBuffer());
-                        return MessageImpl.isEntryPublishedEarlierThan(entryTimestamp, timestamp);
-                    } catch (Exception e) {
-                        log.error("[{}] Error deserializing message for message position find", topicName, e);
-                    } finally {
-                        entry.release();
-                    }
-                    return false;
-                }).thenApply(position -> {
-                    if (position == null) {
-                        return null;
-                    } else {
-                        return new MessageIdImpl(position.getLedgerId(), position.getEntryId(),
-                            topicName.getPartitionIndex());
-                    }
-                });
+                return ((PersistentTopic) topic).getTopicCompactionService().findEntryByPublishTime(timestamp)

Review Comment:
   Maybe use the `compactedTopicContext` to determine whether reading data from the original `ML` or the ledger containing the data compacted is better. Because if the `@param timestamp` is nearly, and there is considerable data in the ledger containing the data compacted,  it costs too much.



-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org