Skip to content

crl-release-25.2: metamorphic: fix shared and external directories #4777

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

Merged

Conversation

RaduBerinde
Copy link
Member

Backport of metamorphic crossversion fixes #4761, #4760, #4755, #4776

Informs #4732

metamorphic: use FS-based remote storage

The metamorphic test uses in-mem remote storage, which doesn't work
when starting with an initial state (as in the crossversion tests).

Note that there is code to save the remote storage contents to disk
when the store is in-memory, but they are not read back when used as
initial state.

This commit changes to using FS-based remote storage in shared
and external subdirs inside the data dir. Note that the store itself
can still be in-memory; the data gets saved automatically with the
store.

We improve the FS-based Storage implementation to sync data and list
objects. We can now allow simulating crashes in the metamorphic test
when shared storage is enabled.

metamorphic: support CreateOnShared on existing store

If the metamorphic test has CreateOnShared set to something other
than "none" and we start with an initial store which did not have
remote storage configured, there can be background errors right after
opening the store, before we get a chance to call SetCreatorID().
The metamorphic test fails on these background errors.

To fix this, we always open with CreateOnSharedNone first, and if
necessary reopen after setting the creator ID.

metamorphic: add random number to external object names

External object names can collide with existing objects when starting
with an initial state. This change adds a random unique number to the
filenames.

db: support store-relative paths for WAL dirs

Relative WAL paths (including the actual WAL, the failover path, and
the recovery paths) are (unfortunately) interpreted as relative to the
current working directory.

The cross-version metamorphic test copies a store from a previous run
as the initial state for a new test. The options will fail the
compatibility check since the path changes.

This change adds support for using a special {store_path} prefix
inside the path. Any such prefix is replaced with the store directory.

We also improve the missing WAL recovery dir error to show what
directories are actually configured.

metamorphic: use store-relative paths

metamorphic: fix code around WAL recovery directories

metamorphic: sanity check initial-state and previous-ops flags

Error out if --initial-state is used without --previous-ops.

crossversion: pass --previous-ops flag

Lack of this flag is the root cause for the crossversion tests not
actually testing across versions (instead, each run was separate).

metamorphic: fix shared and external directories

When multiple stores are used, the shared and external directories
were set up inside the data dir of each store, which is not correct.
This commit fixes the paths.

The metamorphic test uses in-mem remote storage, which doesn't work
when starting with an initial state (as in the crossversion tests).

Note that there is code to save the remote storage contents to disk
when the store is in-memory, but they are not read back when used as
initial state.

This commit changes to using FS-based remote storage in `shared`
and `external` subdirs inside the data dir. Note that the store itself
can still be in-memory; the data gets saved automatically with the
store.

We improve the FS-based Storage implementation to sync data and list
objects. We can now allow simulating crashes in the metamorphic test
when shared storage is enabled.
If the metamorphic test has `CreateOnShared` set to something other
than "none" and we start with an initial store which did not have
remote storage configured, there can be background errors right after
opening the store, before we get a chance to call `SetCreatorID()`.
The metamorphic test fails on these background errors.

To fix this, we always open with `CreateOnSharedNone` first, and if
necessary reopen after setting the creator ID.
External object names can collide with existing objects when starting
with an initial state. This change adds a random unique number to the
filenames.
Relative WAL paths (including the actual WAL, the failover path, and
the recovery paths) are (unfortunately) interpreted as relative to the
current working directory.

The cross-version metamorphic test copies a store from a previous run
as the initial state for a new test. The options will fail the
compatibility check since the path changes.

This change adds support for using a special `{store_path}` prefix
inside the path. Any such prefix is replaced with the store directory.

We also improve the missing WAL recovery dir error to show what
directories are actually configured.
Error out if `--initial-state` is used without `--previous-ops`.
Lack of this flag is the root cause for the crossversion tests not
actually testing across versions (instead, each run was separate).
When multiple stores are used, the shared and external directories
were set up inside the data dir of each store, which is not correct.
This commit fixes the paths.
@RaduBerinde RaduBerinde requested a review from jbowens May 28, 2025 14:16
@RaduBerinde RaduBerinde requested a review from a team as a code owner May 28, 2025 14:16
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Collaborator

@jbowens jbowens left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 3 of 3 files at r1, 3 of 3 files at r2, 3 of 3 files at r3, 5 of 5 files at r4, 2 of 2 files at r5, 3 of 3 files at r6, 3 of 3 files at r7, 1 of 1 files at r8, 1 of 1 files at r9, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @RaduBerinde)

@RaduBerinde
Copy link
Member Author

TFTR!

@RaduBerinde RaduBerinde merged commit 55b3232 into cockroachdb:crl-release-25.2 May 28, 2025
4 checks passed
@RaduBerinde RaduBerinde deleted the fix-cross-meta-25.2 branch May 28, 2025 18:16
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.

3 participants