Write Impact Recipe Changes

[aschomin]

Seagate is proposing the following changes to "Write_Impact_Check.rcp" in order to comply with the public release of the OCS Specification 2.1.

The changes modify the following parameters:

1. All reads are random 4K.
2. All injected writes are 256K.
3. The writes during the 5% Injected sequence are random.
4. The writes during the 10% Injected sequence are sequential.
5. Various cosmetic changes.

[msftclas]

Hi @aschomin, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution! This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. Real humans will now evaluate your PR.

TTYL, MSBOT;

[marksantaniello]

Seems like a lot of whitespace-only changes are making it hard to see the true effect of the change. Could you submit a PR without the whitespace diffs?

[aschomin]

A foreach loop was removed from the recipe so the white space changes are necessary to keep indentations consistent. I do see that my text editor removed a space from the end of line 7 and I can remove that if you would like.

[marksantaniello]

Gotcha, now I see it. Yeah line 7 is noise and should be removed.

I see you changed the read workload's QD to 1, which also matches the spec. Seems good, although Microsoft should weigh in here. I'm not sure if they want this recipe to exactly match the OCS spec, or to be a super set. (But it does seem pretty clearly bad if the recipe doesn't cover some part of the spec at all.)

I find the new line 66 a little bit odd. I see why you are doing it, but I worry that someone could change line 56 later on and not realize they were totally disabling random writes. Maybe there is another way? Something like:

my @write_workloads = ( 
    [ 'Random', 5 ],
    [ 'Sequential', 10 ]
);

foreach my $write_workload (@write_workloads)
{
    my ($write_pct, $write_pattern) = @$write_workload;
    ...
}

[aschomin]

We would be fine with the super set as long as the OCS spec is covered. If there is specific list of workloads Microsoft wants to be covered, including the OCS ones, I would be happy to modify the recipe to include them.

I agree with your proposed code above. I will work on making the changes and commit.

[lauracaulfield]

Looks good! Thanks for making these fixes, and for leaving the super-set. Even though they're not in the spec, I think they'll be interesting to look at.

[aschomin]

The version of the recipe that we requested to merge here is only the spec test cases. What I was saying before is if we want to include the original cases as well I would be happy to put them in. I was just waiting to see if that is what we wanted to do. If that is the case let me know and I will get them in also.

[lauracaulfield]

ah, ok. I think what's in the repo now is what we want -- the spec covers quite a few cases, and we also want to keep test time low.