You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/06/29 03:05:45 UTC
[GitHub] [pulsar] Jason918 opened a new pull request #11139: [Issue 10896] add get message id by timestamp
Jason918 opened a new pull request #11139:
URL: https://github.com/apache/pulsar/pull/11139
<!--
### Contribution Checklist
- Name the pull request in the form "[Issue XYZ][component] Title of the pull request", where *XYZ* should be replaced by the actual issue number.
Skip *Issue XYZ* if there is no associated github issue for this pull request.
Skip *component* if you are unsure about which is the best component. E.g. `[docs] Fix typo in produce method`.
- Fill out the template below to describe the changes contributed by the pull request. That will give reviewers the context they need to do the review.
- Each pull request should address only one issue, not mix up code from multiple issues.
- Each commit in the pull request has a meaningful commit message
- Once all items of the checklist are addressed, remove the above text and this checklist, leaving only the filled out template below.
**(The sections below can be removed for hotfixes of typos)**
-->
*(If this PR fixes a github issue, please add `Fixes #<xyz>`.)*
Fixes #10896
### Motivation
Add getMessageIdByTimestamp in pulsar admin apis
### Modifications
The core searching implementation reuses `org.apache.bookkeeper.mledger.ManagedLedger#asyncFindPosition`.
Add client tool for cmd `pulsar-admin topics get-message-id`
### Verifying this change
- [x] Make sure that the change passes the CI checks.
This change added tests and can be verified as follows:
- *Extended integration test in org.apache.pulsar.broker.admin.PersistentTopicsTest#testGetMessageIdByTimestamp*
### Does this pull request potentially affect one of the following parts:
*If `yes` was chosen, please highlight the changes*
- Dependencies (does it add or upgrade a dependency): (no)
- The public API: (yes)
- The schema: (no)
- The default values of configurations: (no)
- The wire protocol: (no)
- The rest endpoints: (yes)
- The admin cli options: (yes)
- Anything that affects deployment: (no)
### Documentation
- Does this pull request introduce a new feature? (yes)
- If yes, how is the feature documented? (docs and JavaDocs)
--
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
[GitHub] [pulsar] codelipenghui commented on a change in pull request #11139: [Issue 10896] add get message id by timestamp
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on a change in pull request #11139:
URL: https://github.com/apache/pulsar/pull/11139#discussion_r661903290
##########
File path: site2/docs/admin-api-topics.md
##########
@@ -538,6 +538,31 @@ admin.topics().getMessageById(topic, ledgerId, entryId);
<!--END_DOCUSAURUS_CODE_TABS-->
+### Get message ID
+
+You can get message id published at or just after the given datetime.
+
+<!--DOCUSAURUS_CODE_TABS-->
+<!--pulsar-admin-->
+```shell
+$ ./bin/pulsar-admin topics get-message-id \
Review comment:
@Anonymitaet please take a look Jason's comment
--
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
[GitHub] [pulsar] Jason918 commented on a change in pull request #11139: [Issue 10896] add get message id by timestamp
Posted by GitBox <gi...@apache.org>.
Jason918 commented on a change in pull request #11139:
URL: https://github.com/apache/pulsar/pull/11139#discussion_r660593334
##########
File path: site2/docs/admin-api-topics.md
##########
@@ -538,6 +538,31 @@ admin.topics().getMessageById(topic, ledgerId, entryId);
<!--END_DOCUSAURUS_CODE_TABS-->
+### Get message ID
+
+You can get message id published at or just after the given datetime.
Review comment:
This doc is for user, it's more detailed. Modified the previous one in new commit. Please check it out.
--
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
[GitHub] [pulsar] Jason918 commented on a change in pull request #11139: [Issue 10896] add get message id by timestamp
Posted by GitBox <gi...@apache.org>.
Jason918 commented on a change in pull request #11139:
URL: https://github.com/apache/pulsar/pull/11139#discussion_r662759984
##########
File path: site2/docs/admin-api-topics.md
##########
@@ -538,6 +538,31 @@ admin.topics().getMessageById(topic, ledgerId, entryId);
<!--END_DOCUSAURUS_CODE_TABS-->
+### Get message ID
+
+You can get message id published at or just after the given datetime.
+
+<!--DOCUSAURUS_CODE_TABS-->
+<!--pulsar-admin-->
+```shell
+$ ./bin/pulsar-admin topics get-message-id \
Review comment:
@Anonymitaet OK, I have commited this change. Please take a look.
--
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
[GitHub] [pulsar] codelipenghui commented on a change in pull request #11139: [Issue 10896] add get message id by timestamp
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on a change in pull request #11139:
URL: https://github.com/apache/pulsar/pull/11139#discussion_r660397751
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/PersistentTopicsTest.java
##########
@@ -796,4 +798,55 @@ public void testSetReplicatedSubscriptionStatus() {
Assert.assertEquals(responseCaptor.getValue().getStatus(), Response.Status.NO_CONTENT.getStatusCode());
}
+ @Test
+ public void testGetMessageIdByTimestamp() throws Exception {
+ TenantInfoImpl tenantInfo = new TenantInfoImpl(Sets.newHashSet("role1", "role2"), Sets.newHashSet("test"));
+ admin.tenants().createTenant("tenant-xyz", tenantInfo);
+ admin.namespaces().createNamespace("tenant-xyz/ns-abc", Sets.newHashSet("test"));
+ final String topicName = "persistent://tenant-xyz/ns-abc/testGetMessageIdByTimestamp";
+ admin.topics().createNonPartitionedTopic(topicName);
+
+ AtomicLong publishTime = new AtomicLong(0);
+ ProducerBase<byte[]> producer = (ProducerBase<byte[]>) pulsarClient.newProducer().topic(topicName)
+ .enableBatching(false)
+ .intercept(new ProducerInterceptor() {
+ @Override
+ public void close() {
+
+ }
+
+ @Override
+ public boolean eligible(Message message) {
+ return true;
+ }
+
+ @Override
+ public Message beforeSend(Producer producer, Message message) {
+ return message;
+ }
+
+ @Override
+ public void onSendAcknowledgement(Producer producer, Message message, MessageId msgId,
+ Throwable exception) {
+ publishTime.set(message.getPublishTime());
+ }
+ })
+ .create();
+
+ MessageId id1 = producer.send("test1".getBytes());
+ long publish1 = publishTime.get();
+
+ Thread.sleep(10);
Review comment:
Can be instead by Awaitibility? Use sleep here will introduce a flaky test.
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/PersistentTopics.java
##########
@@ -1515,6 +1515,39 @@ public void getMessageById(
}
}
+ @GET
+ @Path("/{tenant}/{namespace}/{topic}/messageid/{timestamp}")
+ @ApiOperation(value = "Get message id at or after this absolute timestamp (in ms).")
+ @ApiResponses(value = {
+ @ApiResponse(code = 307, message = "Current broker doesn't serve the namespace of this topic"),
+ @ApiResponse(code = 401, message = "Don't have permission to administrate resources on this tenant or"
+ + "subscriber is not authorized to access this operation"),
+ @ApiResponse(code = 403, message = "Don't have admin permission"),
+ @ApiResponse(code = 404, message = "Topic does not exist"),
+ @ApiResponse(code = 405, message = "Topic is not non-partitioned and persistent"),
+ @ApiResponse(code = 412, message = "Topic name is not valid"),
+ @ApiResponse(code = 500, message = "Internal server error"),
+ @ApiResponse(code = 503, message = "Failed to validate global cluster configuration")})
+ public void getMessageIdByTimestamp(
+ @Suspended final AsyncResponse asyncResponse,
+ @ApiParam(value = "Specify the tenant", required = true)
+ @PathParam("tenant") String tenant,
+ @ApiParam(value = "Specify the namespace", required = true)
+ @PathParam("namespace") String namespace,
+ @ApiParam(value = "Specify topic name", required = true)
+ @PathParam("topic") @Encoded String encodedTopic,
+ @ApiParam(value = "Specify the timestamp", required = true)
+ @PathParam("timestamp") long timestamp,
+ @ApiParam(value = "Is authentication required to perform this operation")
+ @QueryParam("authoritative") @DefaultValue("false") boolean authoritative) {
+ try {
+ validateTopicName(tenant, namespace, encodedTopic);
+ internalGetMessageIdByTimestamp(asyncResponse, timestamp, authoritative);
+ } catch (Exception e) {
+ asyncResponse.resume(new RestException(e));
Review comment:
the `internalGetMessageIdByTimestamp` will throw RestException, so this will be a problem when resume the asyncResponse by newRestException(RestException)
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
##########
@@ -2328,6 +2330,61 @@ public void readEntryComplete(Entry entry, Object ctx) {
}
}
+ protected void internalGetMessageIdByTimestamp(AsyncResponse asyncResponse, long timestamp, boolean authoritative) {
Review comment:
It's better to return CompletableFuture<MessageIdImpl> directly, it will make the code easier to read. you can see `PersistentTopics.setMaxSubscriptionsPerTopic`
--
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
[GitHub] [pulsar] codelipenghui merged pull request #11139: [Issue 10896] add get message id by timestamp
Posted by GitBox <gi...@apache.org>.
codelipenghui merged pull request #11139:
URL: https://github.com/apache/pulsar/pull/11139
--
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
[GitHub] [pulsar] Anonymitaet commented on a change in pull request #11139: [Issue 10896] add get message id by timestamp
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on a change in pull request #11139:
URL: https://github.com/apache/pulsar/pull/11139#discussion_r662065910
##########
File path: site2/docs/admin-api-topics.md
##########
@@ -538,6 +538,31 @@ admin.topics().getMessageById(topic, ledgerId, entryId);
<!--END_DOCUSAURUS_CODE_TABS-->
+### Get message ID
+
+You can get message id published at or just after the given datetime.
+
+<!--DOCUSAURUS_CODE_TABS-->
+<!--pulsar-admin-->
+```shell
+$ ./bin/pulsar-admin topics get-message-id \
Review comment:
@Jason918 we're removing the $ (though there are still some $ exist), please keep consistent
--
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
[GitHub] [pulsar] Anonymitaet commented on a change in pull request #11139: [Issue 10896] add get message id by timestamp
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on a change in pull request #11139:
URL: https://github.com/apache/pulsar/pull/11139#discussion_r660453233
##########
File path: site2/docs/admin-api-topics.md
##########
@@ -538,6 +538,31 @@ admin.topics().getMessageById(topic, ledgerId, entryId);
<!--END_DOCUSAURUS_CODE_TABS-->
+### Get message ID
+
+You can get message id published at or just after the given datetime.
+
+<!--DOCUSAURUS_CODE_TABS-->
+<!--pulsar-admin-->
+```shell
+$ ./bin/pulsar-admin topics get-message-id \
Review comment:
```suggestion
./bin/pulsar-admin topics get-message-id \
```
Remove $ so that users can copy the command directly and conveniently.
##########
File path: pulsar-client-admin-api/src/main/java/org/apache/pulsar/client/admin/Topics.java
##########
@@ -1349,6 +1349,28 @@ void expireMessagesForAllSubscriptions(String topic, long expireTimeInSeconds)
*/
CompletableFuture<Message<byte[]>> getMessageByIdAsync(String topic, long ledgerId, long entryId);
+ /**
+ * Get message id by timestamp.
Review comment:
```suggestion
* Get message ID by timestamp.
```
Pls pay attention to capitalization and correct all occurrences, thanks.
##########
File path: site2/docs/admin-api-topics.md
##########
@@ -538,6 +538,31 @@ admin.topics().getMessageById(topic, ledgerId, entryId);
<!--END_DOCUSAURUS_CODE_TABS-->
+### Get message ID
+
+You can get message id published at or just after the given datetime.
Review comment:
Why the description here is different from the previous one (`Get message ID by timestamp`)?
--
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
[GitHub] [pulsar] Jason918 commented on pull request #11139: [Issue 10896] add get message id by timestamp
Posted by GitBox <gi...@apache.org>.
Jason918 commented on pull request #11139:
URL: https://github.com/apache/pulsar/pull/11139#issuecomment-870311368
/pulsarbot run-failure-checks
--
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
[GitHub] [pulsar] Jason918 commented on a change in pull request #11139: [Issue 10896] add get message id by timestamp
Posted by GitBox <gi...@apache.org>.
Jason918 commented on a change in pull request #11139:
URL: https://github.com/apache/pulsar/pull/11139#discussion_r660581016
##########
File path: site2/docs/admin-api-topics.md
##########
@@ -538,6 +538,31 @@ admin.topics().getMessageById(topic, ledgerId, entryId);
<!--END_DOCUSAURUS_CODE_TABS-->
+### Get message ID
+
+You can get message id published at or just after the given datetime.
+
+<!--DOCUSAURUS_CODE_TABS-->
+<!--pulsar-admin-->
+```shell
+$ ./bin/pulsar-admin topics get-message-id \
Review comment:
There are $ in all the other shell demos. You sure we should do that? Either is fine by me.
--
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
[GitHub] [pulsar] codelipenghui merged pull request #11139: [Issue 10896] add get message id by timestamp
Posted by GitBox <gi...@apache.org>.
codelipenghui merged pull request #11139:
URL: https://github.com/apache/pulsar/pull/11139
--
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
[GitHub] [pulsar] Jason918 commented on pull request #11139: [Issue 10896] add get message id by timestamp
Posted by GitBox <gi...@apache.org>.
Jason918 commented on pull request #11139:
URL: https://github.com/apache/pulsar/pull/11139#issuecomment-872949392
@eolivelli Hi, Thank you for the reminding, I have added testGetBatchMessageIdByTimestamp for batch message case.
Batch messages are stored in the same entry, which should be considered stored at the same time. So only message id containing ledgerId and entryId is returned.
--
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
[GitHub] [pulsar] Jason918 commented on pull request #11139: [Issue 10896] add get message id by timestamp
Posted by GitBox <gi...@apache.org>.
Jason918 commented on pull request #11139:
URL: https://github.com/apache/pulsar/pull/11139#issuecomment-870311368
--
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
[GitHub] [pulsar] Jason918 commented on a change in pull request #11139: [Issue 10896] add get message id by timestamp
Posted by GitBox <gi...@apache.org>.
Jason918 commented on a change in pull request #11139:
URL: https://github.com/apache/pulsar/pull/11139#discussion_r660581016
##########
File path: site2/docs/admin-api-topics.md
##########
@@ -538,6 +538,31 @@ admin.topics().getMessageById(topic, ledgerId, entryId);
<!--END_DOCUSAURUS_CODE_TABS-->
+### Get message ID
+
+You can get message id published at or just after the given datetime.
+
+<!--DOCUSAURUS_CODE_TABS-->
+<!--pulsar-admin-->
+```shell
+$ ./bin/pulsar-admin topics get-message-id \
Review comment:
There are $ in all the other shell demos. You sure we should do that? Either is fine by me.
##########
File path: site2/docs/admin-api-topics.md
##########
@@ -538,6 +538,31 @@ admin.topics().getMessageById(topic, ledgerId, entryId);
<!--END_DOCUSAURUS_CODE_TABS-->
+### Get message ID
+
+You can get message id published at or just after the given datetime.
Review comment:
This doc is for user, it's more detailed. Modified the previous one in new commit. Please check it out.
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/PersistentTopicsTest.java
##########
@@ -796,4 +798,55 @@ public void testSetReplicatedSubscriptionStatus() {
Assert.assertEquals(responseCaptor.getValue().getStatus(), Response.Status.NO_CONTENT.getStatusCode());
}
+ @Test
+ public void testGetMessageIdByTimestamp() throws Exception {
+ TenantInfoImpl tenantInfo = new TenantInfoImpl(Sets.newHashSet("role1", "role2"), Sets.newHashSet("test"));
+ admin.tenants().createTenant("tenant-xyz", tenantInfo);
+ admin.namespaces().createNamespace("tenant-xyz/ns-abc", Sets.newHashSet("test"));
+ final String topicName = "persistent://tenant-xyz/ns-abc/testGetMessageIdByTimestamp";
+ admin.topics().createNonPartitionedTopic(topicName);
+
+ AtomicLong publishTime = new AtomicLong(0);
+ ProducerBase<byte[]> producer = (ProducerBase<byte[]>) pulsarClient.newProducer().topic(topicName)
+ .enableBatching(false)
+ .intercept(new ProducerInterceptor() {
+ @Override
+ public void close() {
+
+ }
+
+ @Override
+ public boolean eligible(Message message) {
+ return true;
+ }
+
+ @Override
+ public Message beforeSend(Producer producer, Message message) {
+ return message;
+ }
+
+ @Override
+ public void onSendAcknowledgement(Producer producer, Message message, MessageId msgId,
+ Throwable exception) {
+ publishTime.set(message.getPublishTime());
+ }
+ })
+ .create();
+
+ MessageId id1 = producer.send("test1".getBytes());
+ long publish1 = publishTime.get();
+
+ Thread.sleep(10);
Review comment:
This sleep is not waiting for some other operation to finish. It's used for making sure that the publish_time of the two messages is different. Any suggestions how we can do this in a better way?
--
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
[GitHub] [pulsar] codelipenghui commented on a change in pull request #11139: [Issue 10896] add get message id by timestamp
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on a change in pull request #11139:
URL: https://github.com/apache/pulsar/pull/11139#discussion_r660397751
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/PersistentTopicsTest.java
##########
@@ -796,4 +798,55 @@ public void testSetReplicatedSubscriptionStatus() {
Assert.assertEquals(responseCaptor.getValue().getStatus(), Response.Status.NO_CONTENT.getStatusCode());
}
+ @Test
+ public void testGetMessageIdByTimestamp() throws Exception {
+ TenantInfoImpl tenantInfo = new TenantInfoImpl(Sets.newHashSet("role1", "role2"), Sets.newHashSet("test"));
+ admin.tenants().createTenant("tenant-xyz", tenantInfo);
+ admin.namespaces().createNamespace("tenant-xyz/ns-abc", Sets.newHashSet("test"));
+ final String topicName = "persistent://tenant-xyz/ns-abc/testGetMessageIdByTimestamp";
+ admin.topics().createNonPartitionedTopic(topicName);
+
+ AtomicLong publishTime = new AtomicLong(0);
+ ProducerBase<byte[]> producer = (ProducerBase<byte[]>) pulsarClient.newProducer().topic(topicName)
+ .enableBatching(false)
+ .intercept(new ProducerInterceptor() {
+ @Override
+ public void close() {
+
+ }
+
+ @Override
+ public boolean eligible(Message message) {
+ return true;
+ }
+
+ @Override
+ public Message beforeSend(Producer producer, Message message) {
+ return message;
+ }
+
+ @Override
+ public void onSendAcknowledgement(Producer producer, Message message, MessageId msgId,
+ Throwable exception) {
+ publishTime.set(message.getPublishTime());
+ }
+ })
+ .create();
+
+ MessageId id1 = producer.send("test1".getBytes());
+ long publish1 = publishTime.get();
+
+ Thread.sleep(10);
Review comment:
Can be instead by Awaitibility? Use sleep here will introduce a flaky test.
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/PersistentTopics.java
##########
@@ -1515,6 +1515,39 @@ public void getMessageById(
}
}
+ @GET
+ @Path("/{tenant}/{namespace}/{topic}/messageid/{timestamp}")
+ @ApiOperation(value = "Get message id at or after this absolute timestamp (in ms).")
+ @ApiResponses(value = {
+ @ApiResponse(code = 307, message = "Current broker doesn't serve the namespace of this topic"),
+ @ApiResponse(code = 401, message = "Don't have permission to administrate resources on this tenant or"
+ + "subscriber is not authorized to access this operation"),
+ @ApiResponse(code = 403, message = "Don't have admin permission"),
+ @ApiResponse(code = 404, message = "Topic does not exist"),
+ @ApiResponse(code = 405, message = "Topic is not non-partitioned and persistent"),
+ @ApiResponse(code = 412, message = "Topic name is not valid"),
+ @ApiResponse(code = 500, message = "Internal server error"),
+ @ApiResponse(code = 503, message = "Failed to validate global cluster configuration")})
+ public void getMessageIdByTimestamp(
+ @Suspended final AsyncResponse asyncResponse,
+ @ApiParam(value = "Specify the tenant", required = true)
+ @PathParam("tenant") String tenant,
+ @ApiParam(value = "Specify the namespace", required = true)
+ @PathParam("namespace") String namespace,
+ @ApiParam(value = "Specify topic name", required = true)
+ @PathParam("topic") @Encoded String encodedTopic,
+ @ApiParam(value = "Specify the timestamp", required = true)
+ @PathParam("timestamp") long timestamp,
+ @ApiParam(value = "Is authentication required to perform this operation")
+ @QueryParam("authoritative") @DefaultValue("false") boolean authoritative) {
+ try {
+ validateTopicName(tenant, namespace, encodedTopic);
+ internalGetMessageIdByTimestamp(asyncResponse, timestamp, authoritative);
+ } catch (Exception e) {
+ asyncResponse.resume(new RestException(e));
Review comment:
the `internalGetMessageIdByTimestamp` will throw RestException, so this will be a problem when resume the asyncResponse by newRestException(RestException)
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
##########
@@ -2328,6 +2330,61 @@ public void readEntryComplete(Entry entry, Object ctx) {
}
}
+ protected void internalGetMessageIdByTimestamp(AsyncResponse asyncResponse, long timestamp, boolean authoritative) {
Review comment:
It's better to return CompletableFuture<MessageIdImpl> directly, it will make the code easier to read. you can see `PersistentTopics.setMaxSubscriptionsPerTopic`
--
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
[GitHub] [pulsar] Jason918 commented on pull request #11139: [Issue 10896] add get message id by timestamp
Posted by GitBox <gi...@apache.org>.
Jason918 commented on pull request #11139:
URL: https://github.com/apache/pulsar/pull/11139#issuecomment-870371894
@codelipenghui Hi, can you please take a look?
--
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
[GitHub] [pulsar] eolivelli commented on pull request #11139: [Issue 10896] add get message id by timestamp
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #11139:
URL: https://github.com/apache/pulsar/pull/11139#issuecomment-872957285
> Batch messages are stored in the same entry, which should be considered stored at the same time
Very interesting point !
Thanks for the new test
--
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
[GitHub] [pulsar] Anonymitaet commented on a change in pull request #11139: [Issue 10896] add get message id by timestamp
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on a change in pull request #11139:
URL: https://github.com/apache/pulsar/pull/11139#discussion_r660453233
##########
File path: site2/docs/admin-api-topics.md
##########
@@ -538,6 +538,31 @@ admin.topics().getMessageById(topic, ledgerId, entryId);
<!--END_DOCUSAURUS_CODE_TABS-->
+### Get message ID
+
+You can get message id published at or just after the given datetime.
+
+<!--DOCUSAURUS_CODE_TABS-->
+<!--pulsar-admin-->
+```shell
+$ ./bin/pulsar-admin topics get-message-id \
Review comment:
```suggestion
./bin/pulsar-admin topics get-message-id \
```
Remove $ so that users can copy the command directly and conveniently.
##########
File path: pulsar-client-admin-api/src/main/java/org/apache/pulsar/client/admin/Topics.java
##########
@@ -1349,6 +1349,28 @@ void expireMessagesForAllSubscriptions(String topic, long expireTimeInSeconds)
*/
CompletableFuture<Message<byte[]>> getMessageByIdAsync(String topic, long ledgerId, long entryId);
+ /**
+ * Get message id by timestamp.
Review comment:
```suggestion
* Get message ID by timestamp.
```
Pls pay attention to capitalization and correct all occurrences, thanks.
##########
File path: site2/docs/admin-api-topics.md
##########
@@ -538,6 +538,31 @@ admin.topics().getMessageById(topic, ledgerId, entryId);
<!--END_DOCUSAURUS_CODE_TABS-->
+### Get message ID
+
+You can get message id published at or just after the given datetime.
Review comment:
Why the description here is different from the previous one (`Get message ID by timestamp`)?
--
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