Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tt migrations #4499

Merged
merged 16 commits into from
Oct 3, 2024
Merged

Add tt migrations #4499

merged 16 commits into from
Oct 3, 2024

Conversation

p7nov
Copy link
Contributor

@p7nov p7nov commented Sep 12, 2024

Comment on lines 264 to 265
Q: can any migrations in a batch complete successfully? If I apply 2 migrations and call
`tt migrations stop` after the first one is finished without errors, what are migration statuses?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DifferentialOrange please provide the answer.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the first migration has been applied and the second one is interrupted with tt migrations stop, the first one will have APPLIED status and the second one will be in APPLY_STARTED.

- command options ``--config-storage-username`` and ``--config-storage-password``
- the etcd URI, for example, ``https://user:pass@localhost:2379/myapp``

Q: which way has a higher priority?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DifferentialOrange please provide the answer.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The priority of credentials:
environment variables < command flags < URL credentials.

https://github.com/tarantool/tt-ee/blob/4c59e56529d6e4f4e4e514391510ebeef67f9d10/cli/migrations/cmd.go#L85-L86

Copy link
Member

@DifferentialOrange DifferentialOrange left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! Seems valid in general, have left several comments.

doc/tooling/tt_cli/migrations.rst Outdated Show resolved Hide resolved
doc/tooling/tt_cli/migrations.rst Outdated Show resolved Hide resolved
doc/tooling/tt_cli/migrations.rst Outdated Show resolved Hide resolved
.. warning::

Using the options that ignore checks when publishing migration may cause
migration inconsistency in the etcd storage.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not about etcd storage, it's more about the whole cluster. For example, using the --ignore-order-violation option in the case described above may result in the following:

  • replicaset 1 (was in the cluster from the start) has migration 003 applied before 002
  • replicaset 2 (was added later) has migration 002 applied before 003

For example, if 002 adds some data and 003 reverts it, the data will be different between replicasets. etcd storage will be fine (btw, it's just a key-value storage, it will always be fine), but the cluster schema isn't.

Comment on lines 264 to 265
Q: can any migrations in a batch complete successfully? If I apply 2 migrations and call
`tt migrations stop` after the first one is finished without errors, what are migration statuses?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the first migration has been applied and the second one is interrupted with tt migrations stop, the first one will have APPLIED status and the second one will be in APPLY_STARTED.

The centralized migration mechanism works with Tarantool EE clusters that:

- use etcd as a centralized configuration storage
- use the `CRUD <https://github.com/tarantool/crud>`__ module for data distribution
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once again we're in a weird situation when the module is actually out of support (in favor of crud-ee), but it's replacement is unavailable for users (it's removed from customer zone SDKs due to recent Team Product policies).

doc/platform/ddl_dml/migrations/upgrade_migrations_tt.rst Outdated Show resolved Hide resolved
--force-reapply

If execution of the incorrect migration version has failed, you may also need to add
the ``--ignore-preceding-status`` option:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--ignore-preceding-status is needed only in case we've got something like this:

  • 000004 is FAILED
  • 000005 is FAILED
  • we try to reapply 000005 and, for some reason, don't care about 000004 for now.

$ tt migrations stop "http://app_user:config_pass@localhost:2379/myapp" \
--tarantool-username=client --tarantool-password=secret

To avoid such situations in the future, restrict the maximum migration execution time
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, any unfinished migration is bad, since APPLY_STARTED is a cluster inconsistency. Moreover, timeouted migration may be even a bit worse, since iproto requests are not cancelled when the client gets a timeout. It is highly likely that you'll need to call tt migrations stop after a timeout failure to do something.

p7nov and others added 4 commits October 1, 2024 12:35
@p7nov p7nov requested a review from xuniq October 1, 2024 07:36
Copy link
Contributor

@xuniq xuniq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, left some comments

Comment on lines 31 to 35
local users = box.space.users
local fmt = users:format()

table.insert(fmt, { name = 'age', type = 'number', is_nullable = true })
users:format(fmt)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit

  1. The example doesn't show the initial space format before the insertion. It may be confusing for the reader: some field is added to the end of a space, but you don't see the actual space it is added to.

  2. In other migration docs, you use the writers space as an example. I suggest using it here as well (for consistency).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Not sure the initial format is important for this example. We're showing how easy it is to add a field (just 4 lines). With the initial format, the example would look much heavier without additional meaning.
  2. Will use writers, thanks.

.. _centralized_migrations_tt_troubleshoot:

Troubleshooting migrations
--------------------------
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Level 1 heading: "==========="

.. _centralized_migrations_tt_troubleshoot_publish:

Incorrect migration published
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Level 2 heading: "----------"

doc/platform/ddl_dml/migrations/index.rst Outdated Show resolved Hide resolved
doc/tooling/tt_cli/migrations.rst Outdated Show resolved Hide resolved
p7nov and others added 4 commits October 3, 2024 16:29
Co-authored-by: Kseniia Antonova <73473519+xuniq@users.noreply.github.com>
@p7nov p7nov merged commit 344ba58 into latest Oct 3, 2024
1 check passed
@p7nov p7nov deleted the gh-ee269-tt-migrations branch October 3, 2024 14:56
p7nov added a commit that referenced this pull request Oct 8, 2024
Resolves tarantool/enterprise_doc#269, tarantool/enterprise_doc#277, tarantool/enterprise_doc#278, tarantool/enterprise_doc#279

Co-authored-by: Georgy Moiseev <moiseev.georgii@gmail.com>
Co-authored-by: Kseniia Antonova <73473519+xuniq@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants