Ensure that binary logs for PITR are in a shared directory #541

mattlord · 2024-03-12T15:42:23Z

When executing the vtctldclient RestoreFromBackup --restore-to-pos <value> command, the vttablet process in the vttablet container within the vttablet pod — in the RestoreFromBackup tabletmanager RPC — restores the full backup within the VTDATAROOT (specifically /vt/vtdataroot/vt_<tabletUID>/ for the mysql data) that is shared by all containers within the pod using the configured backup engine (e.g. xtrabackup). It orchestrates that in conjunction with the mysqlctld process that's running inside the mysqld container within the same vttablet pod. In the end there is a running mysqld instance inside the mysqld container that is from the restored full backup. Then once the full backup is in place and the mysqld process is running the vttablet process uses the OS tmp dir of /tmp to restore the binary logs from the backup — via the builtinbackupengine — for subsequent application and /tmp is not a shared mount point within the pod so when mysqlbinlog subsequently tries to read them from within the mysqld container it cannot find them in its container's /tmp directory and it fails with an error.

Related issues and PRs:

Signed-off-by: Matt Lord <mattalord@gmail.com>

shlomi-noach

Nice! Should we also provide the flag in yaml files?

Signed-off-by: Matt Lord <mattalord@gmail.com>

mattlord · 2024-03-16T04:00:28Z

Nice! Should we also provide the flag in yaml files?

Yeah. I think this does it. e2e5e8b

shlomi-noach · 2024-03-17T17:41:08Z

Yeah. I think this does it.

How is the value being set, and to what specific value?

mattlord · 2024-03-17T17:45:36Z

How is the value being set, and to what specific value?

The user would specify the flag and value in their cluster yaml definition using the extraFlags parameter, just as they do for mysqld flags, e.g. If they don't specify a value then we enforce the default within the operator.

shlomi-noach

Seems to me like it's feature complete and can be taken out of Draft?

Signed-off-by: Matt Lord <mattalord@gmail.com>

mattlord · 2024-03-27T00:22:04Z

How is the value being set, and to what specific value?

The user would specify the flag and value in their cluster yaml definition using the extraFlags parameter, just as they do for mysqld flags, e.g. If they don't specify a value then we enforce the default within the operator.

The flag ended up being for vttablet and vtbackup, not mysqlctld (although vtbackup is a modified mysqlctld). I will leave the mysqlctld extra flags support though as that may come to be useful.

Signed-off-by: Matt Lord <mattalord@gmail.com>

frouioui · 2024-03-27T00:27:58Z

pkg/operator/vttablet/pod.go

+	// Ensure that binary logs are restored to/from a location that all containers
+	// in the pod can access if no location was explicitly provided.
+	if _, ok := vttabletAllFlags["builtinbackup-incremental-restore-path"]; !ok {
+		vttabletAllFlags["builtinbackup-incremental-restore-path"] = vtDataRootPath
+	}


What would happen if the path specified in --builtinbackup-incremental-restore-path is not accessible to all containers in the pod?

I am guessing it is up to the user to set the same value on all components too? mysqlctl, vttablet and vtbackup

The same thing that happens now to every user. It doesn't work. PITR does not generally work in the operator today.

frouioui · 2024-03-27T00:33:27Z

pkg/operator/vttablet/mysqlctld.go

