src/test: test vectored/buffered sync i/o with buffers exceeding 2GiB in total len by dparmar18 · Pull Request #58481 · ceph/ceph

dparmar18 · 2024-07-09T14:36:23Z

Contribution Guidelines

To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.
When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

Tracker (select at least one)
- References tracker ticket
- Very recent bug; references commit where it was introduced
- New feature (ticket optional)
- Doc update (no ticket needed)
- Code cleanup (no ticket needed)
Component impact
- Affects Dashboard, opened tracker ticket
- Affects Orchestrator, opened tracker ticket
- No impact that needs to be tracked
Documentation (select at least one)
- Updates relevant documentation
- No doc update is appropriate
Tests (select at least one)
- Includes unit test(s)
- Includes integration test(s)
- Includes bug reproducer
- No tests

Show available Jenkins commands

jenkins retest this please
jenkins test classic perf
jenkins test crimson perf
jenkins test signed
jenkins test make check
jenkins test make check arm64
jenkins test submodules
jenkins test dashboard
jenkins test dashboard cephadm
jenkins test api
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox
jenkins test windows
jenkins test rook e2e

vshankar

Looks good. Please squash the top 3 (test related) commits.

vshankar · 2024-07-18T06:22:05Z

src/test/libcephfs/test.cc

+  ASSERT_LT(0, fd);
+
+  const size_t BUFSIZE = static_cast<size_t>(3) * 1024 * 1024 * 1024;
+  auto out_buf = std::make_unique<char[]>(BUFSIZE);


Does this really need to be a unique_ptr?

buffers are large, I cannot allocate such huge memory on stack like it is being done in other test cases where the sizes are tiny like:

char out0[] = "hello "; char out1[] = "world\n";

For heap, I cannot use new and delete since they might leak in case of a abrupt failure. unique_ptr is safe and release memory when gets out of scope.

The abrupt failure is going to crash the process anyway. Are you worried about limit of memory allocation in process stack?

The abrupt failure is going to crash the process anyway. Are you worried about limit of memory allocation in process stack?

yes, that's the only concern, have ran out of stack memory couple of times while testing.

FYI (for anyone reading this): See #58564 (comment) for discussion on this.

vshankar · 2024-07-18T06:22:20Z

src/test/client/syncio.cc

+        {out_buf_2.get(), BUFSIZE}
+    };
+
+    auto in_buf_0 = std::make_unique<char[]>(BUFSIZE);


#58481 (comment)

dparmar18 · 2024-07-18T08:59:38Z

Looks good. Please squash the top 3 (test related) commits.

Doable for 2nd and 3rd but if you see the first one is temp.

github-actions · 2024-08-28T14:51:47Z

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

github-actions · 2024-12-16T09:01:52Z

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

dparmar18 · 2024-12-16T12:59:06Z

ping @rishabh-d-dave

rishabh-d-dave

Skipped the "temp" commit since it belongs to a different PR and will eventually be removed from this PR. Left few nits, but otherwise this PR looks okay to me.

src/test/client/syncio.cc

src/test/libcephfs/test.cc

github-actions · 2025-02-15T17:01:17Z

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

github-actions · 2025-03-03T13:38:15Z

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

dparmar18 · 2025-03-19T12:32:28Z

jenkins retest this please

dparmar18 · 2025-04-23T10:35:24Z

@vshankar @rishabh-d-dave RFR

dparmar18 · 2025-05-08T07:45:15Z

@vshankar ping

dparmar18 · 2025-05-28T11:44:24Z

@vshankar ping

github-actions · 2025-05-29T05:28:17Z

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

vshankar · 2025-06-03T07:26:15Z

@dparmar18 please rebase. @rishabh-d-dave PTAL for review once rebased.

dparmar18 · 2025-06-03T12:04:25Z

@dparmar18 please rebase. @rishabh-d-dave PTAL for review once rebased.

I can, though IMO #54435 should go first. There were some changes made to the helpers recently (based on your review comments) which needs to get a green flag first.

github-actions · 2025-08-02T13:10:38Z

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

github-actions · 2025-10-03T11:01:46Z

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

github-actions · 2025-12-02T15:02:03Z

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

dparmar18 · 2025-12-18T12:25:00Z

client code patch merged as part of d2699c4, this is now a test case only PR.
removed the tmp commit since the dependent PR src/test: add libcephfs tests for async(nonblocking) calls #54435 is merged. Request for a teuthology run

github-actions · 2026-02-16T13:25:58Z

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

dparmar18 · 2026-02-17T03:21:52Z

@vshankar @rishabh-d-dave ping

… len (as per posix, write/read calls are limited to transferring ~2GiB) Fixes: https://tracker.ceph.com/issues/66771 Signed-off-by: Dhairya Parmar <dparmar@redhat.com>

… len Fixes: https://tracker.ceph.com/issues/66771 Signed-off-by: Dhairya Parmar <dparmar@redhat.com>

github-actions bot added cephfs Ceph File System tests labels Jul 9, 2024

dparmar18 force-pushed the wip-66771 branch from ec5ab1f to 3fbad0a Compare July 9, 2024 14:37

dparmar18 requested a review from a team July 9, 2024 14:38

dparmar18 force-pushed the wip-66771 branch from 3fbad0a to b2e8bb4 Compare July 9, 2024 14:40

vshankar reviewed Jul 18, 2024

