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

crutest's RegionInfo needs to support Volume level. #1454

Open
leftwo opened this issue Sep 11, 2024 · 1 comment
Open

crutest's RegionInfo needs to support Volume level. #1454

leftwo opened this issue Sep 11, 2024 · 1 comment
Assignees

Comments

@leftwo
Copy link
Contributor

leftwo commented Sep 11, 2024

The current way crutest works for many tests is to build a RegionInfo struct which looks as follows:

pub struct RegionInfo {                                                             
    block_size: u64,                                                                
    extent_size: Block,                                                             
    total_size: u64,                                                                
    total_blocks: usize,                                                            
    write_log: WriteLog,                                                            
    max_block_io: usize,                                                            
}

This allows it to figure out where the extents end and from that how to create tests that catch edge conditions for us without having to first inspect the region then manually create a specific test for it.

In the volume centric world, where we can have multiple SubVolumes, this single structure is no longer contains the information we need, and can't actually represent what we want.

@leftwo
Copy link
Contributor Author

leftwo commented Sep 11, 2024

Perhaps a new struct, replace RegionInfo with VolumeInfo

For a Volume, We could store
<Vec<RegionInfo>> for each sub_volume
<Option<RegionInfo>> for read_only_parent

With a geust, it would just be:
A vec, with just one RegionInfo
None for read_only_parent

Something like this that mimics the way we do volumes:

struct VolumeInfo {
    // Block size for the volume,
    block_size: u64,
    // Total blocks in the volume
    total_blocks: u64,
    // SubVolumes that make up this volume.
    sub_volume: Vec<SubVolumeInfo>
    read_only_parent: Option<SubVolumeInfo>
    // write log for the volume
    write_log: WriteLog,
} 

With

struct SubVolumeInfo{
    // Number blocks per extent
    extent_size: Block,
    // Total extents in this sub volume
    extent_count: u64,
    // Total size in bytes for this sub volume
    total_size: u64,
    // Range this sub volume covers.
    range: Range,
}

I'm not even sure if we need to (at first) support read only parents in crutest, but we can if we want.

@leftwo leftwo self-assigned this Sep 11, 2024
leftwo pushed a commit that referenced this issue Sep 25, 2024
These changes should be a fix for both issues:
#1451
#1457

This adds support for crutest to use a provided dsc endpoint to construct
a Volume object.

Moved the existing volume creation steps to a new function, and added another
option on how we can create a Volume.  The two previous ways of creating
a volume are not changed (though I changed a log message and added some
warnings).  The new code is in taking the dsc provided endpoint and
using that to construct a volume.

Additional dsc changes were made to help provide Volume info.

Renamed things in dsc to better reflect what information they hold.
Specifically, update a bunch of region set comments, as dsc just controls
crucible-downstairs processes, and does not know which ones are part of
what region set.

New dsc commands:
get_ds_uuid: Returns the UUID for the given client ID.
all_running: Returns true if all downstairs that dsc knows about are currently
in Running state.
get_region_count: Returns the total number of regions that dsc knows about.

New dsc behavior.
dsc will now wait on all downstairs starting before taking any commands.
The ability for dsc to answer a request can be used by a test to confirm that
all downstairs had started.
Add the ability to supply a dsc endpoint to crutest-cli
#1459

tools/test_replay.sh transitioned to using the new --dsc option, as that
test already required a dsc endpoint and was using a hard coded default
value for it.

tools/test_restart_repair.sh was updated to wait for dsc to report that
all downstairs are online after a restart.  This avoids a race where
we told dsc to start, and then start crutest, but the downstairs are not
yet online.

All the tests that use dsc will eventually transition to using it to
construct a Volume, but I'm pushing that work to another PR.

There are more changes coming, specifically:
Updating replace-before-acive and replace-reconcile to use the new
dsc option correctly instead of a default.
Updating tools/test_* to not require targets for crutest and instead use
dsc option.
More updates in crutest to update the current RegionInfo struct to be
aware of multiple sub-volumes.
#1454

Possibly some updates to the BlockIO trait.
#1455

Other work this enables
Making tests that use multiple sub-volumes.
Layering the current set of tests in a way so we can run the same tests

with a single sub-volume and with multiple sub-volumes
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

No branches or pull requests

1 participant