Ensure that binary logs for PITR are in a shared directory by mattlord · Pull Request #541 · planetscale/vitess-operator

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

+	// 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

 	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.

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>

nickvanw · 2026-03-06T23:41:20Z

-    echo "Backup failed"
+  for i in {1..600} ; do
+    out=$(kubectl get vtb -n example --no-headers | wc -l)
+    if echo "${out}" | grep -c "${finalBackupCount}" >/dev/null; then


finalBackupCount does not seem to be initialized before this first wait. With an empty value, grep -c "" returns 1, so we break immediately here. Then after the increment below, the second loop only waits for count 1, which the full backup already satisfies. That means this test can pass without ever observing the incremental backup.

Good catch! Fixed.

nickvanw

I traced the PITR/operator flow through the changed code and upstream Vitess and left a few inline comments. The shared restore-path change looks directionally right, but I found one runtime wiring issue and a couple of gaps in the new end-to-end coverage.

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 GuptaManan100, frouioui and shlomi-noach 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 3 commits March 28, 2024 13:04

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

mattlord added 3 commits July 12, 2024 13:12

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

34b93ad

Add mysqlbinlog to $VTROOT/bin in backup pods

ce28c33

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

Testing...

5e7df2b

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

mattlord force-pushed the point-in-time-recovery branch from ae78d72 to 5e7df2b Compare July 12, 2024 18:55

mattlord removed the request for review from GuptaManan100 March 4, 2026 14:50

mattlord added 4 commits March 6, 2026 14:54

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

e401e32

Cleanup old version check

be3a4f5

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

Fix mounts and test

2e90621

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

Run make generate

5813ef9

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

mattlord requested a review from frouioui March 6, 2026 23:14

nickvanw reviewed Mar 6, 2026

View reviewed changes

Comment thread test/endtoend/utils.sh Outdated

nickvanw reviewed Mar 6, 2026

View reviewed changes

Comment thread pkg/operator/vttablet/env_vars.go Outdated

nickvanw reviewed Mar 6, 2026

View reviewed changes

Address review comments

faf07ea

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

nickvanw approved these changes Mar 7, 2026

View reviewed changes

mattlord added 2 commits March 7, 2026 01:22

Address review feedback

c705697

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

Test fixes

dc8fa0b

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

mattlord force-pushed the point-in-time-recovery branch from 05d2aa7 to dc8fa0b Compare March 7, 2026 03:26

mattlord merged commit 547553e into main Mar 8, 2026
13 checks passed

mattlord deleted the point-in-time-recovery branch March 8, 2026 01:58

Conversation

mattlord commented Mar 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

vtctldclient

vtctld[server]

vttablet

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

vttablet builtinbackupengine

Related issues and PRs:

Uh oh!

shlomi-noach left a comment

Choose a reason for hiding this comment

Uh oh!

mattlord commented Mar 16, 2024

Uh oh!

shlomi-noach commented Mar 17, 2024 • edited by mattlord Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mattlord commented Mar 17, 2024

Uh oh!

shlomi-noach left a comment

Choose a reason for hiding this comment

Uh oh!

mattlord commented Mar 27, 2024

Uh oh!

frouioui Mar 27, 2024

Choose a reason for hiding this comment

Uh oh!

frouioui Mar 27, 2024

Choose a reason for hiding this comment

Uh oh!

mattlord Mar 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

frouioui Mar 27, 2024

Choose a reason for hiding this comment

Uh oh!

mattlord Mar 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nickvanw Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

mattlord Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nickvanw left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mattlord commented Mar 12, 2024 •

edited

Loading

`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

Loading

mattlord Mar 27, 2024 •

edited

Loading

mattlord Mar 27, 2024 •

edited

Loading