View reviewed changes

github-actions bot added the needs-rebase label Aug 28, 2024

dparmar18 force-pushed the wip-66771 branch from b2e8bb4 to 1b09de4 Compare October 9, 2024 10:38

github-actions bot removed the needs-rebase label Oct 9, 2024

dparmar18 force-pushed the wip-66771 branch 2 times, most recently from 3ffa633 to ec6b0ee Compare October 9, 2024 10:42

dparmar18 requested a review from vshankar October 9, 2024 10:43

vshankar assigned rishabh-d-dave Oct 17, 2024

github-actions bot added the stale label Dec 16, 2024

dparmar18 removed the stale label Dec 16, 2024

rishabh-d-dave approved these changes Dec 17, 2024

View reviewed changes

src/test/client/syncio.cc Outdated Show resolved Hide resolved

src/test/client/syncio.cc Show resolved Hide resolved

src/test/libcephfs/test.cc Show resolved Hide resolved

src/test/libcephfs/test.cc Show resolved Hide resolved

github-actions bot added the stale label Feb 15, 2025

dparmar18 removed the stale label Feb 17, 2025

dparmar18 force-pushed the wip-66771 branch from ec6b0ee to 642395a Compare February 18, 2025 11:47

dparmar18 mentioned this pull request Feb 18, 2025

src/test: add libcephfs tests for async(nonblocking) calls #54435

Merged

14 tasks

vshankar self-assigned this Feb 19, 2025

github-actions bot added the needs-rebase label Mar 3, 2025

dparmar18 added the needs-review label Apr 23, 2025

github-actions bot added the needs-rebase label May 29, 2025

github-actions bot added the stale label Aug 2, 2025

dparmar18 removed the stale label Aug 4, 2025

github-actions bot added the stale label Oct 3, 2025

dparmar18 removed the stale label Oct 3, 2025

github-actions bot added the stale label Dec 2, 2025

dparmar18 removed the stale label Dec 2, 2025

dparmar18 force-pushed the wip-66771 branch from 16fdebc to 495c1cf Compare December 18, 2025 12:15

github-actions bot removed the needs-rebase label Dec 18, 2025

dparmar18 changed the title ~~client: fix int overflow in vectored sync i/o code paths~~ src/test: test vectored/buffered sync i/o with buffers exceeding 2GiB in total len Dec 18, 2025

dparmar18 requested a review from rishabh-d-dave December 18, 2025 12:25

github-actions bot added the stale label Feb 16, 2026

dparmar18 removed the stale label Feb 17, 2026

dparmar18 added 2 commits February 19, 2026 10:15

src/test: test vectored sync i/o with buffers exceeding 2GiB in total…

948dbe0

… len (as per posix, write/read calls are limited to transferring ~2GiB) Fixes: https://tracker.ceph.com/issues/66771 Signed-off-by: Dhairya Parmar <dparmar@redhat.com>

src/test: test buffered sync i/o with buffers exceeding 2GiB in total…

d363d84

… len Fixes: https://tracker.ceph.com/issues/66771 Signed-off-by: Dhairya Parmar <dparmar@redhat.com>

dparmar18 force-pushed the wip-66771 branch from 495c1cf to d363d84 Compare February 19, 2026 04:45

Conversation

dparmar18 commented Jul 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution Guidelines

Checklist

Uh oh!

vshankar left a comment

Choose a reason for hiding this comment

Uh oh!

vshankar Jul 18, 2024

Choose a reason for hiding this comment

Uh oh!

dparmar18 Jul 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vshankar Aug 5, 2024

Choose a reason for hiding this comment

Uh oh!

dparmar18 Aug 5, 2024

Choose a reason for hiding this comment

Uh oh!

vshankar Mar 18, 2025

Choose a reason for hiding this comment

Uh oh!

vshankar Jul 18, 2024

Choose a reason for hiding this comment

Uh oh!

dparmar18 Jul 18, 2024

Choose a reason for hiding this comment

Uh oh!

dparmar18 commented Jul 18, 2024

Uh oh!

github-actions bot commented Aug 28, 2024

Uh oh!

github-actions bot commented Dec 16, 2024

Uh oh!

dparmar18 commented Dec 16, 2024

Uh oh!

rishabh-d-dave left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Feb 15, 2025

Uh oh!

github-actions bot commented Mar 3, 2025

Uh oh!

dparmar18 commented Mar 19, 2025

Uh oh!

dparmar18 commented Apr 23, 2025

Uh oh!

dparmar18 commented May 8, 2025

Uh oh!

dparmar18 commented May 28, 2025

Uh oh!

github-actions bot commented May 29, 2025

Uh oh!

vshankar commented Jun 3, 2025

Uh oh!

dparmar18 commented Jun 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Aug 2, 2025

Uh oh!

github-actions bot commented Oct 3, 2025

Uh oh!

github-actions bot commented Dec 2, 2025

Uh oh!

dparmar18 commented Dec 18, 2025

Uh oh!

github-actions bot commented Feb 16, 2026

Uh oh!

dparmar18 commented Feb 17, 2026

Uh oh!

Reviewers

Assignees

Labels

dparmar18 commented Jul 9, 2024 •

edited

Loading

dparmar18 Jul 18, 2024 •

edited

Loading

dparmar18 commented Jun 3, 2025 •

edited

Loading