Xiubo Li [Mon, 27 Nov 2023 07:55:42 +0000 (15:55 +0800)]
mds: set the loner to true for LOCK_EXCL_XSYN
For filelock when in LOCK_EXCL_XSYN state the non-loner clients
should be issued empty caps, but since the loner of this state
is set to false and it could make the Locker to issue the Fcb caps
to them, which is incorrect.
Ilya Dryomov [Fri, 1 Dec 2023 17:29:12 +0000 (18:29 +0100)]
test/librbd: drop DiffIterateTest.DiffIterateRegression6926
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
Ilya Dryomov [Fri, 1 Dec 2023 17:54:19 +0000 (18:54 +0100)]
test/librbd: drop TestLibRBD.SnapDiff
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
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().
Ilya Dryomov [Mon, 27 Nov 2023 10:59:26 +0000 (11:59 +0100)]
librbd: fix read_whole_object handling in ObjectListSnapsRequest
Originally, in commit 2be4840afd4f ("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]:
This is a regression introduced in commit cc87a8bd697e ("librbd:
deep-copy object utilizes image-extent IO methods") by way of commit 11923e234efc ("librbd: generic object list snapshot request").
Ilya Dryomov [Mon, 27 Nov 2023 09:11:52 +0000 (10:11 +0100)]
librbd: fix LIST_SNAPS_FLAG_WHOLE_OBJECT behavior
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 4429ed4f3f4c ("librbd: switch
diff iterate API to use new snaps list dispatch methods") by way of
commit 66dd53d9c4d9 ("librbd: optionally return full object extent for
any snapshot deltas").
Ilya Dryomov [Sun, 19 Nov 2023 21:44:28 +0000 (22:44 +0100)]
test/librbd: make ListSnapsWholeObject actually test stuff
Despite being added in commit 66dd53d9c4d9 ("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.
Ilya Dryomov [Sat, 11 Nov 2023 13:15:49 +0000 (14:15 +0100)]
librados/snap_set_diff: set end_size only if end object exists
Since commit 73f50a13109f ("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.
Ilya Dryomov [Sat, 9 Dec 2023 20:00:42 +0000 (21:00 +0100)]
test/librbd: avoid config-related crashes in DiscardWithPruneWriteOverlap
For reasons that I think no longer apply today, set_val() and
set_val_or_die() refuse to set "type: str" config options that aren't
marked as "can be changed at runtime" -- set_val() returns an error and
set_val_or_die() terminates the process. What is and isn't marked as
"can be changed at runtime" seems to be pretty much random both within
and outside of RBD, so let's just refactor how config is set here.
While at it, I realized that reproducer config is underspecified:
- for rbd_cache_policy and rbd_cache_writethrough_until_flush settings
to matter, rbd_cache must be set to true and rbd_cache_max_dirty must
be set to a positive number
- order should be set explicitly, because rbd_default_order can be as
low as 12 (for 4096-byte objects), interfering with the logic of the
test
Yuri Weinstein [Thu, 7 Dec 2023 16:30:34 +0000 (08:30 -0800)]
Merge pull request #54771 from ajarr/wip-63714-pacific
pacific: qa/workunits/rbd/cli_generic.sh: narrow race window when checking that rbd_support module command fails after blocklisting the module's client
Yuval Lifshitz [Wed, 23 Feb 2022 15:21:10 +0000 (17:21 +0200)]
rgw: prevent spurious/lost notifications in the index completion thread
this was happening when asyn completions happened during reshard.
more information about testing:
https://gist.github.com/yuvalif/d526c0a3a4c5b245b9e951a6c5a10517
we also add more logs to the completion manager.
should allow finding unhandled completions due to reshards.
Joshua Baergen [Thu, 9 Nov 2023 16:43:22 +0000 (09:43 -0700)]
librbd: Append one journal event per image request
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 21a26a752843295ff946d1543c2f5f9fac764593
(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.
Just in case a CInode's nlink is 1, and then a unlink request comes
and then early replies and submits to the MDLogs, but just before
the MDlogs are flushed a link request comes, and the link request
also succeeds and early replies to client.
Later when the unlink/link requests' MDLog events are flushed and
the callbacks are called, which will fire a stray denty reintegration.
But it will pick the new dentry, which is from the link's request
and is a remote dentry, to do the reintegration. While in the
'rename' code when traversing the path it will trigger to call the
'dn->link_remote()', which later will fire a new stray dentry
reintegration.
The problem is if the first 'rename' request is retried several
times, and in each time it will fire a new reintegration, which
makes no sense and maybe blocked for a very long time dues to some
reasons and then will be reported as slow request warning.
... when checking whether a rbd_support module command fails after
blocklisting the module's client.
In tests that check the recovery of the rbd_support module after its
client is blocklisted, the rbd_support module's client is
blocklisted using the `osd blocklist add` command. Next,
`osd blocklist ls` command is issued to confirm that the client is
blocklisted. A rbd_support module command is then issued and expected
to fail in order to verify that the blocklisting has affected the
rbd_support module's operations. Sometimes it was observed that before
this rbd_support module command reached the ceph-mgr, the rbd_support
module detected the blocklisting, recovered from it, and was able to
serve the command. To reduce the race window that occurs when trying to
verify that the rbd_support module's operation is affected by client
blocklisting, get rid of the `osd blocklist ls` command.
Patrick Donnelly [Fri, 15 Jul 2022 20:39:00 +0000 (16:39 -0400)]
mds: ensure next replay is queued on req drop
Not all client replay requests are queued at once since [1]. We require
the next request by queued when completed (unsafely) or during cleanup.
Not all code paths seem to handle this [2] so move it to a generic
location, MDCache::request_cleanup. Even so, this doesn't handle all
errors (so we must still be careful) as sometimes we must queue the next
replay request before an MDRequest is constructed [3] during some error
conditions.
Additionally, preserve the behavior of Server::journal_and_reply
queueing the next replay op. Otherwise, must wait for the request to be
durable before moving onto the next one, unnecessarily.
For reproducing, two specific cases are highlighted (thanks to @Mer1997 on
Github for locating these):
- The request is killed by a session close / eviction while a replayed request
is queued and waiting for a journal flush (e.g. dirty inest locks).
- The request construction fails because the request is already in the
active_requests. This could happen theoretically if a client resends the same
request (same reqid) twice.
The first case is most probable but very difficult to reproduce for testing
purposes. The replayed op would need to wait on a journal flush (to be
restarted by C_MDS_RetryRequest). Then, the request would need killed by a
session close.
Alice Zhao [Mon, 11 Apr 2022 12:35:57 +0000 (08:35 -0400)]
osd: don't require RWEXCL lock for stat+write ops.
In librbd, a stat op is inserted before write op for cloned image. OSD used to use RWEXCL and such requests are processed one by one.
With this fix, OSD will use RWWRITE rather than RWEXCL for such [stat,write] request to allow multiple [stat,write] on the same object and improve performance.
Aashish Sharma [Wed, 4 Oct 2023 06:54:13 +0000 (12:24 +0530)]
mgr/dashboard: Consider null values as zero in grafana panels
After upgrading from RHCS4 to RHCS5..some of the grafana charts broke.
This is because in RHCS5 we do not generate the metrics if its value is
zero as a result the null value from that metric breaks the grafana
charts or graphs. This PR is to fix the above mentioned issue.
projects / ceph.git / log