NEO: make MariaDB database creation more robust

software/neoppod/instance-common.cfg.in

@@ -42,5 +42,6 @@ extra-context =
     key admin_cfg neo-admin:rendered
 {%- if mariadb_location is defined %}
     raw mariadb_location {{ mariadb_location }}
+    raw template_mysqld_wrapper {{ template_mysqld_wrapper }}
     raw template_neo_my_cnf {{ template_neo_my_cnf }}
 {%- endif %}

software/neoppod/instance-neo.cfg.in

@@ -7,18 +7,17 @@
 {% set mysql = storage_type == 'MySQL' -%}
 {% if mysql -%}
 
-[mysqld]
-recipe = slapos.cookbook:generic.mysql.wrap_mysqld
-output = ${directory:etc_run}/mariadb
-binary = ${:mysql-base-directory}/bin/mysqld
-configuration-file = ${my-cnf:rendered}
-data-directory = ${directory:srv_mariadb}
-mysql-install-binary = ${:mysql-base-directory}/scripts/mysql_install_db
-mysql-base-directory = {{ mariadb_location }}
+[{{ section('mysqld') }}]
+recipe = slapos.recipe.template:jinja2
+template = {{ template_mysqld_wrapper }}
+rendered = ${directory:etc_run}/mariadb
+context =
+  key defaults_file my-cnf:rendered
+  key datadir my-cnf-parameters:data-directory
 
 [my-cnf-parameters]
 socket = ${directory:var_run}/mariadb.sock
-data-directory = ${mysqld:data-directory}
+data-directory = ${directory:srv}/mariadb
 tmp-directory = ${directory:tmp}
 pid-file = ${directory:var_run}/mariadb.pid
 error-log = ${directory:log}/mariadb_error.log
@@ -34,9 +33,9 @@ rendered = ${directory:etc}/mariadb.cnf
 template = {{ template_neo_my_cnf }}
 context = section parameter_dict my-cnf-parameters
 
-[{{ section('binary-wrap-mysql') }}]
+[binary-wrap-mysql]
 recipe = slapos.cookbook:wrapper
-command-line = ${mysqld:mysql-base-directory}/bin/${:command} --defaults-file=${my-cnf:rendered}
+command-line = '{{ mariadb_location }}/bin/${:command}' --defaults-file="${my-cnf:rendered}"
 wrapper-path = ${directory:bin}/${:command}
 command = mysql
 
@@ -107,7 +106,7 @@ database-parameters = root@neo{{ i }}${my-cnf-parameters:socket}
 database-parameters = ${directory:db-{{i}}}/db.sqlite
 
 [directory]
-db-{{i}} = ${buildout:directory}/srv/{{ storage_id }}
+db-{{i}} = ${:srv}/{{ storage_id }}
 {%- endif %}
 
 [{{ section('logrotate-storage-' ~ i) }}]
@@ -128,9 +127,9 @@ etc_run =  ${:etc}/run
 var_run =  ${:var}/run
 log = ${buildout:directory}/var/log
 tmp = ${buildout:directory}/tmp
-{% if mysql -%}
-srv_mariadb = ${buildout:directory}/srv/mariadb
+srv = ${buildout:directory}/srv
 
+{% if mysql -%}
 [init-script]
 recipe = slapos.recipe.template:jinja2
 # XXX: is there a better location ?
@@ -142,7 +141,7 @@ template = inline:
 < = logrotate-entry-base
 name = mariadb
 log = ${my-cnf-parameters:error-log} ${my-cnf-parameters:slow-query-log}
-post = ${mysqld:mysql-base-directory}/bin/mysql --defaults-file="${my-cnf:rendered}" -e "FLUSH LOGS"
+post = ${binary-wrap-mysql:command-line} -e "FLUSH LOGS"
 
 {% if runTestSuite_in is defined -%}
 # bin/runTestSuite to run NEO tests
@@ -155,7 +154,7 @@ context =
     section directory         directory
     section my_cnf_parameters my-cnf-parameters
     raw     bin_directory     {{ bin_directory }}
