-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
feat(kv): support for migrations #17145
Conversation
a529539
to
f6a1d7d
Compare
|
||
// ErrMigrationSpecNotFound is returned when a migration specification is missing | ||
// for an already applied migration. | ||
ErrMigrationSpecNotFound = errors.New("migration specification not found") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
be good to make this an inflxudb.Error
type with the necessary code/msg
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This kind of error shouldn't really ever reach a user
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It happens on initialize or through manual invocation via a cli.
3264284
to
95d334f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor questions. Take them or leave them.
chore(kv): fixup comments in migrator types fix(kv): initialize migrator bucket on kv service initialize chore(kv): remove currently unused auth index chore(kv): remove currently unused urm index
671e0e3
to
6dc9343
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@GeorgeMac this is huge.
I really like it, thanks for doing this.
The framework is pretty elastic and enables all sorts of migrations given that you have access to the underlying kv in Up()
.
In this scenario migrations would need to be applied via CD, right?
Up()
and Down()
are in the codebase, and not part of the Migration
object itself.
So, to enable a migration, you'll need to code it and deploy it, or am I missing something?
We could take this offline and think about how we could expose this as an HTTP service, what do you think? This way one could run migrations independently from code base changes.
a0c7c25
to
30b565e
Compare
This change introduces a new
kv.Migrator
and associated migrations types.This change sets a new precedent for how we introduce new resource types and indexes into the
kv
package within InfluxDB.The new
Migrator
type adds support for managing these ongoing types of change as migrations. A migration is effectively a change to kv schema (defined as anUp(...)
function operation) and a definition of how to revert the operation (defined as aDown(...)
function operation).The kinds of "schema" changes expected to be managed by these types (in the near-team) are KV buckets and indexes.
By default,
inmem
andbolt
kv implementations should continue to operate as they always have. On initialization of the KV service, when backed by one of these KV stores implementations the migrations are all applied automatically.Outstanding:
testing
package and invoke for bothinmem
andbolt
.Checklist: