cshannon commented on code in PR #1377:
URL: https://github.com/apache/activemq/pull/1377#discussion_r1931149205
##########
activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/KahaDBStore.java:
##########
@@ -733,35 +733,95 @@ public void execute(Transaction tx) throws Exception {
}
@Override
- public void recoverNextMessages(final int offset, final int
maxReturned, final MessageRecoveryListener listener) throws Exception {
+ public void recoverNextMessages(final long offset, final int
maxReturned, final MessageRecoveryListener listener) throws Exception {
+ recoverNextMessages(offset, maxReturned, listener, false);
Review Comment:
I think the default should be true for a separate cursor, I think it's a bad
idea to be changing the state by default for a backup
##########
activemq-console/src/main/java/org/apache/activemq/console/command/store/StoreBackup.java:
##########
@@ -178,7 +194,19 @@ public boolean recoverMessage(Message message) throws
IOException {
return true;
}
};
- if(offset != null) {
+ if(startMsgId != null || endMsgId != null) {
+ System.out.println("Backing up from startMsgId: " +
startMsgId + " to endMsgId: " + endMsgId);
Review Comment:
I am thinking for System.out it may be better to use slf4j and log4j2 and
configure that for writing to stdout vs directly using System.out.println
##########
activemq-console/src/main/java/org/apache/activemq/console/command/store/StoreBackup.java:
##########
@@ -88,6 +96,14 @@ public void execute() throws Exception {
throw new Exception("optional --offset and --count must be
specified together");
}
+ if ((startMsgId != null || endMsgId != null) && queue == null) {
+ throw new Exception("optional --queue must be specified when using
startMsgId or endMsgId");
Review Comment:
All these exceptions should be more specific, such as
IllegalArgumentException.
All the checks here would benefit from something like
[Preconditions](https://guava.dev/releases/snapshot/api/docs/com/google/common/base/Preconditions.html)
and the checkArgument() methods if we can use guava.
##########
activemq-console/src/main/java/org/apache/activemq/console/command/store/StoreBackup.java:
##########
@@ -178,7 +194,19 @@ public boolean recoverMessage(Message message) throws
IOException {
return true;
}
};
- if(offset != null) {
+ if(startMsgId != null || endMsgId != null) {
+ System.out.println("Backing up from startMsgId: " +
startMsgId + " to endMsgId: " + endMsgId);
+ queue.recoverNextMessages(startMsgId, endMsgId, (count !=
null ? count : Integer.MAX_VALUE), queueRecoveryListener, true);
+ } else if(indexesList != null) {
+ System.out.println("Backing up using indexes count: " +
indexesList.size());
+ for(int idx : indexesList) {
+ if(idx < 0) {
Review Comment:
Why would there be a negative index? I would think this should be an
exception and not just a continue
--
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: gitbox-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
