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

add scrub persist/query API #6898

Merged
merged 15 commits into from Feb 25, 2016
Merged

add scrub persist/query API #6898

merged 15 commits into from Feb 25, 2016

Conversation

@@ -12233,7 +12292,7 @@ void ReplicatedPG::_scrub(
bufferlist bv;
bv.push_back(p->second.attrs[OI_ATTR]);
try {
oi = object_info_t(); // Initialize optional<> before decode into it
oi = object_info_t(); // Initialize optional<> before decode into it
Copy link
Contributor Author

Choose a reason for hiding this comment

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

remove this unintended space only change.

@athanatos
Copy link
Contributor

Some comments, getting close!

@tchaikov
Copy link
Contributor Author

i am preparing a new pull request implementing the get_inconsistent_snapsets().

@tchaikov tchaikov force-pushed the wip-13505 branch 2 times, most recently from ddd3d59 to fb37a0c Compare December 22, 2015 08:44
@dzafman
Copy link
Contributor

dzafman commented Dec 22, 2015

@tchaikov It is unfortunate that an object_info_t here has the same name as the internal OSD struct. As the user visible one is really a complete identifier of an object maybe object_id_t or object_identifier_t would be better.

@tchaikov
Copy link
Contributor Author

@dzafman thanks. will go with object_id_t.

@tchaikov tchaikov mentioned this pull request Dec 28, 2015
@tchaikov tchaikov force-pushed the wip-13505 branch 5 times, most recently from bfd27ff to e32d61d Compare January 13, 2016 13:10
@tchaikov
Copy link
Contributor Author

  • use object_id_t instead of object_info_t
  • fixed the handling of missing object in ObjectError::encode().

@tchaikov
Copy link
Contributor Author

the work-in-progress test suite is posted at ceph/ceph-qa-suite#795 for proof-reading

@@ -3994,6 +4009,16 @@ void PG::chunky_scrub(ThreadPool::TPHandle &handle)
scrubber.reserved_peers.clear();
}

{
hobject_t oid = Scrub::make_scrub_object(info.pgid.pgid);
Copy link
Contributor

Choose a reason for hiding this comment

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

There might be two shards on the same OSD for the same PG. The scrub object should probably be built using the spg_t rather than the pg_t.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will do. and as will not encode the pg into the object name, as temp coll is per pg.

@tchaikov
Copy link
Contributor Author

changes

  • in PG::chunky_scrub(): cleanup before creating a new scrub object.

@tchaikov
Copy link
Contributor Author

the test suite: ceph/ceph-qa-suite#795

{
return ("SCRUB_OBJ_" +
std::to_string(pool) + "." +
oid.name + oid.nspace + std::to_string(oid.snap));
Copy link
Contributor

Choose a reason for hiding this comment

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

This is problematic because you could have two objects in the same pg:

  1. name: foo namespace: bar
  2. name: foob namespace: ar
    We need a seperator. I think there are other places in the code where object names are used to generate keys, how about using hobject_t::to_str()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will do.

@athanatos
Copy link
Contributor

I just set up a vstart cluster to play with it a bit. One thing I noticed is that after a repair, the list-inconsistent-obj api still returns the old incorrect values. I think the simplest solution would be for us to not populate the scrub store during a repair?

@tchaikov
Copy link
Contributor Author

I think the simplest solution would be for us to not populate the scrub store during a repair?

will do.

@tchaikov
Copy link
Contributor Author

changelog

  • osd: use hobject_t::to_str() to build the keys of scrub results.
  • osd: do not populate scrub result when repairing
  • tools/rados: pretty print the snap field. i.e. print "head", "snapdir", or hex of snapid
  • librados: expose rados_inconsistent_pg_list as a C API.
  • pybind: expose Rados.get_inconsistent_pgs
  • rebased against master

@tchaikov tchaikov force-pushed the wip-13505 branch 2 times, most recently from d36129e to e3e23ca Compare February 24, 2016 15:08
tchaikov and others added 15 commits February 25, 2016 12:40
Signed-off-by: Kefu Chai <kchai@redhat.com>
to list the inconsistent PGs of given pool, it's a wrapper
around the "ceph pg ls" command.

Fixes: ceph#13505
Signed-off-by: Kefu Chai <kchai@redhat.com>
Signed-off-by: Kefu Chai <kchai@redhat.com>
to list inconsistent PGs of a given pool. this command exposes
the underlying get_inconsistent_pgs() API to user.

Signed-off-by: Kefu Chai <kchai@redhat.com>
which present the inconsistent objects found in scrub

Fixes: ceph#13505
Signed-off-by: Kefu Chai <kchai@redhat.com>
Signed-off-by: Kefu Chai <kchai@redhat.com>
persist inconsistent objects found when comparing the ScrubMaps
collected from replica/shards. the discrepancies between the auth
copy and the replica are identified as inconsistencies. and hence
encoded into the omap of an object of the temp coll of the PG in
question.
scrub_types.{h,cpp} are introduced to hide the details of how we
persist the scrub types from the librados client.

Signed-off-by: Kefu Chai <kchai@redhat.com>
Signed-off-by: Samuel Just <sjust@redhat.com>
for presenting the inconsistent snapsets found in scrub

Signed-off-by: Kefu Chai <kchai@redhat.com>
the inconsistent snapsets are identified in ReplicatedPG::_scrub()
after we compared the authorized objects with their replica/shards.
these inconsistent information are stored in the omap of objects
with prefix "SCRUB_SS_".

Signed-off-by: Kefu Chai <kchai@redhat.com>
it is a new pg op which returns the encoded objects stored when
scrubbing.

Fixes: ceph#13505
Signed-off-by: Kefu Chai <kchai@redhat.com>
Fixes: ceph#13505
Signed-off-by: Kefu Chai <kchai@redhat.com>
Fixes: ceph#13505
Signed-off-by: Kefu Chai <kchai@redhat.com>
to list inconsistent objects of a given PG, this command exposes
get_inconsistent_objects() rados API to user.

Signed-off-by: Kefu Chai <kchai@redhat.com>
to list inconsistent snapsets of a given PG, this command exposes
get_inconsistent_snapsets() rados API to user.

Fixes: ceph#13505
Signed-off-by: Kefu Chai <kchai@redhat.com>
tchaikov added a commit that referenced this pull request Feb 25, 2016
add scrub persist/query API

Reviewed-by: Samuel Just <sjust@redhat.com>
@tchaikov tchaikov merged commit ce1169d into ceph:master Feb 25, 2016
@tchaikov tchaikov deleted the wip-13505 branch February 25, 2016 04:48
@tchaikov tchaikov added this to the jewel milestone Mar 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants