Skip to content

Fixing multiple issues related to onlineddl/lifecycle#8500

Merged
deepthi merged 1 commit intovitessio:mainfrom
planetscale:onlineddl-stale-migration-compeleted-timestamp
Jul 23, 2021
Merged

Fixing multiple issues related to onlineddl/lifecycle#8500
deepthi merged 1 commit intovitessio:mainfrom
planetscale:onlineddl-stale-migration-compeleted-timestamp

Conversation

@shlomi-noach
Copy link
Copy Markdown
Contributor

Description

Fixes #8498
Fixes #8499 (possibly)

  • stale migration analysis did not update completed_timestamp. As result lifecycle would not run. Now that's fixed.
  • retroactively updating completed_timestamp for existing NULL entries
  • liveness_timestamp now always set with started_timestamp, so that it is never NULL even if migration didn't report liveness. This is essential for detecting and marking stale migrations
  • when garbage collecting artifacts, RENAME statement would create collissions between artifacts of same migration. This is now solved by assigning distinct timestamps to artifacts of same migration
  • on the safe side, though there is no evidence this ever happened, whenever an artifact is added, we set cleanup_timestamp to be NULL

Related Issue(s)

Checklist

  • Tests were added or are not required
  • Documentation was added or is not required

- stale migration analysis did not update completed_timestamp. As result lifecycle would not run
- retroactively updating completed_timestamp for existing NULL entries
- liveness_timestamp now always set with started_timestamp, so that it is never NULL even if migration didn't report liveness. This is essential for detecting and marking stale migrations
- when garbage collecting artifacts, RENAME statement would create collissions between artifacts of same migration. This is now solved by assigning distinct timestamps to artifacts of same migration
- on the safe side, though there is no evidence this ever happened, whenever an artifact is added, we set cleanup_timestamp to be NULL

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Copy link
Copy Markdown
Collaborator

@deepthi deepthi left a comment

Choose a reason for hiding this comment

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

The changes look good.
One question I do have is what happens if someone upgrades to this version of vitess and then downgrades to a previous version? The table definitions in _vt schema will be the new ones. Has code been written to work well in the presence of extra columns?

@deepthi
Copy link
Copy Markdown
Collaborator

deepthi commented Jul 21, 2021

Note: Please make another PR to get this into release-11.0 prior to the GA date.

@shlomi-noach
Copy link
Copy Markdown
Contributor Author

Has code been written to work well in the presence of extra columns?

It has. I don't have a endtoend test to prove it, but my ongoing development cycle show it.

@shlomi-noach
Copy link
Copy Markdown
Contributor Author

Note: Please make another PR to get this into release-11.0 prior to the GA date.

done: #8517

@deepthi deepthi merged commit 00089fd into vitessio:main Jul 23, 2021
@deepthi deepthi deleted the onlineddl-stale-migration-compeleted-timestamp branch July 23, 2021 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

3 participants