-    raw     prepend_path      ${mysqld:mysql-base-directory}/bin
+    raw     prepend_path      {{ mariadb_location }}/bin
 {%- endif %}
 
 {%- endif %}

software/neoppod/software-common.cfg

@@ -94,7 +94,7 @@ mode = 644
 recipe = slapos.recipe.template:jinja2
 template = ${:_profile_base_location_}/${:_buildout_section_name_}.cfg.in
 rendered = ${buildout:directory}/${:_buildout_section_name_}.cfg
-md5sum = c0e22816537b56bceef0b4c2b40f6219
+md5sum = 0a3a54fcc7be0bbd63cbd64f006ceebc
 context =
     key bin_directory buildout:bin-directory
     key develop_eggs_directory buildout:develop-eggs-directory
@@ -107,6 +107,7 @@ context =
     ${:adapter-context}
 adapter-context =
     key mariadb_location mariadb:location
+    key template_mysqld_wrapper template-mysqld-wrapper:rendered
     key template_neo_my_cnf template-neo-my-cnf:target
 
 [root-common]
@@ -123,13 +124,34 @@ md5sum = 4faee020eaf7cd495cd6210dfa4eb0c1
 
 [instance-neo]
 <= download-base-neo
-md5sum = 3e394ae616e554e1eacb2b86ee74fc76
+md5sum = 5fc9fcaec3a5387625af34fe686097ae
 
 [template-neo-my-cnf]
 <= download-base-neo
 url = ${:_profile_base_location_}/my.cnf.in
 md5sum = 9f6f8f2b5f4cb0d97d50ffc1d3837e2f
 
+[template-mysqld-wrapper]
+recipe = slapos.recipe.template:jinja2
+rendered = ${buildout:parts-directory}/${:_buildout_section_name_}/mysqld.in
+mode = 644
+template =
+  inline:{% raw %}#!/bin/sh -e
+  datadir='{{datadir}}'
+  [ -e "$datadir" ] || {
+    rm -vrf "$datadir.new"
+    '${mariadb:location}/scripts/mysql_install_db' \
+      --defaults-file='{{defaults_file}}' \
+      --skip-name-resolve \
+      --basedir='${mariadb:location}' \
+      --datadir="$datadir.new"
+    mv -v "$datadir.new" "$datadir"
+  }
+  exec '${mariadb:location}/bin/mysqld' \
+    --defaults-file='{{defaults_file}}' \
+    "$@"
+  {% endraw %}
+
 [versions]
 BTrees = 4.5.1
 ZODB = 4.4.5

[Nicolas Wavrant]

@jm : this commit breaks the script used to import a dump in mariadb, and the script used to setup a replication slave :

https://lab.nexedi.com/nexedi/slapos/blob/a23a55dc00190dde9e8d8bf26203d5288cc6e888/stack/erp5/instance-mariadb-resiliency-after-import-script.sh.in#L35

https://lab.nexedi.com/nexedi/slapos/blob/a23a55dc00190dde9e8d8bf26203d5288cc6e888/stack/erp5/instance-mariadb-start-clone-from-backup.sh.in#L66

As they empty datadir instead of deleting it, this script doesn't run (the directory exists), but mariadb crashes at start as it can't find any table.

Would it be if I update the 2 scripts as such ?

- find "$DATA_DIRECTORY" -mindepth 1 -delete
+ if [ -d "$DATA_DIRECTORY" ] ; then rm -rf "$DATA_DIRECTORY"; fi

/cc @vpelletier

[Nicolas Wavrant]

On issue I see with this rm $DATA_DIRECTORY would be for cases where a faster disk is mounted on this directory. Then rm fails with Device or resource busy

(this case is not innocent, I know 2 projects doing so)

[Nicolas Wavrant]

Also, the newly created $DATA_DIRECTORY doesn't have the same set of permissions as the original directory created by buildout

[Julien Muchembled]

Part of this commit is a workaround for the fact that MariaDB does not provide a way to check whether initialization could be completed or not. And with this workaround, initialization is already broken if datadir is a mount point, but such configuration seems to be never used when setting up a new instance (i.e. a mount point is set up only after the DB has exceeded some size/load).

- find "$DATA_DIRECTORY" -mindepth 1 -delete
+ if [ -d "$DATA_DIRECTORY" ] ; then rm -rf "$DATA_DIRECTORY"; fi

No. Among all code dealing with MariaDB data, the current find line is the only part that can't be written differently (hmm... except in below case 2).

1. Ideally, mysql_install_db should end by creating a marker file (which should be deleted explicitly just before any code emptying the datadir), since there does not seem to exist any. The difficult part is to compatibility: how to distinguish a datadir that must be upgraded from one that didn't complete initialization. I'm also not fond of patching upstream code: we should rather request such change upstream but it may not be easy to convince.

2. Another option is to be stricter about mount points, by mounting an ancestor directory of the datadir, and being able to configuring a different parent directory for datadir (the default being ~/srv). Thus, the datadir is never a mount point.

3. But the easier solution is that the resilience does its own initialization. Here, there's no need to initialize in an atomic way.

[Vincent Pelletier]

(i.e. a mount point is set up only after the DB has exceeded some size/load).

This is not always the case, precisely in Nicolas' case of restoring a backup:

• we already know the data will eventually be big, it would be wasteful to start restoring on a drive and then move a massive database to another drive, when we can just restore directly to the final drive.
• the restoration script must ensure it starts from a completely empty database, and to do so uses the given find command.

This said, what the backup restoration script could rather do is to call mysql_install_db right after that find statement, as it knows the datadir is not initialised (despite existing).

If I continue this train of thought, I reach the conclusion that it's wrong for mariadb start script to try to create the database: it should rather be called by buildout during instanciation. I believe it was not done at that point because of the (misguided, IMHO) idea that instanciation should not do "slow" things.

[Julien Muchembled]

You misread « DB ». I'm not considering the size/load of a datadir/mysqld at some precise moment before restoration because it is empty/nonexistent. I'm rather talking about the project, usually the production instance of this project: at the beginning, it's small and with little load and it's only after we start tuning. And of course, moving a datadir to a separate mount point is likely to also apply to the restoration part.

So...

we already know the data will eventually be big, it would be wasteful to start restoring on a drive and then move a massive database to another drive, when we can just restore directly to the final drive.

I didn't say the contrary.

it should rather be called by buildout during instanciation

That does not solve anything. The goal is to have something idempotent: if initialization crashes in the middle, it must recover. Given how MariaDB is complex (we could let mysql_install_db, as we used to do in the past, but it may not work if MariaDB version has changed since the crash), it's easier by making initialization atomic.

[Vincent Pelletier]

You misread « DB ».

I did not. We are talking about datadir initialisation and DB backup restoration.

I'm rather talking about the project, usually the production instance of this project: at the beginning, it's small and with little load and it's only after we start tuning.

I don't see the the relation of such statement with current discussion. We are not in the case you describe, as it does not involve initialising the datadir but merely moving it.

Unlike restoring a backup, which is the current discussion, which requires emptying the existing datadir and initialising it, because backup restoration may happen after the instance has seen some use.

That does not solve anything.

It does solve a few things:

• It removes database initialisation from statrup script. By this removal, it becomes clearer that it must not be expected from the startup script to be able to succesfuly initialise the datadir (because it fails at doing so in our case).
• It makes it clearer that the applicability scope of this datadir initialisation strategy only applies to the code which also creates parent folder.

With these, the solution (to our current issue) of initialising the datadir explicitly within the backup restoration script becomes obvious, and we would not need to have this discussion. And in such case, the "create elsewhere and move" strategy is not needed, the restoration script anyway empties the directory before every attempt, and exits if DB initialisation fails.

Do you see now the advantages of this approach ?

[Julien Muchembled]

the solution (to our current issue) of initialising the datadir explicitly within the backup restoration script

Note that this is exactly what I described as solution 3 above. So, I'm fine with this.

But the startup script has to remain as it is.