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

nautilus: os/bluestore: list obj which equals to pend #44981

Open
wants to merge 2 commits into
base: nautilus
Choose a base branch
from

Conversation

ifed01
Copy link
Contributor

@ifed01 ifed01 commented Feb 10, 2022

Just in case - let's backport this to Nautilus...

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

backport of #43289
parent tracker: https://tracker.ceph.com/issues/52705

Signed-off-by: Igor Fedotov igor.fedotov@croit.io

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)
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

@github-actions github-actions bot added this to the nautilus milestone Feb 10, 2022
@aclamk aclamk self-requested a review February 14, 2022 09:22
@@ -30,6 +30,7 @@
#include "include/stringify.h"
#include "include/str_map.h"
#include "include/util.h"
#include "include/scope_guard.h"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: You seem to add this to a wrong commit. I would expect to see this in "os/bluestore: use scope_guard to log latency". And it is probably worth mentioning in the log message conflicts section.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: You seem to add this to a wrong commit. I would expect to see this in "os/bluestore: use scope_guard to log latency". And it is probably worth mentioning in the log message conflicts section.

yep, fixed. Thanks!

simpler this way, and avoid using `goto`.

Signed-off-by: Kefu Chai <tchaikov@gmail.com>
(cherry picked from commit 715a838)
  Conflicts:
    src/os/bluestoreBlueStore.cc
    no header loaded to define make_scoped_guard
otherwise we could have failures like

scrub : stat mismatch, got 3/4 objects, 1/2 clones, 3/4 dirty, 3/4 omap, 0/0 pinned, 0/0 hit_set_archive, 0/0 whiteouts, 49/56 bytes, 0/0 manifest objects, 0/0 hit_set_archive bytes."

where the numbers of scrubbed object, clones, dirty and omap are always
less than the total number of corresponding numbers, if the PG contains
object(s) whose hash happens to be 0xffffffff.

in this change, if the calculated hash of the upper bound is greater
than the maximum possible number represented by uint32_t, in addition to
setting the hash of the upper bound hobj to 0xffffffff, we also set the
nspace of hobj of the upper bound to "\xff", so that the upper bound
is greater than an hobj whose hash happens to be 0xfffffff. please note,
the nspace of "\xff" is not an ascii string, so it's not likely to be
less than a real-world nspace of an hobj.

with this new *greater* upper bound, we are able to include the previous
missing hobj when listing the objects in a PG. so the scrub won't be
annoyed when the number of objects does not match.

Fixes: https://tracker.ceph.com/issues/52705
Signed-off-by: Mykola Golub <mykola.golub@clyso.com>
Signed-off-by: Kefu Chai <tchaikov@gmail.com>
(cherry picked from commit ffab13b)

 Conflicts:
	src/os/bluestore/BlueStore.cc
 - trivial, different get_coll_range func signature formatting
@ifed01
Copy link
Contributor Author

ifed01 commented Feb 17, 2022

jenkins test make check

1 similar comment
@ifed01
Copy link
Contributor Author

ifed01 commented Feb 17, 2022

jenkins test make check

@ifed01
Copy link
Contributor Author

ifed01 commented Feb 18, 2022

@neha-ojha - do you think I can proceed with the merge? make check failures look unrelated and I doubt we're planning to have QA run for Nautilus...

@neha-ojha
Copy link
Member

@yuriw were you able to do a successful nautilus run, the last time you tried?

@ljflores
Copy link
Contributor

ljflores commented Mar 9, 2022

jenkins retest this please

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants