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

c/self-test/diskcheck: Avoid large allocations #11173

Merged
merged 1 commit into from
Jun 5, 2023

Conversation

BenPope
Copy link
Member

@BenPope BenPope commented Jun 2, 2023

diskcheck_opts::request_size is provided by the user.

To avoid very large allocations, split the request size into at most 128KiB chunks, and read/write them via an iovec.

Local testing with a single core:

            Prior to the change                           With the change

NAME        512KB sequential r/w throughput disk test     512KB sequential r/w throughput disk test
INFO        write run                                     write run
TYPE        disk                                          disk
TEST ID     ff291688-11f8-4d94-a5bc-62329c105f55          90b89748-f61e-4ab9-bd3b-425ad7672e42
TIMEOUTS    0                                             0
DURATION    30006ms                                       30004ms
IOPS        386 req/sec                                   382 req/sec
THROUGHPUT  193MiB/sec                                    191.3MiB/sec
LATENCY     P50     P90      P99      P999     MAX        P50     P90      P99      P999     MAX
            7167us  26623us  27647us  45055us  81919us    6911us  26623us  28671us  43007us  51199us
                                                          
NAME        512KB sequential r/w throughput disk test     512KB sequential r/w throughput disk test
INFO        read run                                      read run
TYPE        disk                                          disk
TEST ID     ff291688-11f8-4d94-a5bc-62329c105f55          90b89748-f61e-4ab9-bd3b-425ad7672e42
TIMEOUTS    0                                             0
DURATION    30000ms                                       30000ms
IOPS        11312 req/sec                                 11277 req/sec
THROUGHPUT  5.524GiB/sec                                  5.506GiB/sec
LATENCY     P50    P90    P99    P999   MAX               P50    P90    P99    P999    MAX
            351us  495us  703us  959us  1919us            351us  495us  703us  1151us  3583us
                                                          
NAME        4KB sequential r/w latency/iops disk test     4KB sequential r/w latency/iops disk test
INFO        write run                                     write run
TYPE        disk                                          disk
TEST ID     ff291688-11f8-4d94-a5bc-62329c105f55          90b89748-f61e-4ab9-bd3b-425ad7672e42
TIMEOUTS    0                                             0
DURATION    30011ms                                       30010ms
IOPS        135 req/sec                                   134 req/sec
THROUGHPUT  542KiB/sec                                    537.2KiB/sec
LATENCY     P50      P90      P99      P999     MAX       P50      P90      P99      P999     MAX
            14847us  14847us  21503us  30719us  40959us   14847us  15359us  23551us  36863us  49151us
                                                          
NAME        4KB sequential r/w latency/iops disk test     4KB sequential r/w latency/iops disk test
INFO        read run                                      read run
TYPE        disk                                          disk
TEST ID     ff291688-11f8-4d94-a5bc-62329c105f55          90b89748-f61e-4ab9-bd3b-425ad7672e42
TIMEOUTS    0                                             0
DURATION    30000ms                                       30000ms
IOPS        1278559 req/sec                               1262953 req/sec
THROUGHPUT  4.877GiB/sec                                  4.818GiB/sec
LATENCY     P50   P90   P99   P999  MAX                   P50   P90   P99   P999  MAX
            1us   1us   9us   71us  9215us                1us   1us   9us   14us  9215us

Fixes #10305

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v23.1.x
  • v22.3.x
  • v22.2.x

Release Notes

Improvements

  • Self-Test: Avoid large allocations during diskcheck

`diskcheck_opts::request_size` is provided by the user.

To avoid very large allocations, split the request size into
at most 128KiB chunks, and read/write them via an iovec.

Preliminary testing shows very little difference in the
performance.

Fixes redpanda-data#10305

Signed-off-by: Ben Pope <ben@redpanda.com>
@BenPope BenPope force-pushed the cluster-diskcheck-allocs branch from b4e2227 to 33313e9 Compare June 2, 2023 17:35
@BenPope BenPope marked this pull request as ready for review June 2, 2023 17:35
@dotnwat dotnwat requested a review from graphcareful June 2, 2023 18:38
iov.reserve(_opts.request_size / buf_len);
for (size_t offset = 0; offset < _opts.request_size; offset += buf_len) {
size_t len = std::min(_opts.request_size - offset, buf_len);
iov.push_back(iovec{buf.get(), len});
Copy link
Member

Choose a reason for hiding this comment

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

should we regenerate with new random data here? potentially to avoid making compression more effective?

Copy link
Contributor

Choose a reason for hiding this comment

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

There's no compression option in self_test::diskcheck , so i think its cool

@dotnwat
Copy link
Member

dotnwat commented Jun 2, 2023

ping @graphcareful

Copy link
Contributor

@graphcareful graphcareful left a comment

Choose a reason for hiding this comment

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

LGTM nice job posting how the change makes no huge negative impact on results

iov.reserve(_opts.request_size / buf_len);
for (size_t offset = 0; offset < _opts.request_size; offset += buf_len) {
size_t len = std::min(_opts.request_size - offset, buf_len);
iov.push_back(iovec{buf.get(), len});
Copy link
Contributor

Choose a reason for hiding this comment

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

There's no compression option in self_test::diskcheck , so i think its cool

@dotnwat dotnwat merged commit c4f3793 into redpanda-data:dev Jun 5, 2023
@vbotbuildovich
Copy link
Collaborator

/backport v23.1.x

@piyushredpanda
Copy link
Contributor

Thanks for this fix, @BenPope!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Oversized allocation: 524288 bytes in cluster::self_test::diskcheck::read_or_write
5 participants