Koado commented on code in PR #7082:
URL: https://github.com/apache/rocketmq/pull/7082#discussion_r1278309596
##########
broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java:
##########
@@ -988,7 +988,15 @@ private RemotingCommand
rewriteRequestForStaticTopic(SearchOffsetRequestHeader r
continue;
}
if (mappingDetail.getBname().equals(item.getBname())) {
- offset =
this.brokerController.getMessageStore().getOffsetInQueueByTime(mappingContext.getTopic(),
item.getQueueId(), timestamp);
+ MessageStore messageStore =
this.brokerController.getMessageStore();
+ if (messageStore instanceof DefaultMessageStore) {
+ // get offset with specific boundary type
+ offset = ((DefaultMessageStore)
messageStore).getOffsetInQueueByTime(requestHeader.getTopic(),
+ requestHeader.getQueueId(),
requestHeader.getTimestamp(), requestHeader.getBoundaryType());
+ } else {
+ offset =
messageStore.getOffsetInQueueByTime(requestHeader.getTopic(),
requestHeader.getQueueId(),
+ requestHeader.getTimestamp());
+ }
Review Comment:
Yes, I also wanted to add an interface directly at the beginning. But later
I found that the **BoundaryType** used in **TieredMessageStore** belongs to
package _org.apache.rocketmq.tieredstore.common_ and the **BoundaryType** used
in **DefaultMessageStore** belongs to package _org.apache.rocketmq.common_. So
if I wanna to abstract an interface, the argument of the interface might be
boundaryTypeName(String), which means that the enum class is converted to a
string, or the string is converted to the enum class, each time the argument is
passed and the argument is fetched.
Or maybe I should unify the two enum classes？What do you think I should do
in this situation?
##########
broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java:
##########
@@ -988,7 +988,15 @@ private RemotingCommand
rewriteRequestForStaticTopic(SearchOffsetRequestHeader r
continue;
}
if (mappingDetail.getBname().equals(item.getBname())) {
- offset =
this.brokerController.getMessageStore().getOffsetInQueueByTime(mappingContext.getTopic(),
item.getQueueId(), timestamp);
+ MessageStore messageStore =
this.brokerController.getMessageStore();
+ if (messageStore instanceof DefaultMessageStore) {
+ // get offset with specific boundary type
+ offset = ((DefaultMessageStore)
messageStore).getOffsetInQueueByTime(requestHeader.getTopic(),
+ requestHeader.getQueueId(),
requestHeader.getTimestamp(), requestHeader.getBoundaryType());
+ } else {
+ offset =
messageStore.getOffsetInQueueByTime(requestHeader.getTopic(),
requestHeader.getQueueId(),
+ requestHeader.getTimestamp());
+ }
Review Comment:
Yes, I also wanted to add an interface directly at the beginning. But later
I found that the **BoundaryType** used in **TieredMessageStore** belongs to
package _org.apache.rocketmq.tieredstore.common_ and the **BoundaryType** used
in **DefaultMessageStore** belongs to package _org.apache.rocketmq.common_. So
if I wanna to abstract an interface, the argument of the interface might be
boundaryTypeName(String), which means that the enum class is converted to a
string, or the string is converted to the enum class, each time the argument is
passed and the argument is fetched.
Or maybe I should unify the two enum classes？What do you think I should do
in this situation?
--
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-unsubscr...@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
