tt: add command `tt upgrade`

mandesero commented 2 months ago

`tt upgrade` command steps:

For each replicaset:
- On the master instance:
  1. Execute the following commands sequentially:
```
box.schema.upgrade()
box.snapshot()
```
- On each replica:
  1. Wait until the replica applies all transactions produced by box.schema.upgrade() on the master by comparing the vector clocks (vclock).
  2. Once synchronization is confirmed, execute the following command on the replica:
```
box.snapshot()
```

If any errors occur during the upgrade process, the process will stop and an error report will be generated.

The replica is waiting for synchronization for Timeout seconds. The default value for Timeout is 5 seconds, but you can specify it manually using the --timeout option.

$tt upgrade [<APP_NAME>] --timeout 10

You can also specify which replicaset(s) to upgrade by using the --replicaset option.

$tt upgrade [<APP_NAME>] --replicaset <RS_NAME_1> -r <RS_NAME_2> ...

mandesero commented 2 months ago

Examples:

Case 1: OK

INSTANCE            STATUS   PID    MODE
 app2:storage-002-b  RUNNING  12556  RO
 app2:router-001-a   RUNNING  12560  RW
 app2:storage-001-a  RUNNING  12567  RO
 app2:storage-001-b  RUNNING  12554  RO
 app2:storage-002-a  RUNNING  12555  RW

 $ tt upgrade app2
• storage-001: ok
• storage-002: ok
• router-001: ok

Case 2: More than one master in the same replicaset

INSTANCE            STATUS   PID    MODE
 app2:storage-002-b  RUNNING  12556  RO
 app2:router-001-a   RUNNING  12560  RW
 app2:storage-001-a  RUNNING  12567  RO
 app2:storage-001-b  RUNNING  12554  RW
 app2:storage-002-a  RUNNING  12555  RW

 $ tt upgrade app2
• storage-001: error
   ⨯ [storage-001]: app2:storage-001-a and app2:storage-001-b are both masters

Case 3: LSN didn't update

$ tt upgrade app2
• storage-001: error
   ⨯ [storage-001]: LSN wait timeout: error waiting LSN 2003085 in vclock component 1 on app2:storage-001-b: time quota 5 seconds exceeded

Case 4: There is a replicaset that does not have a master

 INSTANCE            STATUS   PID    MODE
 app2:storage-002-b  RUNNING  12556  RO
 app2:router-001-a   RUNNING  12560  RW
 app2:storage-001-a  RUNNING  12567  RO
 app2:storage-001-b  RUNNING  12554  RO
 app2:storage-002-a  RUNNING  12555  RO

$ tt upgrade app2
• storage-001: error
   ⨯ [storage-001]: has not master instance

Case 5: A non-existent replicaset was specified

$ tt upgrade app2 --replicaset foo
   ⨯ replicaset with alias "foo" doesn't exist

psergee commented 2 months ago

⨯ [storage-001]: LSN wait timeout: error waiting LSN 2003085 in vclock component 1

This message about LSN seems a bit confusing to me. Maybe add something like "Schema upgrade replication timeout exceeded:" before "error waiting LSN..." to make it clearer what has just happened?

Totktonada commented 2 months ago

We have a special subcomand tt replicaset for such staff. I propose to add a tt replicaset upgrade command instead of the tt upgrade.

@oleg-jukovec Is it a decision or a proposal to discuss?

If the former, OK.

If the latter I would highlight my opinion on it.

We also have tt cluster and the command relates to the cluster in some sense -- it may work with several replicasets. From a user point of view tt cluster and tt replicaset looks very close and where a particular subcommand resides seems arbitrary. And there is tt cluster replicaset...
As a user who wish to perform an upgrade I would look for it on the top level first and, if it is not there, would look in tt cluster, tt replicaset, tt cluster replicaset. The verb (the action) looks more important here. I'm thinking on 'upgrading a cluster', not on 'do something with a cluster and this action is upgrade' (it reminds me find command syntax :)).
I see no point in these three namespaces, TBH. tt status is just tt status and nobody complains that it is not tt <..something..> status. I'm not a big fan of doing things less obvious for a user, because things were done in a non-obvious way before. Maybe it is a good time to start to change this approach piece-by-piece?
As a meaningful alternative I can propose tt admin upgrade, because the upgrade looks as a typical administration task. OTOH, tt is at all about administration and development tasks. So tt upgrade is still my favorite.

oleg-jukovec commented 2 months ago

We have a special subcomand tt replicaset for such staff. I propose to add a tt replicaset upgrade command instead of the tt upgrade.

@oleg-jukovec Is it a decision or a proposal to discuss?

If the former, OK.

If the latter I would highlight my opinion on it.

* We also have `tt cluster` and the command relates to the cluster in some sense -- it may work with several replicasets. From a user point of view `tt cluster` and `tt replicaset` looks very close and where a particular subcommand resides seems arbitrary. And there is `tt cluster replicaset`...

* As a user who wish to perform an upgrade I would look for it on the top level first and, if it is not there, would look in `tt cluster`, `tt replicaset`, `tt cluster replicaset`. The verb (the action) looks more important here. I'm thinking on 'upgrading a cluster', not on 'do something with a cluster and this action is upgrade' (it reminds me `find` command syntax :)).

