Implement schema migration and validate storage version during boostrap #13200
Conversation
serathius
changed the title
serathius
force-pushed
the
downgrade-storage
branch
4 times, most recently
from
163b47a
to
77ac6e9
Compare
serathius
force-pushed
the
downgrade-storage
branch
from
77ac6e9
to
cbfab98
Compare
There was a problem hiding this comment.
Couple of comments and suggestions 🎉
I would find it useful to maybe add a bit more context to the description of the PRs in the future, as its harder to follow if I miss one or two. I read the design doc and the issue but still might be useful IMHO, thank you!
serathius
force-pushed
the
downgrade-storage
branch
2 times, most recently
from
631ad45
to
284b99e
Compare
|
PTAL @lilic |
serathius
force-pushed
the
downgrade-storage
branch
from
284b99e
to
4bc6d45
Compare
There was a problem hiding this comment.
Couple more comments. I do believe we need more consensus on the breaking change here.
cc @hexfusion
serathius
force-pushed
the
downgrade-storage
branch
6 times, most recently
from
e44ec67
to
16e916d
Compare
|
Addressed all issues, PTAL |
serathius
force-pushed
the
downgrade-storage
branch
2 times, most recently
from
6e02060
to
af58a2d
Compare
|
Send an announcement about the breaking change to make sure community is aware of it. https://groups.google.com/g/etcd-dev/c/-u_vl_G-nRg |
serathius
force-pushed
the
downgrade-storage
branch
4 times, most recently
from
898223e
to
3ad6f79
Compare
server/storage/schema/actions.go
Outdated
| return revert, nil | ||
| } | ||
|
|
||
| func currentStateFieldAction(tx backend.BatchTx, bucket backend.Bucket, fieldName []byte) action { |
There was a problem hiding this comment.
(make it clear that it produces a rollback action)
There was a problem hiding this comment.
Do we need to encapsulate this function? Maybe for better readable just writing the logic in unsafeDo functions?
There was a problem hiding this comment.
I think it's clearer to have in separate function, just the original name was bad. I changed the name to restoreFieldValueAction, please let me know what you think.
server/storage/schema/actions.go
Outdated
| return revert, nil | ||
| } | ||
|
|
||
| func currentStateFieldAction(tx backend.BatchTx, bucket backend.Bucket, fieldName []byte) action { |
There was a problem hiding this comment.
Do we need to encapsulate this function? Maybe for better readable just writing the logic in unsafeDo functions?
serathius
force-pushed
the
downgrade-storage
branch
2 times, most recently
from
e455d25
to
bc6a0ac
Compare
server/storage/schema/actions.go
Outdated
| return nil | ||
| } | ||
|
|
||
| // unsafeExecuteInReverseOrder executes actions in revered order. Will panic on |
serathius
force-pushed
the
downgrade-storage
branch
6 times, most recently
from
dd72e7f
to
bc6a0ac
Compare
|
Latest changes have broken integrations tests in a way that I don't understand nor can reproduce locally. Reverting to state from yesterday. Will try to reapply latest changes but it will take multiple iterations. |
serathius
force-pushed
the
downgrade-storage
branch
4 times, most recently
from
40a6323
to
aed03ad
Compare
…change and panic if unknown storage version is found Storage version should follow cluster version. During upgrades this should be immidiate as storage version can be always upgraded as storage is backward compatible. During downgrades it will be delayed and will require time for incompatible changes to be snapshotted. As storage version change can happen long after cluster is running, we need to add a step during bootstrap to validate if loaded data can be understood by migrator.
Co-authored-by: Lili Cosic <cosiclili@gmail.com>
serathius
force-pushed
the
downgrade-storage
branch
from
aed03ad
to
79f6faa
Compare
serathius
changed the title
Part of milestone 1 of #13168
cc @ptabor