fix sequence in meta proto

[corylanou]

We skipped a sequence for some reason. Assuming this was not intentional.

The messages have the right sequence later in the file. This fix aligns the sequences back up.

@nathanielc can you verify this?

[dgnorton]

@corylanou Will this break existing DBs?

[corylanou]

I'm not sure, it may, but I think that might be better than allowing the bug to live forever. It should only be part of the nightly for now and not a previous release. @pauldix thoughts?

[otoolep]

Does it matter? Just use 21 next time we have a message, right?

[corylanou]

@otoolep if you look further down, the messages use 121, and 122, not 122, and 123.. so this is likely going to be a bug some day for us I suspect.

[otoolep]

I took a look at the code.

We store these commands in the Raft log AFAIK. Therefore this change is not backwards compatible and will force people to blow away their systems on upgrade. Normally this change would be a non-starter, but because no-one should have run these commands, we should be OK.

[otoolep]

At least, that is my reading from a quick glance at the code. Since this is code that should never have been run yet, +1 from me.

[corylanou]

For context the reason I caught this is I was doing a rebase on my branch where I used 21 and 121, so if we don't fix it, I'll end up with 21 and then 123 for the corresponding message. Effectively we'll have 3 enums that don't line up with the corresponding message sequences.

We would have these enums:

RemovePeerCommand                = 21;
CreateSubscriptionCommand        = 22;
DropSubscriptionCommand          = 23;

And these messages:

message CreateSubscriptionCommand {
    extend Command {
        optional CreateSubscriptionCommand command = 121;
    }
    required string Name = 1;
    required string Database = 2;
    required string RetentionPolicy = 3;
    required string Mode = 4;
    repeated string Destinations = 5;

}

message DropSubscriptionCommand {
    extend Command {
        optional DropSubscriptionCommand command = 122;
    }
    required string Name = 1;
    required string Database = 2;
    required string RetentionPolicy = 3;
}
message RemovePeerCommand {
    extend Command {
        optional RemovePeerCommand command = 123;
    }
        required uint64 ID = 1;
        required string Addr = 2;
}

[nathanielc]

@corylanou The fix looks good to me. Where did 'drop server' go? That was the one that got added while I re-based and so I bumped up the sequence. Just want to make sure that it wasn't accidentally removed.

[corylanou]

@nathanielc It got reverted in a future pull. That makes sense why that got skipped then. The real issue is that the message numbers didn't line up with the enums. If it had, this would be a non-issue as we could of just used that on the next PR. Regardless, this should get everything synced up. Thanks!