* I see no point in these three namespaces, TBH. `tt status` is just `tt status` and nobody complains that it is not `tt <..something..> status`. I'm not a big fan of doing things less obvious for a user, because things were done in a non-obvious way before. Maybe it is a good time to start to change this approach piece-by-piece?

* As a meaningful alternative I can propose `tt admin upgrade`, because the upgrade looks as a typical administration task. OTOH, `tt` is at all about administration and development tasks. So `tt upgrade` is still my favorite.

I agree that the current naming tt replicaset, tt cluster, and tt cluster replicaset are confusing. This is a separate issue that we will try to fix in the next major release.

Currently, tt replicaset is used for managing a local cluster, including actions like bootstrap, vshard bootstrap, and rebootstrap. The upgrade command, in its current context, logically complements this set of commands.

I don’t see a point to introduce further confusion with individual commands/another approach or to add new sets of subcommands. This should be a centralized design decision that is outside the scope of this pull request.

Totktonada commented 2 months ago

Hm. My expectation is that the upgrade command is suitable for an arbitrary cluster, not only local cluster. (AFAIU, tt can switch to iproto for remote instances, at least in some commands.) Does it change its category from tt replicaset to something different?

This should be a centralized design decision that is outside the scope of this pull request.

OK. I'm a bit worrying about a need for compatibility aliases even for such a new functionality. However, I understand the wish to separate the problems.

It sounds like a decision, so I'll not insist on my feelings here anymore (unless the discussion will gain new points from others).

psergee commented 2 months ago

Regarding the tt upgrade command compared to the tt replicaset upgrade command, I agree with Oleg that it should be a subcommand of the replicaset command. I still find it confusing to use the cluster, cluster replicaset, and replicaset commands, and I hope that they will be reorganized in a more user-friendly manner in the future. Yes, it could even be called the tt admin command or something similar.

mandesero commented 1 month ago

The command is now called tt replicaset upgrade [<APP_NAME> | <URI>] [flags] (No tests yet). The command works similarly to tt replicaset status [..args..], collecting information about replicasets using the Discovery mechanism.

Changes:

The structure has been refactored to maximize reuse of the new functionality in tt replicaset downgrade.
An initial draft has been created for working with remote instances (getInstanceConnector). However, further discussion is needed to determine the best approach for implementing this functionality. The following issues are debatable:
- To connect to a remote instance using iproto, the user needs to grant the super or lua_eval role.
- Configuration of the remote cluster needs to be specified (likely requiring a configuration for Tarantool 3.x).
- There may be other factors to consider.

In the current version, there is already a (workaround-based) ability to perform an upgrade on a remote cluster (each replicaset separately). This uses a mechanism similar to tt replicaset status <URI>, where replicaset information is obtained from box.info.replication of the instance connected via .

For example, the application is deployed on the server 10.0.10.123 with the following configuration:

Config.yaml

```yaml credentials: users: client: password: 'secret' roles: [super] replicator: password: 'secret' roles: [replication] storage: password: 'secret' roles: [sharding] iproto: advertise: peer: login: replicator sharding: login: storage sharding: bucket_count: 3000 groups: storages: app: module: storage sharding: roles: [storage] replication: failover: manual replicasets: storage-001: leader: storage-001-a instances: storage-001-a: iproto: listen: - uri: 10.0.10.123:3301 storage-001-b: iproto: listen: - uri: 10.0.10.123:3302 storage-002: leader: storage-002-a instances: storage-002-a: iproto: listen: - uri: 10.0.10.123:3303 storage-002-b: iproto: listen: - uri: 10.0.10.123:3304 routers: app: module: router sharding: roles: [router] replicasets: router-001: instances: router-001-a: iproto: listen: - uri: 10.0.10.123:3305 ```

Upgrade commands:

$ tt replicaset upgrade tcp://client:secret@10.0.10.123:3301
• storage-001: ok

$ tt replicaset upgrade tcp://client:secret@10.0.10.123:3303
• storage-002: ok

tt replicaset upgrade tcp://client:secret@10.0.10.123:3305
• router-001: ok

However, this is likely not the ideal solution that we aim for, since this method requires a lot of manual work when upgrading a large cluster.

We can make the TCP connection a mock in this patch, support only upgrades on local instances. However, since the mechanism for connecting to remote instances already exists, we might want to implement the full upgrade functionality right away. I would like to hear your thoughts on this @oleg-jukovec @psergee.

oleg-jukovec commented 1 month ago

Please, rebase on the latest commit in the master branch.

mandesero commented 3 weeks ago

Please, rebase on the latest commit in the master branch.

Rebased.

mandesero commented 6 days ago

It was found that sometimes the replicaset name might not reach tt for some reason. I simplified the check in the test so that the test wouldn't be flaky.

For example:

• 5b3a3d0d-5ee2-40d1-989e-d4d68687581e: ok
• router-001: ok
• storage-001: ok

But should be:

• storage-001: ok
• storage-002: ok
• router-001: ok

tarantool / tt