@@ -29,6 +29,7 @@ const (
 	vtRootInitScript = `set -ex
 mkdir -p /mnt/vt/bin
 cp --no-clobber /vt/bin/mysqlctld /mnt/vt/bin/
+cp --no-clobber $(command -v mysqlbinlog) /mnt/vt/bin/ || true


The directory /mnt/.../ is shared across all the containers in the pod I am assuming? In which case, this line would resolve what you wrote in the PR's description:

/tmp is not a shared mount point within the pod so when mysqlbinlog subsequently tries to read them from within the mysqld container it cannot find them in its container's /tmp directory and it fails with an error

This is simply about copying the mysqlbinlog binary from the vitess/lite container image to the mysqlctld/vtbackup container (if it's not already there), as it looks like we'll need to keep that around in the lite image because the MySQL images do not contain that binary and it's needed for PITR.

frouioui · 2024-03-27T00:35:22Z

pkg/apis/planetscale/v2/vitessshard_types.go

+// MysqlctldSpec configures the local mysqlctld gRPC server within a tablet.
+type MysqlctldSpec struct {
+	// Resources specify the compute resources to allocate for just the MySQL Control Daemon.
+	Resources corev1.ResourceRequirements `json:"resources"`
+
+	// ExtraFlags can optionally be used to override default flags set by the
+	// operator, or pass additional flags to mysqlctld. All entries must be
+	// key-value string pairs of the form "flag": "value". The flag name should
+	// not have any prefix (just "flag", not "-flag"). To set a boolean flag,
+	// set the string value to either "true" or "false".
+	ExtraFlags map[string]string `json:"extraFlags,omitempty"`
+}


Unless I am missing something, this does not seem to be used anywhere. I think we should remove it to avoid unrequired changes

I'll remove it if it indeed is not needed after I do some testing to confirm that things are now working.

Coming back on what I said. I think it would be nice to keep that around so that people can use custom mysqlctld flags.

Signed-off-by: Matt Lord <mattalord@gmail.com>

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

test/endtoend/utils.sh

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

Signed-off-by: Matt Lord <mattalord@gmail.com>

This includes mysqld (of course) and mysqlbinlog But it does NOT include xtrabackup Signed-off-by: Matt Lord <mattalord@gmail.com>

Signed-off-by: Matt Lord <mattalord@gmail.com>

mattlord force-pushed the point-in-time-recovery branch 5 times, most recently from 1824f91 to 0562d49 Compare March 13, 2024 01:15

mattlord requested review from shlomi-noach, frouioui and GuptaManan100 and removed request for GuptaManan100 March 13, 2024 01:15

mattlord changed the title ~~Ensure that binary logs for PITR are restored to a shared location~~ Ensure that binary logs for PITR are use a shared location Mar 13, 2024

mattlord changed the title ~~Ensure that binary logs for PITR are use a shared location~~ Ensure that binary logs for PITR are use a shared directory Mar 13, 2024

mattlord changed the title ~~Ensure that binary logs for PITR are use a shared directory~~ Ensure that binary logs for PITR are in a shared directory Mar 13, 2024

Ensure that binary logs for PITR are restored to a shared location

d259730

Signed-off-by: Matt Lord <mattalord@gmail.com>

mattlord force-pushed the point-in-time-recovery branch from 0562d49 to d259730 Compare March 13, 2024 02:25

shlomi-noach reviewed Mar 13, 2024

View reviewed changes

Add ability to specify flags for mysqlctld

e2e5e8b

Signed-off-by: Matt Lord <mattalord@gmail.com>

shlomi-noach approved these changes Mar 18, 2024

View reviewed changes

mattlord marked this pull request as ready for review March 18, 2024 12:18

mattlord requested a review from GuptaManan100 March 18, 2024 12:18

shlomi-noach requested a review from a team March 20, 2024 06:28

mattlord added 3 commits March 26, 2024 12:35

Merge remote-tracking branch 'origin/main' into point-in-time-recovery

9992074

It's a tablet flag not a mysqlctld one

10a9524

Signed-off-by: Matt Lord <mattalord@gmail.com>

Merge remote-tracking branch 'origin/main' into point-in-time-recovery

af6ebcb

Move the flag handling code

48ba49f

Signed-off-by: Matt Lord <mattalord@gmail.com>

frouioui reviewed Mar 27, 2024

View reviewed changes

mattlord added 5 commits March 27, 2024 11:30

Only set flag on 2.12+

411993c

Signed-off-by: Matt Lord <mattalord@gmail.com>

Add PITR restore to backup test

79bf28d

Signed-off-by: Matt Lord <mattalord@gmail.com>

Use recent vtctldclient

8277844

Signed-off-by: Matt Lord <mattalord@gmail.com>

Use different date command

9d2b978

Signed-off-by: Matt Lord <mattalord@gmail.com>

Use different date format

db311ed

Signed-off-by: Matt Lord <mattalord@gmail.com>

mattlord force-pushed the point-in-time-recovery branch 4 times, most recently from 4abb9ce to a2d80d2 Compare March 27, 2024 21:45

Test incremental backup and restore

63abf80

Signed-off-by: Matt Lord <mattalord@gmail.com>

mattlord force-pushed the point-in-time-recovery branch from a2d80d2 to 63abf80 Compare March 27, 2024 22:08

mattlord and others added 2 commits March 27, 2024 23:26

WiP

425b725

Signed-off-by: Matt Lord <mattalord@gmail.com>

attempt to fix tests

75c701d

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

shlomi-noach reviewed Mar 28, 2024

View reviewed changes

test/endtoend/utils.sh Outdated Show resolved Hide resolved

shlomi-noach and others added 7 commits March 28, 2024 08:26

more debug output

f09d7be

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

more debug info

7716135

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

list backups as debug step

3452359

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

GetBackups

b1a0cb3

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

Get test working locally

919119b

Signed-off-by: Matt Lord <mattalord@gmail.com>

Ignore vtdataroot path created when running local tests

49b9332

Signed-off-by: Matt Lord <mattalord@gmail.com>

Merge remote-tracking branch 'origin/main' into point-in-time-recovery

5c809cc

mattlord force-pushed the point-in-time-recovery branch from c9793dd to e069303 Compare April 10, 2024 18:32

Fix backup test; use percona image

2aeb36b

This includes mysqld (of course) and mysqlbinlog But it does NOT include xtrabackup Signed-off-by: Matt Lord <mattalord@gmail.com>

mattlord force-pushed the point-in-time-recovery branch from e069303 to 2aeb36b Compare April 10, 2024 20:30

mattlord added 2 commits April 30, 2024 16:44

Merge remote-tracking branch 'origin/main' into point-in-time-recovery

573b44f

Use /vt/bin for mysqlbinlog and add it to PATH

2f7768a

Signed-off-by: Matt Lord <mattalord@gmail.com>

frouioui mentioned this pull request May 7, 2024

Install linux generic libc2.12 mysqlbinlog binary and libraries vitessio/vitess#15854

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ensure that binary logs for PITR are in a shared directory #541

Ensure that binary logs for PITR are in a shared directory #541

mattlord commented Mar 12, 2024 •

edited

shlomi-noach left a comment

mattlord commented Mar 16, 2024

shlomi-noach commented Mar 17, 2024 •

edited by mattlord

mattlord commented Mar 17, 2024

shlomi-noach left a comment

mattlord commented Mar 27, 2024

frouioui Mar 27, 2024

frouioui Mar 27, 2024

mattlord Mar 27, 2024 •

edited

frouioui Mar 27, 2024

mattlord Mar 27, 2024 •

edited

frouioui Mar 27, 2024

mattlord Mar 27, 2024

frouioui Mar 27, 2024

Ensure that binary logs for PITR are in a shared directory #541

Are you sure you want to change the base?

Ensure that binary logs for PITR are in a shared directory #541

Conversation

mattlord commented Mar 12, 2024 • edited

vtctldclient

vtctld[server]

vttablet

mysqlctld (rather than mysqlctl, and which runs in the mysql container)

vttablet builtinbackupengine

Related issues and PRs:

shlomi-noach left a comment

Choose a reason for hiding this comment

mattlord commented Mar 16, 2024

shlomi-noach commented Mar 17, 2024 • edited by mattlord

mattlord commented Mar 17, 2024

shlomi-noach left a comment

Choose a reason for hiding this comment

mattlord commented Mar 27, 2024

frouioui Mar 27, 2024

Choose a reason for hiding this comment

frouioui Mar 27, 2024

Choose a reason for hiding this comment

mattlord Mar 27, 2024 • edited

Choose a reason for hiding this comment

frouioui Mar 27, 2024

Choose a reason for hiding this comment

mattlord Mar 27, 2024 • edited

Choose a reason for hiding this comment

frouioui Mar 27, 2024

Choose a reason for hiding this comment

mattlord Mar 27, 2024

Choose a reason for hiding this comment

frouioui Mar 27, 2024

Choose a reason for hiding this comment

mattlord commented Mar 12, 2024 •

edited

`vtctldclient`

`vtctld`[server]

`vttablet`

`mysqlctld` (rather than `mysqlctl`, and which runs in the `mysql` container)

`vttablet` `builtinbackupengine`

shlomi-noach commented Mar 17, 2024 •

edited by mattlord

mattlord Mar 27, 2024 •

edited

mattlord Mar 27, 2024 •

edited