[AMQ-9646] Support selecting specific messages for command line backup

[mattrpav]

• Tested indexed backup
• Tested messageId range backup

Example usage:

./bin/activemq backup --filename=backup.tar.gz --indexes=0,1,5,7,1000,1001     

[cshannon]

I don't really get the use case to be giving specific index values from the order index. How would people know what index to use for the messages they want?

It would seem much more beneficial to use something like message ids to find the messages to backup.

[mattrpav]

@cshannon indexes are handy when performing surgery to get specific messages out (may already know position based on browse or console UI), or when a user wishes to get the last message off the queue.

MessageId based is a good idea, that has been added in the latest commit.

[cshannon]

I only started looking at this but one thing I wanted to point out that could be changed as part of this and something I missed before when #1258 was done.... instead of having a position counter and having to scan through couldn't you just set the batch for recovery at the offset location? Also, the offset should probably be a long and not an int in the method signature because long is used by the order index.

I haven't tested it super well other than running the KahaDBOffsetRecoveryListenerTest, but here is an example against the main branch of what I mean: cshannon@648a580

The other impls (temp kaha, memory) may be able to be changed too, not sure.

[cshannon]

The biggest issue I see is if you are trying to do "online" backups, every time you are calling recover messages you are messing with the internal state of the queue if you are hopping around to select certain messages. You could avoid this by using a different MessageOrderCursor

activemq/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/MessageDatabase.java

Line 3989 in a4a4227

Iterator<Entry<Long, MessageKeys>> iterator(Transaction tx, MessageOrderCursor m) throws IOException{

[cshannon]

I still need to review it a bit more but made some initial quick comments as I was scrolling.

Something else is I'm thinking that the recoverNextMessages() method may keep expanding with more and more flags/options so instead of having several it might be better to use a configuration object and the builder pattern to make it simpler.

Something like the following, the naming can be changed, I wasn't sure what to call it.

default void recoverNextMessages(MessageRecoveryConfig config, MessageRecoveryListener listener) throws Exception {
    //...
}

static class MessageRecoveryConfig {
    final String startMsgId;
    final String endMsgId;
    final int maxReturned;
    final boolean useDedicatedCursor;

    private MessageRecoveryConfig(String startMsgId, String endMsgId, int maxReturned,
      boolean useDedicatedCursor) {

        // These would need to be validated to make sure they are compatible
        this.startMsgId = startMsgId;
        this.endMsgId = endMsgId;
        this.maxReturned = maxReturned;
        this.useDedicatedCursor = useDedicatedCursor;
    }
    // Todo add anymore settings later
   // Todo add getters, etc

   // Create a builder object to make it easier to configure
    static class Builder {

       private String startMessageId;
       private boolean useDedicatedCursor = true;
       ///....
       // Add more variables, maybe set defaults here

       public Builder startMessageId(String startMsgId) {
           this.startMessageId = startMsgId;
           return this;
       }

       // Add more setters

       public MessageRecoveryConfig build() {
           return new MessageRecoveryConfig(startMessageId, endMsgId, maxReturned,
             build().useDedicatedCursor);
       }
   }
}

[mattrpav]

static class MessageRecoveryConfig {
final String startMsgId;
...

I was thinking a MessageRecoveryContext would make sense where it could be an extension of the listener w/ config options.

Then the method could take one parameter: MessageRecoveryContext ..

static class MessageRecoveryContext {

   final MessageRecoveryListener listener;
   final long offset;
   final String startMsgId ..

However, this also introduces complexity of deciding which fields to use.. the offset+maxRecovered and/or the startMsgId and/or the endMsgId.. etc.

[cshannon]

static class MessageRecoveryConfig {
final String startMsgId;
...

I was thinking a MessageRecoveryContext would make sense where it could be an extension of the listener w/ config options.

Then the method could take one parameter: MessageRecoveryContext ..

static class MessageRecoveryContext {

   final MessageRecoveryListener listener;
   final long offset;
   final String startMsgId ..

However, this also introduces complexity of deciding which fields to use.. the offset+maxRecovered and/or the startMsgId and/or the endMsgId.. etc.

Yeah, you have to get into validation in that case. Really the question is do you plan to keep expanding the features? I don't think we want to get into a situation where we have 20 different overloaded methods with different argument combinations. The more flags we have the better it is to just make some sort of configuration type object and then just validate the flag combinations that are set are correct.