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

storage,server: replace StickyInMemEngines with vfs.FS registry #107177

Closed
jbowens opened this issue Jul 19, 2023 · 1 comment · Fixed by #107829
Closed

storage,server: replace StickyInMemEngines with vfs.FS registry #107177

jbowens opened this issue Jul 19, 2023 · 1 comment · Fixed by #107829
Assignees
Labels
A-kv-server Relating to the KV-level RPC server A-storage Relating to our storage engine (Pebble) on-disk storage. C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior. quality-friday A good issue to work on on Quality Friday T-storage Storage Team

Comments

@jbowens
Copy link
Collaborator

jbowens commented Jul 19, 2023

Many tests across the codebase make use of the StickyInMemEnginesRegistry. This registry dates back to before Pebble, and is used to preserve state across Engine Closes. Typically this is intended to test behavior across node restarts. Reusing the entire engine avoids exercising all the code paths associated with recovering engine state from a crash. We should change as many of these tests as we can to only reuse a vfs.MemFS. Additionally, we can add support for using vfs.NewStrictMem which can be used to simulate hard crashes by calling SetIgnoreFsyncs(true) before closing the Engine and ResetToSyncedState afterward to revert to the prior state.

Jira issue: CRDB-29934

@jbowens jbowens added C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior. A-storage Relating to our storage engine (Pebble) on-disk storage. A-kv-server Relating to the KV-level RPC server T-storage Storage Team labels Jul 19, 2023
@jbowens jbowens added the quality-friday A good issue to work on on Quality Friday label Jul 20, 2023
@jbowens
Copy link
Collaborator Author

jbowens commented Jul 25, 2023

Additionally, we can add support for using vfs.NewStrictMem which can be used to simulate hard crashes by calling SetIgnoreFsyncs(true) before closing the Engine and ResetToSyncedState afterward to revert to the prior state.

To use this in tests with multiple nodes, I think we'll also need a mechanism of stopping all network connections from the node. It seems worthwhile but quite a lift.

@jbowens jbowens self-assigned this Jul 28, 2023
jbowens added a commit to jbowens/cockroach that referenced this issue Jul 31, 2023
The 'StickyInMemEngineRegistry' has historically been the method of preserving
local physical state within tests across node/TestCluster restarts. This
registry predates Pebble and its pure-Go VFS interface. This commit refactors
the sticky engine registry into a registry of virtual filesystems. This
improves test coverage by exercising storage engine teardown and recovery code.

The previous behavior of reusing the storage Engine without closing and
re-opening is still supported with the server.ReuseEngines option.

Close cockroachdb#107177.
Epic: None
Release note: None
jbowens added a commit to jbowens/cockroach that referenced this issue Aug 2, 2023
The 'StickyInMemEngineRegistry' has historically been the method of preserving
local physical state within tests across node/TestCluster restarts. This
registry predates Pebble and its pure-Go VFS interface. This commit refactors
the sticky engine registry into a registry of virtual filesystems. This
improves test coverage by exercising storage engine teardown and recovery code.

The previous behavior of reusing the storage Engine without closing and
re-opening is still supported with the server.ReuseEngines option.

Close cockroachdb#107177.
Epic: None
Release note: None
craig bot pushed a commit that referenced this issue Aug 3, 2023
107829: server: refactor the sticky engine registry into a VFS registry r=jbowens a=jbowens

The 'StickyInMemEngineRegistry' has historically been the method of preserving local physical state within tests across node/TestCluster restarts. This registry predates Pebble and its pure-Go VFS interface. This commit refactors the sticky engine registry into a registry of virtual filesystems. This improves test coverage by exercising storage engine teardown and recovery code.

The previous behavior of reusing the storage Engine without closing and re-opening is still supported with the server.ReuseEngines option.

Close #107177.
Epic: None
Release note: None

107988: sql: respect timezone for SHOW QUERIES/SESSIONS r=rafiss a=rafiss

fixes #107650
fixes #107649
Release note (sql change): The SHOW QUERIES and SHOW SESSIONS commands now will display timestamps using the session's timezone setting.

Co-authored-by: Jackson Owens <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>
@craig craig bot closed this as completed in 6ca7d2f Aug 3, 2023
pav-kv pushed a commit to pav-kv/cockroach that referenced this issue Jan 8, 2024
The 'StickyInMemEngineRegistry' has historically been the method of preserving
local physical state within tests across node/TestCluster restarts. This
registry predates Pebble and its pure-Go VFS interface. This commit refactors
the sticky engine registry into a registry of virtual filesystems. This
improves test coverage by exercising storage engine teardown and recovery code.

The previous behavior of reusing the storage Engine without closing and
re-opening is still supported with the server.ReuseEngines option.

Close cockroachdb#107177.
Epic: None
Release note: None
@jbowens jbowens moved this to Done in [Deprecated] Storage Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-kv-server Relating to the KV-level RPC server A-storage Relating to our storage engine (Pebble) on-disk storage. C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior. quality-friday A good issue to work on on Quality Friday T-storage Storage Team
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant