Initialize DB during image build #457

danielhollas · 2024-05-21T02:26:12Z

Speed up container startup by initialized Posgresql DB during image build.

Turns out we were already doing that in the base-with-services image, but this initialized DB had a wrong name, inconsistent with 20_start-postgresql.sh. To prevent this mismatch in the future, we now set PGDATA envvar in the image, which is automatically picked up by initdb and pg_ctl commands. See e.g. https://www.postgresql.org/docs/current/app-initdb.html#APP-INITDB-OPTION-PGDATA

This PR is build on top of #456. Taking these changes all together, the full-stack image now starts up in 9s on my machine.

This reverts commit 79231b6.

stack/full-stack/Dockerfile

stack/base-with-services/before-notebook.d/20_start-postgresql.sh

danielhollas · 2024-05-26T20:21:05Z

docker-bake.hcl

@@ -54,22 +54,8 @@ group "default" {
  targets = "${TARGETS}"
 }

-target "base-meta" {


Unrelated cleanup

unkcpz · 2024-06-04T12:48:17Z

Are you sure this is the correct way to go? The purpose of initialize DB during the runtime phase is to having the DB in home and therefore in the persistent volume.

danielhollas · 2024-06-04T13:14:41Z

I am not sure what you mean @unkcpz, the DB here is initialized in the home directory. So this should work when using aiidalab-launch and normal Docker. In Kubernetes, you'll have the same issue as with everything else in the home directory.

unkcpz · 2024-06-04T13:19:43Z

In Kubernetes, you'll have the same issue as with everything else in the home directory.

For everything (not everything but something) else, we do "home prepare" with https://github.com/aiidalab/aiidalab-docker-stack/blob/main/stack/base/before-notebook.d/10_prepare-home-config.sh. This not happened to the DB folder.
Either we adapt the same strategy as we are planing for the qeapp image to have a home some where else and load it back after. Otherwise, with your changes here, the full-stack will fail on k8s deployment.

danielhollas · 2024-06-04T13:23:58Z

For everything (not everything but something) else, we do "home prepare" with https://github.com/aiidalab/aiidalab-docker-stack/blob/main/stack/base/before-notebook.d/10_prepare-home-config.sh. This not happened to the DB folder.

Yes, but that's because of k8s right?

Either we adapt the same strategy as we are planing for the qeapp image to have a home some where else and load it back after. Otherwise, with your changes here, the full-stack will fail on k8s deployment.

I don't think so, I didn't actually changed the logic in stack/base-with-services/before-notebook.d/20_start-postgresql.sh. If the DB folder doesn't exist, it will get created at startup, as was done till now, so k8s should work I believe.

unkcpz · 2024-06-04T13:34:52Z

I didn't actually changed the logic in stack/base-with-services/before-notebook.d/20_start-postgresql.sh

I see, I didn't take close look at the PR, sorry. Then all make sense.

unkcpz

Thanks @danielhollas! Sorry for the late review.

The changes are all good, just some minor questions.

Since we are in the place, it is worth to discuss, do we really need a environment aiida-core-services? But for sure it will be another PR if we decide to move the conda env into base.

unkcpz · 2024-06-07T11:28:56Z

.github/workflows/build.yml

@@ -74,6 +74,6 @@ jobs:
            - name: Set output variables
              id: bake_metadata
              run: |
-                  .github/workflows/extract-image-names.sh | tee -a "${GITHUB_OUTPUT}"
+                  .github/workflows/extract-image-names.sh | tee -a "${GITHUB_OUTPUT}" | awk -F'=' '{print $2}' | jq


This I don't understand, what different it will make?

Oh, this just pretty prints the json output so that it is easier to read in the Github Actions logs. Previously it was on a single line, and it was a bit annoying when I needed to copy the image name.

This is how it looks now in the amd64 image build

unkcpz · 2024-06-07T11:32:52Z

stack/base-with-services/Dockerfile

@@ -58,4 +65,4 @@ USER ${NB_USER}
 WORKDIR "/home/${NB_USER}"

 # Initialize the database
-RUN mamba run -n aiida-core-services initdb -D aiida_db -U aiida
+RUN mamba run -n aiida-core-services initdb


What is the DB name used when create aiida profile after this change? We set it explicitly in config-quick-setup.yaml as aiida_db.

BTW, we need keep in mind to adapt the change to use verdi presto after we move to 2.6.0

Good question. -D parameter doesn't set the db name, it sets the DB location.

https://www.postgresql.org/docs/current/app-initdb.html#APP-INITDB-OPTION-PGDATA

in this case it would set it to ~/aiida_db. But this was broken anyway, since the location in stack/base-with-services/before-notebook.d/20_start-postgresql.sh was set to ~/.postgresql, so essentially this initdb command was being ignored. Now the location is set via the PGDATA variable and thus is guaranteed to be the same everywhere to prevent these kind of mismatches in the future.

Note that the the -U parameter sets the so-called bootstrap superuser I am not sure if this would have any effect, but we are not using this parameter in stack/base-with-services/before-notebook.d/20_start-postgresql.sh so I deleted it here as well.

BTW, we need keep in mind to adapt the change to use verdi presto after we move to 2.6.0

I don't think we'll want to use verdi presto since we'd loose some config options. Instead I believe we should migrate from verdi quicksetup to the full verdi profile setup. But that's not that urgent, quicksetup has been deprecated but not removed.

Makes sense, thanks!

danielhollas · 2024-06-10T11:51:22Z

Since we are in the place, it is worth to discuss, do we really need a environment aiida-core-services? But for sure it will be another PR if we decide to move the conda env into base.

Why would you want to move it to the base environment? base environment contains literally everything else so I quite like that we can have at least these services in a separate one. If you disagree, could you please open an issue with your reasoning?

unkcpz · 2024-06-11T08:36:32Z

Why would you want to move it to the base environment? base environment contains literally everything else so I quite like that we can have at least these services in a separate one.

The main reason is of this mlocate which is in the aiida-core-services environment and required to run pytest. It just over complex to have an extra conda environment that we gain little from it. I open the issue for the discussion #467

unkcpz

Thanks! All good to me. @danielhollas I'll let you merge and write up for the commit message.

danielhollas added 13 commits May 16, 2024 09:33

Reduce number of verdi invocations on startup

f48a799

Set debug outputs

bdd8ef1

Cleanup 20_start-postgresql.sh

f02bc28

More tweaks

6f639aa

Prepare DB during build

da9cffd

Try without root

79231b6

Use PGDATA

a6320de

Revert "Try without root"

fac06ff

This reverts commit 79231b6.

PGDATA everywhere

b4e6145

back

ac248db

Fix?

2e5ba46

Pass PGDATA for full-stack

50e2611

This?

be5846d

danielhollas requested a review from superstar54 May 23, 2024 12:50

danielhollas commented May 24, 2024

View reviewed changes

stack/full-stack/Dockerfile Outdated Show resolved Hide resolved

Update stack/full-stack/Dockerfile

e89f505

danielhollas changed the title ~~WIP: Initialize DB during image build~~ Initialize DB during image build May 24, 2024

danielhollas commented May 24, 2024

View reviewed changes

stack/full-stack/Dockerfile Outdated Show resolved Hide resolved

Update stack/full-stack/Dockerfile

9aaa20d

danielhollas commented May 24, 2024

View reviewed changes

stack/full-stack/Dockerfile Outdated Show resolved Hide resolved

danielhollas and others added 6 commits May 24, 2024 13:55

Update stack/full-stack/Dockerfile

163e4f2

Add comment about Erlang

e4c45e1

Come on!

1ac56fb

Remove meta targets

bdf3907

Preserve logfile

c8c1cef

sigh

2ddeac0

danielhollas commented May 25, 2024

View reviewed changes

stack/base-with-services/before-notebook.d/20_start-postgresql.sh Outdated Show resolved Hide resolved

danielhollas and others added 3 commits May 25, 2024 12:43

fix

7e9c91c

remove pg_ctl status

2d0e8f4

To hell with it

d03f11b

danielhollas force-pushed the initdb branch from a2a64f3 to d03f11b Compare May 26, 2024 17:18

danielhollas added 8 commits May 26, 2024 18:50

Try thisA

94ab041

?

ae59bdb

Sigh

8378fa5

pretty print json output

236902b

Pretty please

5b2d0bd

hmm

2cfe15a

...

d480087

Getting closer

daa56ea

danielhollas force-pushed the initdb branch from ea780c2 to daa56ea Compare May 26, 2024 19:33

danielhollas marked this pull request as ready for review May 26, 2024 19:34

Screw this

c08dd3d

danielhollas commented May 26, 2024

View reviewed changes

danielhollas requested a review from unkcpz May 26, 2024 20:31

Base automatically changed from speedup-startup to main June 4, 2024 13:24

Merge branch 'main' into initdb

7f80842

unkcpz requested changes Jun 7, 2024

View reviewed changes

danielhollas force-pushed the initdb branch from 0603a5e to 7f80842 Compare June 10, 2024 11:44

danielhollas requested a review from unkcpz June 10, 2024 11:53

unkcpz approved these changes Jun 11, 2024

View reviewed changes

danielhollas merged commit 3208794 into main Jun 11, 2024
27 checks passed

danielhollas deleted the initdb branch June 11, 2024 09:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initialize DB during image build #457

Initialize DB during image build #457

danielhollas commented May 21, 2024 •

edited

Loading

danielhollas May 26, 2024

unkcpz commented Jun 4, 2024

danielhollas commented Jun 4, 2024

unkcpz commented Jun 4, 2024

danielhollas commented Jun 4, 2024

unkcpz commented Jun 4, 2024

unkcpz left a comment

unkcpz Jun 7, 2024

danielhollas Jun 10, 2024

danielhollas Jun 10, 2024

unkcpz Jun 7, 2024

danielhollas Jun 10, 2024

danielhollas Jun 10, 2024

unkcpz Jun 11, 2024

danielhollas commented Jun 10, 2024

unkcpz commented Jun 11, 2024 •

edited

Loading

unkcpz left a comment

Initialize DB during image build #457

Initialize DB during image build #457

Conversation

danielhollas commented May 21, 2024 • edited Loading

Choose a reason for hiding this comment

unkcpz commented Jun 4, 2024

danielhollas commented Jun 4, 2024

unkcpz commented Jun 4, 2024

danielhollas commented Jun 4, 2024

unkcpz commented Jun 4, 2024

unkcpz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danielhollas commented Jun 10, 2024

unkcpz commented Jun 11, 2024 • edited Loading

unkcpz left a comment

Choose a reason for hiding this comment

danielhollas commented May 21, 2024 •

edited

Loading

unkcpz commented Jun 11, 2024 •

edited

Loading