-
Notifications
You must be signed in to change notification settings - Fork 3.6k
[feat][admin] PIP-415: Support getting message ID by index #24222
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 10 commits
7f876b6
75194f7
f29c42e
392b975
33fd340
f2e5259
c83f4f1
d4cdceb
2070e30
be588b8
6f588d9
7025b1c
ad9b66b
44386f2
35a5ae5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5489,4 +5489,114 @@ protected CompletableFuture<AutoSubscriptionCreationOverride> internalGetAutoSub | |
return null; | ||
})); | ||
} | ||
|
||
protected CompletableFuture<MessageId> internalGetMessageIDByIndexAsync(Long index, boolean authoritative) { | ||
if (!pulsar().getBrokerService().isBrokerEntryMetadataEnabled()) { | ||
return FutureUtil.failedFuture(new RestException(Status.PRECONDITION_FAILED, | ||
"GetMessageIDByIndex is not allowed when broker entry metadata is disabled")); | ||
} | ||
if (index == null || index < 0) { | ||
return FutureUtil.failedFuture(new RestException(Status.NOT_FOUND, | ||
"Invalid message index: " + index)); | ||
} | ||
int partitionIndex = topicName.getPartitionIndex(); | ||
CompletableFuture<Void> future = validateTopicOperationAsync(topicName, TopicOperation.PEEK_MESSAGES); | ||
return future.thenCompose(__ -> { | ||
if (topicName.isGlobal()) { | ||
return validateGlobalNamespaceOwnershipAsync(namespaceName); | ||
} else { | ||
return CompletableFuture.completedFuture(null); | ||
} | ||
}).thenCompose(__ -> { | ||
if (topicName.isPartitioned()) { | ||
return CompletableFuture.completedFuture(null); | ||
} else { | ||
return getPartitionedTopicMetadataAsync(topicName, authoritative, false) | ||
.thenAccept(topicMetadata -> { | ||
if (topicMetadata.partitions > 0) { | ||
log.warn("[{}] Not supported getMessageIdByIndex operation on " | ||
+ "partitioned-topic {}", clientAppId(), topicName); | ||
throw new RestException(Status.METHOD_NOT_ALLOWED, | ||
"GetMessageIDByIndex is not allowed on partitioned-topic"); | ||
} | ||
}); | ||
} | ||
}).thenCompose(ignore -> validateTopicOwnershipAsync(topicName, authoritative)) | ||
.thenCompose(__ -> getTopicReferenceAsync(topicName)) | ||
.thenCompose(topic -> { | ||
if (!(topic instanceof PersistentTopic persistentTopic)) { | ||
log.error("[{}] Get message id by index on a non-persistent topic {} is not allowed", | ||
clientAppId(), topicName); | ||
return FutureUtil.failedFuture(new RestException(Status.METHOD_NOT_ALLOWED, | ||
"Get message id by index on a non-persistent topic is not allowed")); | ||
} | ||
ManagedLedger managedLedger = persistentTopic.getManagedLedger(); | ||
return findMessageIndexByPosition( | ||
PositionFactory.create(managedLedger.getFirstPosition().getLedgerId(), 0), | ||
managedLedger) | ||
.thenCompose(firstIndex -> { | ||
if (index < firstIndex) { | ||
return CompletableFuture.completedFuture(PositionFactory.EARLIEST); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or consider returning null? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you explain why you think null is better than earliest? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Explicit signal that no valid position exists for the given index. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe you can get more context here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, got.
It's just my opinion. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Return earliest is OK, but do we pass the index to the clients? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
It's already documented, so it's good.
Do you mean the actual first available index? If so, I think the API could be unnecessarily complicated (a good API does not do multiple things) |
||
} else { | ||
return managedLedger.asyncFindPosition(entry -> { | ||
try { | ||
BrokerEntryMetadata brokerEntryMetadata = | ||
Commands.parseBrokerEntryMetadataIfExist(entry.getDataBuffer()); | ||
// Skip messages without index | ||
if (brokerEntryMetadata == null) { | ||
return true; | ||
} | ||
return brokerEntryMetadata.getIndex() < index; | ||
} catch (Exception e) { | ||
log.error("Error deserialize message for message position find", e); | ||
liangyepianzhou marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} finally { | ||
entry.release(); | ||
} | ||
return false; | ||
}); | ||
} | ||
}).thenCompose(position -> { | ||
Position lastPosition = managedLedger.getLastConfirmedEntry(); | ||
if (position == null || position.compareTo(lastPosition) > 0) { | ||
return FutureUtil.failedFuture(new RestException(Status.NOT_FOUND, | ||
liangyepianzhou marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"Message not found for index " + index)); | ||
} else { | ||
return CompletableFuture.completedFuture(position); | ||
} | ||
}); | ||
}).thenCompose(position -> CompletableFuture.completedFuture( | ||
new MessageIdImpl(position.getLedgerId(), position.getEntryId(), partitionIndex))); | ||
} | ||
|
||
protected CompletableFuture<Long> findMessageIndexByPosition(Position position, ManagedLedger managedLedger) { | ||
CompletableFuture<Long> indexFuture = new CompletableFuture<>(); | ||
managedLedger.asyncReadEntry(position, new AsyncCallbacks.ReadEntryCallback() { | ||
@Override | ||
public void readEntryComplete(Entry entry, Object ctx) { | ||
BrokerEntryMetadata brokerEntryMetadata = | ||
Commands.parseBrokerEntryMetadataIfExist(entry.getDataBuffer()); | ||
if (brokerEntryMetadata == null) { | ||
indexFuture.completeExceptionally(new RestException(Status.PRECONDITION_FAILED, | ||
"Broker entry metadata is not present in the message")); | ||
} else { | ||
long index = brokerEntryMetadata.getIndex(); | ||
if (index < 0) { | ||
indexFuture.completeExceptionally(new RestException(Status.PRECONDITION_FAILED, | ||
"Invalid message index: " + index)); | ||
} else { | ||
indexFuture.complete(index); | ||
} | ||
} | ||
liangyepianzhou marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
@Override | ||
public void readEntryFailed(ManagedLedgerException exception, Object ctx) { | ||
log.error("[{}] Failed to read position {} on topic {}", | ||
clientAppId(), position, topicName, exception); | ||
indexFuture.completeExceptionally(exception); | ||
} | ||
}, null); | ||
return indexFuture; | ||
} | ||
liangyepianzhou marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
} |
Uh oh!
There was an error while loading. Please reload this page.