23 Apr
2021
23 Apr
'21
9:52 a.m.
Hi, I would like to report the serious problem about data loss in customer environment.
An OSD data loss has been propagated to other OSDs. If backfill is performed when shard is
missing in a primary OSD, the shard that is corresponding to the shard in a primary OSD is
also missing in the OSD to which the backfill is directed.
In case of 4+2 erasure coding, if copies are occurred against two OSDs during one
backfill, three shards are missing(primary + two copies), making data recovery impossible.
This data loss depends on setting of erasure coding and the number of copies during
backfill.
In fact, I could reproduce this situation. This is the actual data loss, and we need to
fix this problem.
I will verify this with the latest version of ceph, and issue a ticket to redmine later
and also report detail information.
In this mail, I share simple information of environment and procedure to reproduce at
first.
Environment:
- Ceph version: Nautilus
- Erasure coding: 4+2
- Type: filestore
Step to Reproduce:
1. Setup more than 6 OSDs (with leaving some extra OSD out).
2. Store some object to pool.
3. Delete a file from a primary OSD in the PG.
(In fact, the shard on the primary OSD was unrecognized due to medium error of the
primary OSD in the customer environment. To simulate this situation, run `rm`.)
e.g.) rm -f /var/lib/ceph/osd/ceph-7/current/1.0s0_head/<some
file>.04.21.09\:55\:*
4. Cause backfill in the PG.
This time, I could occur backfill by setting OSD to `in` from `out`.
e.g.) ceph osd in osd.5
5. ceph -s show active+clean status but object is lost on both primary and backfilled
OSDs.
23 Apr
23 Apr
9:01 p.m.
Hi!
On Fri, Apr 23, 2021 at 3:53 AM <hase.jin(a)fujitsu.com> wrote:
Hi, I would like to report the serious problem about
data loss in customer environment.
An OSD data loss has been propagated to other OSDs. If backfill is performed when shard
is missing in a primary OSD, the shard that is corresponding to the shard in a primary OSD
is also missing in the OSD to which the backfill is directed.
In case of 4+2 erasure coding, if copies are occurred against two OSDs during one
backfill, three shards are missing(primary + two copies), making data recovery impossible.
This data loss depends on setting of erasure coding and the number of copies during
backfill.
In fact, I could reproduce this situation. This is the actual data loss, and we need to
fix this problem.
I will verify this with the latest version of ceph, and issue a ticket to redmine later
and also report detail information.
In this mail, I share simple information of environment and procedure to reproduce at
first.
Environment:
- Ceph version: Nautilus
- Erasure coding: 4+2
- Type: filestore
Step to Reproduce:
1. Setup more than 6 OSDs (with leaving some extra OSD out).
2. Store some object to pool.
3. Delete a file from a primary OSD in the PG.
(In fact, the shard on the primary OSD was unrecognized due to medium error of the
primary OSD in the customer environment. To simulate this situation, run `rm`.)
e.g.) rm -f /var/lib/ceph/osd/ceph-7/current/1.0s0_head/<some
file>.04.21.09\:55\:*
This could be causing two different simulated "failures":
1. A listing of objects during recovery doesn't find the object, and
thus doesn't recover it. The first case is really a problem with
filestore. If the medium error you got led to an incomplete readdir()
result from XFS, then Ceph doesn't try to cope with that.
2. The object is in the pg log and recovery tries to read it, but gets
ENOENT, and backfill (silently?) skips it. This would be a real
problem that affects bluestore as well. An error in this case should
either recover from remaining shards (if possible) or log an 'unfound'
object.
Do you know which of these was triggered by your medium error?
sage
26 Apr
26 Apr
6:36 a.m.
Thanks for your reply.
Do you know which of these was triggered by your
medium error?
This is caused by 1.
And I share additional information.
This is caused during backfilling process(not recovering process).
I confirmed that missing data was recovered by recovering.
So, the problem is data loss when backfilling occurs.
10:04 a.m.
Hi Sage,
If the medium error you got led to an incomplete
readdir()
result from XFS, then Ceph doesn't try to cope with that.
Do you mean this behavior is in Ceph specifications?
It is a problem that data loss actually occurs, so I think we need to solve that.
I would like to consider either of the following in Ceph community:
-A: Changing to Ceph specifications that does not cause data loss
-B: Establish usage or configuration to avoid data loss
What do you think about this?
4:09 p.m.
On Mon, Apr 26, 2021 at 4:04 AM <hase.jin(a)fujitsu.com> wrote:
Hi Sage,
I would frame it like this:
- With FileStore, Ceph assumed that XFS would return results we could
trust (i.e., it would not silently skip files). Trusting XFS turned
out to be a bad idea, and not just because of readdir--we also
couldn't trust that any data returned by XFS was correct since XFS
does not do any sort of data checksums.
- We replaced FileStore with BlueStore, which checksums both metadata
and data, solving this entire class of problems.
The "fix" in this case is to replace your FileStore OSDs with
BlueStore. This particular backfill corner case is just one of many
bad things that can happen with FileStore and media errors.
sage
If the medium error you got led to an incomplete
readdir()
result from XFS, then Ceph doesn't try to cope with that.
Do you mean this behavior is in Ceph specifications?
It is a problem that data loss actually occurs, so I think we need to solve that. 
6:48 p.m.
On Mon, Apr 26, 2021 at 8:09 AM Sage Weil <sage(a)newdream.net> wrote:
On Mon, Apr 26, 2021 at 4:04 AM <hase.jin(a)fujitsu.com> wrote:
I agree, bluestore handles such errors in a much better way than
filestore and we have further improvements in the pipeline like
https://trello.com/c/pWbCyYsz/614-bluestore-make-asserts-unique-per-return-…,
which will help distinguish issues with the underlying layer more
easily.
Neha
Hi Sage,
I would frame it like this:
- With FileStore, Ceph assumed that XFS would return results we could
trust (i.e., it would not silently skip files). Trusting XFS turned
out to be a bad idea, and not just because of readdir--we also
couldn't trust that any data returned by XFS was correct since XFS
does not do any sort of data checksums.
- We replaced FileStore with BlueStore, which checksums both metadata
and data, solving this entire class of problems.
The "fix" in this case is to replace your FileStore OSDs with
BlueStore. This particular backfill corner case is just one of many
bad things that can happen with FileStore and media errors. If the medium error you got led to an incomplete
readdir()
result from XFS, then Ceph doesn't try to cope with that.
Do you mean this behavior is in Ceph specifications?
It is a problem that data loss actually occurs, so I think we need to solve that.
sage
_______________________________________________
Dev mailing list -- dev(a)ceph.io
To unsubscribe send an email to dev-leave(a)ceph.io
27 Apr
27 Apr
9:30 a.m.
Hi Sage, Neha,
I agree that we should use bluestore.
(All customers will eventually migrate from filestore to bluestore.)
However, there is a problem that the migration takes time.
Currently my customer runs their system on hundreds of disks, and they cannot shut down
the system.
So they need to carefully plan their migration and migrate only a few disks in a single
migration.
Probably it will take more than a year to complete the migration of all the disks.
Therefore, I think it is necessary to consider code fixes or operational workarounds for
the problems that are detected so that data loss does not occur again by the time the
migration is complete. (I understand we should use bluestore, but I think there is still a
need to maintain filestore for users who take a long time to migrate.)
Does such an idea make sense?
For example, I would like to consider the following ideas in detail.
1: In case of operational workarounds
We can scrub manually before osd in/out. However, this is an incomplete workaround, as
it cannot be address if an automatic backfill occurs in the background.
2: In case of modifying the code.
There is no problem with `recovery`. So is it possible to adopt a part of mechanism of
`recovery` into `backfill`? (We need to investigate the code in detail.)
I'm very happy If you have some idea to solve it.
Jin
28 Apr
28 Apr
11:05 a.m.
Hi,
Let me share current idea about source code modification.
Regarding readdir(), errno is set to indicate the error, so we can check readdir() error
in Ceph codes.
[readdir() specification]
------------------------------------------------------------
https://man7.org/linux/man-pages/man3/readdir.3.html
DESCRIPTION
... It returns NULL on reaching the end of the directory stream or if an error occurred.
RETURN VALUE
... If the end of the directory stream is reached, NULL is returned
and errno is not changed. If an error occurs, NULL is returned
and errno is set to indicate the error. To distinguish end of
stream from an error, set errno to zero before calling readdir()
and then check the value of errno if NULL is returned.
------------------------------------------------------------
How about modify the codes where readdir () is being executed in filestore as follows:
------------------------------------------------------------
diff --git a/src/os/filestore/BtrfsFileStoreBackend.cc
b/src/os/filestore/BtrfsFileStoreBackend.cc
index df1d2452a1f..ad0c4d1bb1e 100644
--- a/src/os/filestore/BtrfsFileStoreBackend.cc
+++ b/src/os/filestore/BtrfsFileStoreBackend.cc
@@ -326,7 +326,13 @@ int BtrfsFileStoreBackend::list_checkpoints(list<string>&
ls)
list<string> snaps;
char path[PATH_MAX];
struct dirent *de;
- while ((de = ::readdir(dir))) {
+ while (true) {
+ errno = 0;
+ de = ::readdir(dir);
+ if (de == nullptr) {
+ ceph_assert(errno == 0);
+ break;
+ }
snprintf(path, sizeof(path), "%s/%s", get_basedir_path().c_str(),
de->d_name);
struct stat st;
diff --git a/src/os/filestore/FileStore.cc b/src/os/filestore/FileStore.cc
index f059331b090..1aeec855e9a 100644
--- a/src/os/filestore/FileStore.cc
+++ b/src/os/filestore/FileStore.cc
@@ -4987,7 +4987,13 @@ int FileStore::list_collections(vector<coll_t>& ls, bool
include_temp)
}
struct dirent *de = nullptr;
- while ((de = ::readdir(dir))) {
+ while (true) {
+ errno = 0;
+ de = ::readdir(dir);
+ if (de == nullptr) {
+ ceph_assert(errno == 0);
+ break;
+ }
if (de->d_type == DT_UNKNOWN) {
// d_type not supported (non-ext[234], btrfs), must stat
struct stat sb;
diff --git a/src/os/filestore/LFNIndex.cc b/src/os/filestore/LFNIndex.cc
index bbf65bdc66a..530d7501ddc 100644
--- a/src/os/filestore/LFNIndex.cc
+++ b/src/os/filestore/LFNIndex.cc
@@ -429,7 +429,13 @@ int LFNIndex::list_objects(const vector<string> &to_list,
int max_objs,
int r = 0;
int listed = 0;
bool end = true;
- while ((de = ::readdir(dir))) {
+ while (true) {
+ errno = 0;
+ de = ::readdir(dir);
+ if (de == nullptr) {
+ ceph_assert(errno == 0);
+ break;
+ }
end = false;
if (max_objs > 0 && listed >= max_objs) {
break;
@@ -477,7 +483,13 @@ int LFNIndex::list_subdirs(const vector<string> &to_list,
return -errno;
struct dirent *de = nullptr;
- while ((de = ::readdir(dir))) {
+ while (true) {
+ errno = 0;
+ de = ::readdir(dir);
+ if (de == nullptr) {
+ ceph_assert(errno == 0);
+ break;
+ }
string short_name(de->d_name);
string demangled_name;
if (lfn_is_subdir(short_name, &demangled_name)) {
------------------------------------------------------------
I haven't verified this modification yet, but is this idea (check readdir() errors)
acceptable?
If Ceph can't detect the error, it's hard to deal with.
But regarding readdir(), it can detect the error, so I would like to fix it.
Jin
12:39 p.m.
On Wed, Apr 28, 2021 at 5:05 AM <hase.jin(a)fujitsu.com> wrote:
Hi,
Let me share current idea about source code modification.
Regarding readdir(), errno is set to indicate the error, so we can check readdir() error
in Ceph codes.
[readdir() specification]
------------------------------------------------------------
https://man7.org/linux/man-pages/man3/readdir.3.html
DESCRIPTION
... It returns NULL on reaching the end of the directory stream or if an error occurred.
RETURN VALUE
... If the end of the directory stream is reached, NULL is returned
and errno is not changed. If an error occurs, NULL is returned
and errno is set to indicate the error. To distinguish end of
stream from an error, set errno to zero before calling readdir()
and then check the value of errno if NULL is returned.
------------------------------------------------------------
How about modify the codes where readdir () is being executed in filestore as follows:
------------------------------------------------------------
diff --git a/src/os/filestore/BtrfsFileStoreBackend.cc
b/src/os/filestore/BtrfsFileStoreBackend.cc
index df1d2452a1f..ad0c4d1bb1e 100644
--- a/src/os/filestore/BtrfsFileStoreBackend.cc
+++ b/src/os/filestore/BtrfsFileStoreBackend.cc
@@ -326,7 +326,13 @@ int BtrfsFileStoreBackend::list_checkpoints(list<string>&
ls)
list<string> snaps;
char path[PATH_MAX];
struct dirent *de;
- while ((de = ::readdir(dir))) {
+ while (true) {
+ errno = 0;
+ de = ::readdir(dir);
+ if (de == nullptr) {
+ ceph_assert(errno == 0);
+ break;
+ }
Yes! Checking the readdir result is something we should have been
doing; ignoring it is a bug. I would adjust this code to also print
the error code to the log, though.
sage
> snprintf(path, sizeof(path), "%s/%s", get_basedir_path().c_str(),
de->d_name);
>
> struct stat st;
> diff --git a/src/os/filestore/FileStore.cc b/src/os/filestore/FileStore.cc
> index f059331b090..1aeec855e9a 100644
> --- a/src/os/filestore/FileStore.cc
> +++ b/src/os/filestore/FileStore.cc
> @@ -4987,7 +4987,13 @@ int FileStore::list_collections(vector<coll_t>& ls,
bool include_temp)
> }
>
> struct dirent *de = nullptr;
> - while ((de = ::readdir(dir))) {
> + while (true) {
> + errno = 0;
> + de = ::readdir(dir);
> + if (de == nullptr) {
> + ceph_assert(errno == 0);
> + break;
> + }
> if (de->d_type == DT_UNKNOWN) {
> // d_type not supported (non-ext[234], btrfs), must stat
> struct stat sb;
> diff --git a/src/os/filestore/LFNIndex.cc b/src/os/filestore/LFNIndex.cc
> index bbf65bdc66a..530d7501ddc 100644
> --- a/src/os/filestore/LFNIndex.cc
> +++ b/src/os/filestore/LFNIndex.cc
> @@ -429,7 +429,13 @@ int LFNIndex::list_objects(const vector<string>
&to_list, int max_objs,
> int r = 0;
> int listed = 0;
> bool end = true;
> - while ((de = ::readdir(dir))) {
> + while (true) {
> + errno = 0;
> + de = ::readdir(dir);
> + if (de == nullptr) {
> + ceph_assert(errno == 0);
> + break;
> + }
> end = false;
> if (max_objs > 0 && listed >= max_objs) {
> break;
> @@ -477,7 +483,13 @@ int LFNIndex::list_subdirs(const vector<string>
&to_list,
> return -errno;
>
> struct dirent *de = nullptr;
> - while ((de = ::readdir(dir))) {
> + while (true) {
> + errno = 0;
> + de = ::readdir(dir);
> + if (de == nullptr) {
> + ceph_assert(errno == 0);
> + break;
> + }
> string short_name(de->d_name);
> string demangled_name;
> if (lfn_is_subdir(short_name, &demangled_name)) {
> ------------------------------------------------------------
>
> I haven't verified this modification yet, but is this idea (check readdir()
errors) acceptable?
> If Ceph can't detect the error, it's hard to deal with.
> But regarding readdir(), it can detect the error, so I would like to fix it.
>
> Jin
> _______________________________________________
> Dev mailing list -- dev(a)ceph.io
> To unsubscribe send an email to dev-leave(a)ceph.io
1:21 p.m.
Hi Sage,
I confirmed that master has this problem, and issued ticket to redmine.
https://tracker.ceph.com/issues/50558
Yes! Checking the readdir result is something we
should have been
doing; ignoring it is a bug. I would adjust this code to also print
the error code to the log, though.
Thank you! I also consider how fix it and continue to verify.
29 Apr
29 Apr
12:41 p.m.
Hi,
My colleague issued a PR.(https://github.com/ceph/ceph/pull/41080)
I would be very happy if you could review it.
Jin
1116
days inactive
1122
days old
10 comments
3 participants
participants (3)
-
hase.jin＠fujitsu.com -
Neha Ojha -
Sage Weil