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

quincy: test/bufferlist: ensure rebuild_aligned_size_and_memory() always rebuilds. #44891

Merged
merged 2 commits into from Feb 3, 2022

Conversation

cbodley
Copy link
Contributor

@cbodley cbodley commented Feb 3, 2022

backport tracker: https://tracker.ceph.com/issues/53974

clean cherry-pick required additional commit from #44675

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

…y() actually happens.

For the investigation of failures like the following one:

```
[ RUN      ] BufferList.rebuild_aligned_size_and_memory
../src/test/bufferlist.cc:1865: Failure
Expected equality of these values:
  bl.get_num_buffers()
    Which is: 2
  1
[  FAILED  ] BufferList.rebuild_aligned_size_and_memory (0 ms)
```

The test case assumes the rebuild before the failed clause **always**
happens while `bufferlist::rebuild_aligned_size_and_memory()` skips it
if buffers are already aligned.

Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
(cherry picked from commit 2de5f17)
…ilds.

Before the patch the test case was showing an unreliable behaviour
dependent on the underlying memory allocator. It was because
the bufferlist rebuild can be skipped, resulting in unchanged number
of buffers, if all of them begin at aligned addresses.

The commit fixes that by allocating a 4 KiB-aligned buffer and
offsetting it by a small constant (42) to ensure the memory added
to the bufferlist begins at non-4 KiB address.

Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
(cherry picked from commit 88176ac)
@cbodley
Copy link
Contributor Author

cbodley commented Feb 3, 2022

@neha-ojha @rzarzynski would like to see this merge asap, as the bufferlist failure is blocking merge of other quincy PRs

@cbodley
Copy link
Contributor Author

cbodley commented Feb 3, 2022

p.s. thanks for the fix Radek <3

@rzarzynski
Copy link
Contributor

Thanks for the backport, Casey! If there are not objections, I will merge it today, just after the big green button becomes available. It's a unittest fix, so I doubt we need a teuthology run.

@neha-ojha
Copy link
Member

Thanks for the backport, Casey! If there are not objections, I will merge it today, just after the big green button becomes available. It's a unittest fix, so I doubt we need a teuthology run.

go for it

@idryomov idryomov merged commit 0b568ae into ceph:quincy Feb 3, 2022
@cbodley cbodley deleted the wip-53974 branch February 7, 2022 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants