In the case where an image request is split across multiple object
extents and journaling is enabled, multiple journal events are appended.
Prior to this change, all object requests would wait for the last
journal event to complete, since journal events complete in order and
thus the last one completing implies that all prior journal events were
safe at that point.
The issue with this is that there's nothing stopping that last journal
event from being cleaned up before all object requests have stopped
referring to it. Thus, it's entirely possible for the following sequence
to occur:
1. An image request gets split into two image extents and two object
requests. Journal events are appended (one per image extent).
2. The first object request gets delayed due to an overlap, but the
second object request gets submitted and starts waiting on the last
journal event (which also causes a C_CommitIOEvent to be instantiated
against that journal event).
3. Journaling completes, and the C_CommitIOEvent fires. The
C_CommitIOEvent covers the entire range of data that was journaled in
this event, and so the event is cleaned up.
4. The first object request from above is allowed to make progress; it
tries to wait for the journal event that was just cleaned up which
causes the assert in wait_event() to fire.
As far as I can tell, this is only possible on the discard path today,
and only recently. Up until 21a26a7528
(librbd: Fix local rbd mirror journals growing forever), m_image_extents
always contained a single extent for all I/O types; this commit changed
the discard path so that if discard granularity changed the discard
request, m_image_extents would be repopulated, and if the request
happened to cross objects then there would be multiple m_image_extents.
It appears that the intent here was that there should be one journal
event per image request and the pending_extents kept track of what had
completed thus far. This commit restores that 1:1 relationship.
Fixes: https://tracker.ceph.com/issues/63422
Signed-off-by: Joshua Baergen <jbaergen@digitalocean.com>
An upcoming commits will use this to change how multi-extent image
requests are appended to the journal.
Signed-off-by: Joshua Baergen <jbaergen@digitalocean.com>
[681/1140] Building CXX object src/osd/CMakeFiles/osd.dir/scrubber/scrub_resources.cc.o
src/osd/scrubber/scrub_resources.cc: In member function ‘bool Scrub::ScrubResources::inc_scrubs_remote(pg_t)’:
src/osd/scrubber/scrub_resources.cc:84:18: warning: comparison of integer expressions of different signedness: ‘long unsigned int’ and ‘const int64_t’ {aka ‘const long int’} [-Wsign-compare]
84 | if (pre_op_cnt < conf->osd_max_scrubs) {
| ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
Signed-off-by: Casey Bodley <cbodley@redhat.com>
Update link in doc/radosgw/rgw-cache.rst. The link updated here is a
link to all the Nginx configuration files. The old link was broken. This
update comes to us from an anonymous report on
https://pad.ceph.com/p/Report_Documentation_Bugs.
Signed-off-by: Zac Dover <zac.dover@proton.me>
fix a 'multiple definition' error when included by multiple sources:
src/common/versioned_variant.h:31: multiple definition of `ceph::encode(std::monostate const&, ceph::buffer::v15_2_0::list&)';
rgw_main.cc.o:src/common/versioned_variant.h:31: first defined here
Signed-off-by: Casey Bodley <cbodley@redhat.com>
This was added to test [1]. It's duplicated by several cases in
DiffIterateTest.DiffIterateDeterministicPP now. Specifically, the
issue could be reproduced by any of:
(8) beginning of time -> snap2
(9) snap1 -> snap2
(10) beginning of time -> snap1
[1] https://tracker.ceph.com/issues/6926
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
This was added to integration test [1], separate from the fix which
went in only with unit test adjustments. It's duplicated by several
cases in DiffIterateTest.DiffIterateDeterministic now. Specifically,
the issue could be reproduced by any of:
(3) snap2 -> HEAD
(4) snap3 -> HEAD
(7) snap2 -> snap3
[1] https://tracker.ceph.com/issues/50787
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
scribble()-based DiffIterate tests are too weak: at least two
regressions that should been caught by DiffIterate.DiffIterate or
DiffIterate.DiffIterateStress were missed [1][2]. Aside from the
randomness which can be both a good and a bad thing, asserts there
ensure only that the returned diff covers all changes that were made.
If the returned diff is too excessive or otherwise bogus, this isn't
detected [3].
Add a deterministic test to systematically cover the most common cases
that don't involve discards. A similar test for discards will be added
with the fix for [4].
Comment out debug log in vector_iterate_cb() like it's done in
iterate_cb().
[1] https://tracker.ceph.com/issues/50787
[2] https://tracker.ceph.com/issues/63654
[3] https://tracker.ceph.com/issues/63719
[4] https://tracker.ceph.com/issues/53897
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
mgr/dashboard: add support for editing and deleting rgw roles
Reviewed-by: Pedro Gonzalez Gomez <pegonzal@redhat.com>
Reviewed-by: Aashish Sharma <aasharma@redhat.com>
Setting legacy_implicit_noexcept compiler directive to True will cause
Cython 3.0 to have the same semantics as Cython 0.x.
Fixes: https://tracker.ceph.com/issues/62140
Signed-off-by: Mykola Golub <mgolub@suse.com>
A basic test for ceph-nvmeof[1] where
nvmeof initiator is created.
It requires use of a new task "nvmeof_gateway_cfg"
under cephadm which shares config information
between two remote hosts.
[1] https://github.com/ceph/ceph-nvmeof/
Signed-off-by: Vallari Agrawal <val.agl002@gmail.com>
Remove a section of doc/rados/operations/stretch-mode.rst that I wrongly
re-included after its removal. The request for this (re)-removal is
here: https://github.com/ceph/ceph/pull/54689#discussion_r1413007655.
Signed-off-by: Zac Dover <zac.dover@proton.me>
Update stretch mode docs, min_size and max_size are no longer defined in
the CRUSH map and the example rule given will fail to compile.
Specify that the tiebreaker data centre cannot be defined in CRUSH as
this produces an error.
Signed-off-by: Michael Collins <perthserverplus@gmail.com>
as a non-default, non-noexcept move ctor is ignored by
stl containers.
See clang-tidy's performance-noexcept-move-constructor
Signed-off-by: Ronen Friedman <rfriedma@redhat.com>
Originally, in commit 2be4840afd ("librados/snap_set_diff: don't
assert on empty snapset"), exists was set to true. This didn't make
ObjectListSnapsRequest, causing the following deep-copy tests to fail
when run against calc_snap_set_diff() rigged to return "whole object"
as described in [1]:
TestDeepCopy.Snaps
TestDeepCopy.SnapDiscard
TestDeepCopy.CloneHideParent
TestDeepCopy.Snaps_LargerDstObjSize
TestDeepCopy.Snaps_SmallerDstObjSize
This is a regression introduced in commit cc87a8bd69 ("librbd:
deep-copy object utilizes image-extent IO methods") by way of commit
11923e234e ("librbd: generic object list snapshot request").
[1] https://github.com/ceph/ceph/pull/20648#issuecomment-369292309
Fixes: https://tracker.ceph.com/issues/63654
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
Bundling read_whole_object and LIST_SNAPS_FLAG_WHOLE_OBJECT cases
together is wrong:
- In read_whole_object case, calc_snap_set_diff() sets just
read_whole_object. Everything else is zeroed out and may require
resetting to fit with the rest of ObjectListSnapsRequest logic.
- In LIST_SNAPS_FLAG_WHOLE_OBJECT case, only the diff should be
expanded. Everything else is set by calc_snap_set_diff() and should
be used as is. This goes for end_size in particular -- if it's reset
to object size, bogus zero extents may be returned as the object
would appear to have grown.
This is a regression introduced in commit 4429ed4f3f ("librbd: switch
diff iterate API to use new snaps list dispatch methods") by way of
commit 66dd53d9c4 ("librbd: optionally return full object extent for
any snapshot deltas").
Fixes: https://tracker.ceph.com/issues/63654
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
Despite being added in commit 66dd53d9c4 ("librbd: optionally return
full object extent for any snapshot deltas") ostensibly to test the new
LIST_SNAPS_FLAG_WHOLE_OBJECT code, it surely doesn't do that because
the flag isn't even passed to MockObjectListSnapsRequest::create().
I can only guess, but it looks like snap ID 3 was intended to be
a starting point. Otherwise, with 0 and CEPH_NOSNAP passed as snap
IDs, the overlap that is set up for the clone wouldn't affect the
computation in any way.
Use snap ID 3 as a starting point and run both with and without
LIST_SNAPS_FLAG_WHOLE_OBJECT on the same snapset to pinpoint the
difference.
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
Since commit 73f50a1310 ("rbd-mirror: use generalized deep copy for
image sync"), the only user of calc_snap_set_diff() immediately unsets
end_size otherwise.
calc_snap_set_diff() semantics are clearer if end_size is set together
with end_exists and clone_end_snap_id.
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
Repair the formatting of a string that had a string inside backticks
that itself was inside double asterisks. The presence of the asterisks
around the entire string caused the backticks to appear in the rendered
documentation.
Signed-off-by: Zac Dover <zac.dover@proton.me>
as the scrub reservation changes had made it obsolete.
Note - it is not an issue of fixing the test, but rather
that the tested functionality is no longer there.
Signed-off-by: Ronen Friedman <rfriedma@redhat.com>
adds two encoding strategies for `std::variant<>` under the namespaces
`ceph::versioned_variant` and `ceph::converted_variant`
these versioned encodings allow the variant to be extended with new
types, provided that they're always added to the end without changing
or removing existing types. because of this requirement, no default
encoding is provided for `std::variant`. callers must opt in to one
namespace or the other
the `converted_variant` encoding requires the variant's first type T
to use versioned encoding, and guarantees that the variant's encoding
is backward-compatible with T's
Signed-off-by: Casey Bodley <cbodley@redhat.com>
