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

release-22.1: docker: fix docker image to suit storage in memory #80355

Merged
merged 2 commits into from
Apr 22, 2022

Conversation

blathers-crl[bot]
Copy link

@blathers-crl blathers-crl bot commented Apr 22, 2022

Backport 2/2 commits from #80036 on behalf of @ZhouXing19.

/cc @cockroachdb/release


Currently the datasets created in the init process are stored on disk,
and they are inaccessible if the “real” server is started with storage in memory.

This is because, for the container initialization, the current logic is:

  1. start_init_server: start the single-node server in the background, with
    default on-disk storage setting;
  2. run_init_queries: create default db and user, and run the init scripts
    saved in
    docker-entrypoint-initdb.d;
  3. stop the init server and restart the server with given parameters in foreground.

In this commit we start and keep running the init server with the given
parameters, and eliminate the “stop and restart” step. Also, bring the server
process from background to foreground with fg once all init queries are finished.

Fixes #80005.

Release note (docker): refactor the initialization process of the docker image
to accomodate initialization scripts with memory storage.


Release justification: (Bug fixes and low-risk updates to new functionality) fix the docker image for the memory storage use case

Currently the datasets created in the init process are stored on disk,
and they are inaccessible if the “real” server is started with storage in memory.

This is because for the container initialization, the current logic is:
1. start_init_server: start the single-node server in the background, with
default on-disk storage setting;
2. run_init_queries: create default db and user, and run the init scripts
saved in
docker-entrypoint-initdb.d;
3. *stop the init server and restart the server* with given parameters in foreground.

In this commit we start and keep running the init server with the given
parameters, and eliminate the “stop and restart” step. Also, bring the server
process from background to foreground with fg once all init queries are finished.

Fixes #80005.

Release note (docker): refactor the initialization process of the docker image
to accomodate the use case with memory storage.
@blathers-crl blathers-crl bot requested a review from a team as a code owner April 22, 2022 05:42
@blathers-crl blathers-crl bot force-pushed the blathers/backport-release-22.1-80036 branch from ae761e4 to 7b2bce8 Compare April 22, 2022 05:42
@blathers-crl blathers-crl bot requested review from otan, rafiss and ZhouXing19 April 22, 2022 05:42
@blathers-crl blathers-crl bot added blathers-backport This is a backport that Blathers created automatically. O-robot Originated from a bot. labels Apr 22, 2022
@blathers-crl
Copy link
Author

blathers-crl bot commented Apr 22, 2022

Thanks for opening a backport.

Please check the backport criteria before merging:

  • Patches should only be created for serious issues or test-only changes.
  • Patches should not break backwards-compatibility.
  • Patches should change as little code as possible.
  • Patches should not change on-disk formats or node communication protocols.
  • Patches should not add new functionality.
  • Patches must not add, edit, or otherwise modify cluster versions; or add version gates.
If some of the basic criteria cannot be satisfied, ensure that the exceptional criteria are satisfied within.
  • There is a high priority need for the functionality that cannot wait until the next release and is difficult to address in another way.
  • The new functionality is additive-only and only runs for clusters which have specifically “opted in” to it (e.g. by a cluster setting).
  • New code is protected by a conditional check that is trivial to verify and ensures that it only runs for opt-in clusters.
  • The PM and TL on the team that owns the changed code have signed off that the change obeys the above rules.

Add a brief release justification to the body of your PR to justify this backport.

Some other things to consider:

  • What did we do to ensure that a user that doesn’t know & care about this backport, has no idea that it happened?
  • Will this work in a cluster of mixed patch versions? Did we test that?
  • If a user upgrades a patch version, uses this feature, and then downgrades, what happens?

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@janpio
Copy link

janpio commented Apr 22, 2022

(Subscribing so I get a notification when this goes out - hopefully)

@otan
Copy link
Contributor

otan commented Apr 22, 2022

@ZhouXing19 @rafiss should we merge this to v22.1.0?

@rafiss
Copy link
Collaborator

rafiss commented Apr 22, 2022

I'm in favor of adding it to 22.1.0. let me see what's needed

@ZhouXing19 ZhouXing19 merged commit 281c949 into release-22.1 Apr 22, 2022
@ZhouXing19 ZhouXing19 deleted the blathers/backport-release-22.1-80036 branch April 22, 2022 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blathers-backport This is a backport that Blathers created automatically. O-robot Originated from a bot.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants