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

Add logic for upgrading datadir #34

Merged
merged 8 commits into from
May 4, 2018
Merged

Add logic for upgrading datadir #34

merged 8 commits into from
May 4, 2018

Conversation

hhorak
Copy link
Member

@hhorak hhorak commented Apr 25, 2017

MySQL and MariaDB use versions that consist of three numbers X.Y.Z (e.g. 5.6.23).
For version changes in Z part, the server's binary data format stays compatible and thus no
special upgrade procedure is needed. For upgrades from X.Y to X.Y+1, consider doing manual
steps as described at
https://mariadb.com/kb/en/library/upgrading-from-mariadb-101-to-mariadb-102/

Skipping versions like from X.Y to X.Y+2 or downgrading to lower version is not supported;
the only exception is ugrading from MariaDB 5.5 to MariaDB 10.0.

Important: Upgrading to a new version is always risky and users are expected to make a full
back-up of all data before.

A safer solution to upgrade is to dump all data using mysqldump or mysqldbexport and then
load the data using mysql or mysqldbimport into an empty (freshly initialized) database.

Another way of proceeding with the upgrade is starting the new version of the mysqld daemon
and run mysql_upgrade right after the start. This so called in-place upgrade is generally
faster for large data directory, but only possible if upgrading from the very previous version,
so skipping versions is not supported.

This container detects whether the data needs to be upgraded using mysql_upgrade and
we can control it by setting MYSQL_UPGRADE variable, which can have one or more of the following values:

  • warn -- If the data version can be determined and the data come from a different version
    of the daemon, a warning is printed but the container starts. This is the default value.
    Since historically the version file mysql_upgrade_info was not created, when using this option,
    the version file is created if not exist, but no mysql_upgrade will be called.
    However, this automatic creation will be removed after few months, since the version should be
    created on most deployments at that point.
  • auto -- mysql_upgrade is run at the beginning of the container start, if the data version
    can be determined and the data come with the very previous version. A warning is printed if the data
    come from even older or newer version. This value effectively enables automatic upgrades,
    but it is always risky and users should still back-up all the data before starting the newer container.
    Set this option only if you have very good back-ups at any moment and you are fine to fail-over
    from the back-up.
  • force -- mysql_upgrade is run right after the daemon has started, no matter what version the data
    come from. This is also the way to create the missing version file mysql_upgrade_info if not present
    in the root of the data directory; this file holds information about the version of the data.
  • optimize -- runs mysqlcheck --optimize before starting the mysqld daemon, no matter what version
    of the data is detected. It optimizes all the tables.
  • analyze -- runs mysqlcheck --analyze before starting the mysqld daemon, no matter what version
    of the data is detected. It analyzes all the tables.
  • disable -- nothing is done regarding data directory version.

Multiple values are separated by comma and run in-order, e.g. MYSQL_UPGRADE="optimize,analyze".

@hhorak
Copy link
Member Author

hhorak commented Apr 25, 2017

@bparees @praiskup @pkubatrh FYI
This is a first draft to make the container a bit more bullet-proof. On the other hand, the 'auto' mode might be a bit risky, since it's something we don't do automatically in RPM cases (in comparison to Ubuntu, who do run mysql_upgrade automatically), so any feedback welcome..

if [ -f ${CONTAINER_SCRIPTS_PATH}/check-upgrade.sh ]; then
log_info 'Sourcing check-upgrade.sh ...'
source ${CONTAINER_SCRIPTS_PATH}/check-upgrade.sh
fi

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like we should "upgrade" first and then do "post-init.sh".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. The user could run some script as part of post-init that could fail when the data is not upgraded.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, makes sense, I'll fix this.

@@ -92,6 +94,7 @@ function initialize_database() {
# Using empty --basedir to work-around https://bugzilla.redhat.com/show_bug.cgi?id=1406391
mysql_install_db --rpm --datadir=$MYSQL_DATADIR --basedir=''
start_local_mysql "$@"
mysql_upgrade --socket=/tmp/mysql.sock

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I'm not familiar with mysql/mariadb "upgrading" procedures, I'm curious why this method changed... Can we have small comment above this command?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's something I found during investigation, that mysql_upgrade also marks data properly with a version, so we can either add it manually or let mysql_upgrade do it, while it also checks the data dir, which in empty (freshly initialize) case is very quick and might find potential issues (which is not expected, but one never knows).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, but comment would be nice.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

definitely, will add

# Converts string version to the integer format (5.5 is converted to 505,
# 10.1 is converted into 1001, etc.
function version2number() {
local version=0

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I we have bash (we have I believe), this would be simpler:

printf %d%02d ${1%%.*} ${1##*.}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, we likely do more than what is in function description ... :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. mariadb uses version in format e.g. 10.0.12-MariaDB, so we need to remove the -MariaDB or any similar suffix as well. It might still be done better in bash though, so will try to fix..

echo "Error: version '$version_text' cannot be converted to the number."
echo "0"
fi
echo "${version}"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is personal preference, but I can't resist -> I really like this pattern:

versionNumber() {
# Do something...
# and then set global variable $<function_name>_result (versionNumber_result)
printf -v versionNumber_result %d%02d ${1%%.*} ${1##*.}
}

By this pattern, you avoid additional shell forks and error prone statements like:

variable=`method with params`

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, make sense to me


# Prints version of the mysqld that is currently available (string)
function mysqld_version() {
${MYSQL_PREFIX}/libexec/mysqld -V | awk '{print $3}'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Fedora I get: 10.1.21-MariaDB

local version=$(mysqld_version)
local upgrade_info_file=$(get_mysql_upgrade_info_file "$datadir")
echo "${version}" > "${upgrade_info_file}"
log_info "Storing version '${version}' information into the data dir '${upgrade_info_file}'"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please reverse the two commands above, or do s/Storing/Stored/.

Copy link

@praiskup praiskup Apr 26, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, this function seems to be unused... and perhaps should be in initialization part.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's replaced by mysql_upgrade as described above, so I'll probably remove this function at all..

local datadir_version=$(get_datadir_version "$datadir")
local mysqld_version=$(mysqld_compat_version)

case "${MYSQL_UPGRADE}" in

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double quotes are not needed here.

Upgrading and datadir checking
------------------------------
MySQL and MariaDB use versions that consist of three numbers (e.g. 5.6.23). When two
versions are different only in the last part, they are compatible, but when upgrading
Copy link

@praiskup praiskup Apr 26, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I've read this like dozen of times, ... :) and now I get it right... I understood that statement like 'when two numbers are different only in the last part', so I would suggest:
" ... that consists of three numbers X.Y.Z (e.g. 5.6.23). For version changes in Z part, the server's binary data format stays compatible and thus no special upgrade procedure is needed. For upgrades from X.Y to X.Y+1, we need to consider upgrading ... "

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would document what happens when X is bumped, ...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice wording, I admit that the initial one was not understandable for me either.

versions are different only in the last part, they are compatible, but when upgrading
from e.g. 5.6 to 5.7 or from 10.0 to 10.1, we need to consider upgrading as described
in https://dev.mysql.com/doc/refman/5.7/en/upgrading-from-previous-series.html.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/consider upgrading as described/consider doing manual steps as described/ ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, thx

we can control it by setting `MYSQL_UPGRADE` variable, which can have the following values:

* `warn` -- If the data version can be determined and the data come from a different version
of the daemon, a warning is printed but the container starts. This is the default value.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it really safe to start mysqld against incompatible data version by default?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I definitely would not do this by default since the warning will be missed (unless something breaks).
I have looked into how the image updates are managed in Openshift (via imagestreams) and it seems like as long as the user is using mariadb:latest in the imagestream and a new mariadb image (that can be of a different version) gets tagged by :latest, Openshift will just take down the old deployment and deploy the new version automatically.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are users who know what they are doing and in that case starting a container and running mysql_upgrade right after it, even manually, is totally valid use case. But I see the point in that it maybe should not be the default.. On the other hand, RPMs currently do the same by default -- the newer daemon starts, but an error/warning is printed into the log and we don't have any reports for this. Anyway, the situation with the image stream makes it a bit complicated and less visible, so we might pick a different strategy here. I'm still not decided though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion running mysql_upgrade by default might actually be a bit better (=less evil) in this case, since at least this way the fail would occur at the moment of upgrade and not far off in the future as with the ignored warning (and probably be less/not-at-all destructive?).
Best case would be if there were no :latest tag in the imagestream for DB images but I am not sure if that is possible or even wanted by Openshift.

Set this option only if you have very good back-ups at any moment and you are fine to fail-over
from the back-up.
* `force` -- `mysql_upgrade` is run right after the daemon has started, no matter what version the data
come from. This can be also used to creating the `mysql_upgrade_info` file if not present in the root

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/used to creating/used to create/

But I wouldn't probably mention that this is "the way to create the missing version file" .. Perhaps this deserves another compat option ...?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see your point, but on the other hand less options means simpler image.. so I'm not sure whether it is worth it..

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still thinking about this.. mysql_upgrade's man page says this: "...also saves the MariaDB version number in a file named mysql_upgrade_info in the data directory. This is used to quickly check whether all tables have been checked for this release so that table-checking can be skipped..."

So the file presence means the data were checked for a particular version, it's not just information about a version.. and thus it makes sense to create it only after mysql_upgrade run. So maybe we should not include the recommendation of creating the file manually and rather advise to use --force for creating this file..

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does that mean that if the file is not possible to be created (some table is not checked against particular mariadb version), the mysql_upgrade is guaranteed to fail? It would be perfect.

come from. This can be also used to creating the `mysql_upgrade_info` file if not present in the root
of the data directory; this file holds information about the version of the data.
* `optimize` -- runs `mysqlcheck --optimize` before starting the mysqld daemon, no matter what version
of the data is detected. It optimizes all the tables.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice if this was not exclusive option, so users could use:
MYSQL_UPGRADE=optimize,force,analyze
(if that makes sense)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not that familiar with mariadb/mysql but the analyze and optimize options seem like something that would be useful even outside data upgrade.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, that makes sense to me as well.


force)
log_info 'Running mysql_upgrade --force ...'
mysql_upgrade --socket=/tmp/mysql.sock --force

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we document this is safe ("no-op") if the version isn't changed? OTOH, I'm afraid of situations where users will use MYSQL_UPGRADE=force temporarily (because of missing version file), and they forgot to remove this variable from configuration .... :/ then simple changing of image to "too new" version might end up with disaster...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand it correctly all the --force option does is ignore the existence of a version file. I expect mysql_upgrade should still fail if it encounters some incompatibilities with the existing version.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But in case of "auto|warn", we do manual version checking (instead of delegating the checks to mysql_upgrade). At least we don't always run the mysql_upgrade -> so it seemed to me that it is somehow unsafe action.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should take a closer look into the code, but quick look at the behavior and into the man page it looks like it is effectively no-op.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've checked the code and it really is a no-op.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Taking back, if we use '--force', than it is not a no-op.. it will call mysqlcheck which runs optimize/check/repair tables..

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But my expectation is that even running it more times should be safe..

mysql_upgrade --socket=/tmp/mysql.sock
fi
else
log_info "MySQL server is version '${mysqld_version}' and datadir is version '${datadir_version}', which are incompatible."

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why isn't failure expected here?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, maybe we should info user about the possibilities (what is the name of the container that supports the upgrade from this datadir version, and that multiple upgrade steps might be needed e.g.).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds fine to me, thanks for the suggestions.

@praiskup
Copy link

praiskup commented Apr 26, 2017

Thanks for the progress! I have some notes:

  • what's the storage implication of "in place" upgrade, do we need some additional data or is that really "in situ"?
  • could we (perhaps, if storage allows us) do the "in container" backup and restore it in case of failure? Or would that be over-engineering?
  • what is the expected time constraints (the downtime caused by "in-place upgrade")
  • do we have a way to detect database is "up&running"? ... so if the upgrade takes some time, the application container can wait ... I mean, with this approach the app deployment plan isn't changed and I'm curious whether it hasn't some side effects

While I really dislike the N -> N+1 -> N+2 upgrade approach (if some major version needs to be skipped -> which is the most likely scenario btw), it looks to me that in case of mariadb this "in-place" procedure has no real obvious drawbacks to not have it implemented.

This was referenced Apr 26, 2017
@hhorak
Copy link
Member Author

hhorak commented Apr 26, 2017

what's the storage implication of "in place" upgrade, do we need some additional data or is that really "in situ"?

If you mean space-wise, whether we need more space, then we should not need much more, to my knowledge; the upgrade is basically about changing some internal structures mostly, analyzing tables, etc.

@hhorak
Copy link
Member Author

hhorak commented Apr 26, 2017

could we (perhaps, if storage allows us) do the "in container" backup and restore it in case of failure? Or would that be over-engineering?

That would bring new requirements on space in the volume and users would then either have another back-up somewhere else or would need to figure out how the back-up can be used. I'd say that conscious user backs-up data regularly and especially before upgrade, so going back and using back-up in case of troubles is something what users should be counted with always. I don't think we need to do this job for them.

@hhorak
Copy link
Member Author

hhorak commented Apr 26, 2017

what is the expected time constraints (the downtime caused by "in-place upgrade")

Depending on the size, I expect up to minutes in case of reasonably big tables, but I don't have any specific data. My guess is that the data are primarily read and only some quick operation is proceeded on them, so maybe thinking about what is the speed of sequential read might give an idea..

@hhorak
Copy link
Member Author

hhorak commented Apr 26, 2017

application container can wait ... I mean, with this approach the app deployment plan isn't changed and I'm curious whether it hasn't some side effects

The upgrade should IMO happen at the time the daemon is not listening outside, which means the case when MYSQL_UPGRADE is set to 'auto'. Then the application should be smart enough to wait for the database to come up. However, even if the database upgrade is run on live DB (listening outside), the data are locked for reading, so application will be able to know..

* `analyze` -- runs `mysqlcheck --analyze` before starting the mysqld daemon, no matter what version
of the data is detected. It analyzes all the tables.

More values are separated by comma, e.g. `MYSQL_UPGRADE="optimize,analyze"`.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the order mattes now, it might or might not be an issue ... if it is not, then at least a small hint should be mentioned here.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/More/Multiple/

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah explaining that the actions will be run in order is good... most people probably want to upgrade before optimizing, i would think.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, fixing..

10.1/test/run Outdated
echo " FAIL"
echo " Giving up: Failed to connect."
echo -n "Trying to ping the IP ${ip}: "
ping -c 1 ${ip} &>/dev/null && echo "OK" || echo "FAIL"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Meh, how it comes we have iputils installed? :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there systems without ping? :) Well, to be honest, what I really wanted to do is checking whether the container is running, so we should probably do it using docker inspect...

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Status: Downloaded newer image for registry.access.redhat.com/rhscl/mariadb-101-rhel7:latest
bash-4.2$ ping 
bash: ping: command not found

from the back-up.
* `force` -- `mysql_upgrade` is run right after the daemon has started, no matter what version the data
come from. This is also the way to create the missing version file `mysql_upgrade_info` if not present
in the root of the data directory; this file holds information about the version of the data.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this file not always being created?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should, but it is not. I'd call this a bug, that this PR should fix. At least I believe that all instances that will initiate the datadir after this PR is fixed, they will have the file...

* `analyze` -- runs `mysqlcheck --analyze` before starting the mysqld daemon, no matter what version
of the data is detected. It analyzes all the tables.

More values are separated by comma, e.g. `MYSQL_UPGRADE="optimize,analyze"`.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/More/Multiple/

fi

if [ $(( ${datadir_version} + 1 )) -eq "${mysqld_version}" -o "${datadir_version}" -eq 505 -a "${mysqld_version}" -eq 1000 ] ; then
log_info "MySQL server is version '${mysqld_version}' and datadir is version '${datadir_version}'."
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this doesn't strike me as being obviously a warning. it seems pretty easy to overlook/ignore.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I'll try something less overlook-able..

* `analyze` -- runs `mysqlcheck --analyze` before starting the mysqld daemon, no matter what version
of the data is detected. It analyzes all the tables.

More values are separated by comma, e.g. `MYSQL_UPGRADE="optimize,analyze"`.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah explaining that the actions will be run in order is good... most people probably want to upgrade before optimizing, i would think.

function version2number() {
local version_major=${1%.*}
printf %d%02d ${version_major%%.*} ${version_major##*.}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems fragile. if additional dots are introduced in the string, or if the minor version number exceeds 2 digits, it will break.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Solution version_major=$(echo "$1" | grep -o -e '^[0-9]*\.[0-9]*') looks simplest from what I was thinking about, but if anybody has an idea how to make it correct enough without a fork, let me know..

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#! /bin/sh -x

version2number ()
{
    set -- "${1%%.*}" "$1"
    set -- "$1" "${2##$1\.}"
    set -- "$1" "${2%%.*}"
    printf -v version2number_result %d%02d "$1" "$2"
}

version2number "$1"
echo "$version2number_result"


# Returns version from the daemon in integer format
function mysqld_compat_version() {
local compat_version=$(mysqld_version)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why's the point of this variable? doesn't seem used.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left-over from previous implementation, I'll remove the line.

@hhorak
Copy link
Member Author

hhorak commented Feb 2, 2018

[test-openshift]

@hhorak hhorak changed the title [WIP] Add logic for upgrading datadir Add logic for upgrading datadir Feb 2, 2018
@hhorak
Copy link
Member Author

hhorak commented Feb 2, 2018

This PR is now rebased and hopefully prepared finally.

@hhorak
Copy link
Member Author

hhorak commented Feb 3, 2018

[test-openshift]

2 similar comments
@hhorak
Copy link
Member Author

hhorak commented Feb 4, 2018

[test-openshift]

@pkubatrh
Copy link
Member

[test-openshift]

@pkubatrh
Copy link
Member

@hhorak can you update common scripts to get more waiting time for docker registry?

echo "$datadir/mysql_upgrade_info"
}

# Writes version string of the daemon into mysql_upgrade_info file (should be only used when the file is missing and only when
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there missing something on the end after "when"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that was supposed to say something like "only until it gets to most of the deployments"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will fix..

version2number $(mysqld_version)
}

# Returns version from the datadir
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add "in integer format"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion, will add this.

write_mysql_upgrade_info_file "${MYSQL_DATADIR}"
continue
# This is currently a dead-code, but should be enabled after the mysql_upgrade_info
# file gets to the deployments (after few monts most of the deployments should already have the file)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

months

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hups

else
log_warn "Automatic upgrade is not turned on, proceed with the upgrade."\
"In order to upgrade the data directory, run this container with the MYSQL_UPGRADE"\
"environment variable set to 'auto' or run running mysql_upgrade manually. $(upstream_upgrade_info)"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"run running"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will remove 'running'

the version file is created if not exist, but no `mysql_upgrade` will be called.
However, this automatic creation will be removed after few months, since the version should be
created on most deployments at that point.
* `auto` -- `mysql_upgrade` is run at the beginning of the container start, if the data version
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is run after container start or after daemon, like force?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both run at the same time, so will use also the same wording, good catch.

hhorak added 2 commits May 3, 2018 18:13
MySQL and MariaDB use versions that consist of three numbers X.Y.Z (e.g. 5.6.23).
For version changes in Z part, the server's binary data format stays compatible and thus no
special upgrade procedure is needed. For upgrades from X.Y to X.Y+1, consider doing manual
steps as described at
https://mariadb.com/kb/en/library/upgrading-from-mariadb-101-to-mariadb-102/

Skipping versions like from X.Y to X.Y+2 or downgrading to lower version is not supported;
the only exception is ugrading from MariaDB 5.5 to MariaDB 10.0.

**Important**: Upgrading to a new version is always risky and users are expected to make a full
back-up of all data before.

A safer solution to upgrade is to dump all data using `mysqldump` or `mysqldbexport` and then
load the data using `mysql` or `mysqldbimport` into an empty (freshly initialized) database.

Another way of proceeding with the upgrade is starting the new version of the `mysqld` daemon
and run `mysql_upgrade` right after the start. This so called in-place upgrade is generally
faster for large data directory, but only possible if upgrading from the very previous version,
so skipping versions is not supported.

This container detects whether the data needs to be upgraded using `mysql_upgrade` and
we can control it by setting `MYSQL_UPGRADE` variable, which can have one or more of the following values:

 * `warn` -- If the data version can be determined and the data come from a different version
   of the daemon, a warning is printed but the container starts. This is the default value.
   Since historically the version file `mysql_upgrade_info` was not created, when using this option,
   the version file is created if not exist, but no `mysql_upgrade` will be called.
   However, this automatic creation will be removed after few months, since the version should be
   created on most deployments at that point.
 * `auto` -- `mysql_upgrade` is run at the beginning of the container start, if the data version
   can be determined and the data come with the very previous version. A warning is printed if the data
   come from even older or newer version. This value effectively enables automatic upgrades,
   but it is always risky and users should still back-up all the data before starting the newer container.
   Set this option only if you have very good back-ups at any moment and you are fine to fail-over
   from the back-up.
 * `force` -- `mysql_upgrade` is run right after the daemon has started, no matter what version the data
   come from. This is also the way to create the missing version file `mysql_upgrade_info` if not present
   in the root of the data directory; this file holds information about the version of the data.
 * `optimize` -- runs `mysqlcheck --optimize` before starting the mysqld daemon, no matter what version
   of the data is detected. It optimizes all the tables.
 * `analyze` -- runs `mysqlcheck --analyze` before starting the mysqld daemon, no matter what version
   of the data is detected. It analyzes all the tables.
 * `disable` -- nothing is done regarding data directory version.

Multiple values are separated by comma and run in-order, e.g. `MYSQL_UPGRADE="optimize,analyze"`.
@hhorak
Copy link
Member Author

hhorak commented May 3, 2018

[test-openshift]

@hhorak
Copy link
Member Author

hhorak commented May 3, 2018

[test-openshift]

@hhorak
Copy link
Member Author

hhorak commented May 4, 2018

@kubco2, thanks for all the comments, I believe I fixed all the suggestions, and now even the tests seem to pass. Anything else you'd like to see changed before merging?

@hhorak hhorak merged commit 0924c10 into sclorg:master May 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants