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

"allow_inconsistent_metadata" env var missing from cli-migrations image #8095

Open
jflambert opened this issue Jan 28, 2022 · 4 comments
Open
Labels
c/cli Related to CLI k/bug Something isn't working t/product-platform

Comments

@jflambert
Copy link
Contributor

Version Information

Server Version: 2.1.1

Environment

Linux

What is the expected behaviour?

2.0.0 added the allow_inconsistent_metadata argument to the replace_metadata API call. Unless documentation is wrong, it doesn't seem to be available as an environment variable for the cli-migrations image.

It would also be nice to have a checkbox in the frontend when importing metadata.json there, but I don't personally need that.

What is the current behaviour?

allow_inconsistent_metadata only seems available through API calls.

@jflambert jflambert added the k/bug Something isn't working label Jan 28, 2022
@rikinsk rikinsk added the c/cli Related to CLI label Jan 28, 2022
@scriptonist
Copy link
Contributor

scriptonist commented Aug 16, 2022

@jflambert can you elaborate a bit on the behaviour that you expect here?

In the CLI migrations image at a high level, the following are the steps which are executed:

$ hasura metadata apply
$ hasura migrate apply --all-databases
$ hasura metadata reload

if after the metadata reload the metadata is inconsistent will you expect the applied migrations to be rolled back and the metadata to be restored to the "old" state? ie whatever it was before this process started.

cc @rikinsk

@jflambert
Copy link
Contributor Author

jflambert commented Aug 16, 2022

I expect:

  • fully applied migrations
  • broken metadata

to be honest I never want rollbacks to "old" state. I never write SQL downgrades.

@scriptonist
Copy link
Contributor

@jflambert Thanks. Can you clarify what you mean by "broken metadata"?

If we call the new metadata that we are applying "new" and the current metadata on the server "old". Then if when trying to apply "new" metadata results in an inconsistency you expect the metadata on the server to be unchanged ie the server will have "old" metadata. is that so?

@chardo
Copy link

chardo commented Nov 14, 2024

Hello, I know this is a very old thread, but I just wanted to chime in to say that exposing the CLI's --disallow-inconsistent-metadata flag via the cli-migrations image would be really helpful. Without that flag, rolling updates to graphql-engine deployments will apply metadata to all running instances, even the old ones (because they listen for changes to metadata), even if that metadata is inconsistent.

This setup breaks the standard mental model of rolling updates (in which a functioning deployment's replicas won't be torn down until the new deployment has available replicas) because the inconsistent metadata "leaks" into the old replicas. My team ran into this issue earlier this week, and while obviously we can do our best to avoid these situations in the future, a production deployment of Hasura relies on a lot of coordination that any team of human beings is bound to get wrong every once in a while. Ensuring that inconsistent metadata will fail a new deployment's rollout without affecting an existing deployment would go a long way towards making me feel safer about running Hasura in prod.

@scriptonist curious to get your thoughts about exposing a flag or configuration env variable in the cli-migrations docker-entrypoint.sh script. Would it make more sense for me to open a new issue or even a pull request for this myself?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c/cli Related to CLI k/bug Something isn't working t/product-platform
Projects
None yet
Development

No branches or pull requests

5 participants