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

ceph-objectstore-tool: do not populate snapmapper with missing clones #15787

Merged
merged 3 commits into from Jun 21, 2017
Merged
Diff settings

Always

Just for now

@@ -3300,6 +3300,108 @@ void *BlueStore::MempoolThread::entry()

// =======================================================

// OmapIteratorImpl

#undef dout_prefix
#define dout_prefix *_dout << "bluestore.OmapIteratorImpl(" << this << ") "

BlueStore::OmapIteratorImpl::OmapIteratorImpl(
CollectionRef c, OnodeRef o, KeyValueDB::Iterator it)
: c(c), o(o), it(it)
{
RWLock::RLocker l(c->lock);
if (o->onode.has_omap()) {
get_omap_key(o->onode.nid, string(), &head);
get_omap_tail(o->onode.nid, &tail);
it->lower_bound(head);
}
}

int BlueStore::OmapIteratorImpl::seek_to_first()
{
RWLock::RLocker l(c->lock);
if (o->onode.has_omap()) {
it->lower_bound(head);
} else {
it = KeyValueDB::Iterator();
}
return 0;
}

int BlueStore::OmapIteratorImpl::upper_bound(const string& after)
{
RWLock::RLocker l(c->lock);
if (o->onode.has_omap()) {
string key;
get_omap_key(o->onode.nid, after, &key);
ldout(c->store->cct,20) << __func__ << " after " << after << " key "
<< pretty_binary_string(key) << dendl;
it->upper_bound(key);
} else {
it = KeyValueDB::Iterator();
}
return 0;
}

int BlueStore::OmapIteratorImpl::lower_bound(const string& to)
{
RWLock::RLocker l(c->lock);
if (o->onode.has_omap()) {
string key;
get_omap_key(o->onode.nid, to, &key);
ldout(c->store->cct,20) << __func__ << " to " << to << " key "
<< pretty_binary_string(key) << dendl;
it->lower_bound(key);
} else {
it = KeyValueDB::Iterator();
}
return 0;
}

bool BlueStore::OmapIteratorImpl::valid()
{
RWLock::RLocker l(c->lock);
bool r = o->onode.has_omap() && it && it->valid() &&
it->raw_key().second <= tail;
if (it && it->valid()) {
ldout(c->store->cct,20) << __func__ << " is at "
<< pretty_binary_string(it->raw_key().second)
<< dendl;
}
return r;
}

int BlueStore::OmapIteratorImpl::next(bool validate)
{
RWLock::RLocker l(c->lock);
if (o->onode.has_omap()) {
it->next();
return 0;
} else {
return -1;
}
}

string BlueStore::OmapIteratorImpl::key()
{
RWLock::RLocker l(c->lock);
assert(it->valid());
string db_key = it->raw_key().second;
string user_key;
decode_omap_key(db_key, &user_key);
return user_key;
}

bufferlist BlueStore::OmapIteratorImpl::value()
{
RWLock::RLocker l(c->lock);
assert(it->valid());
return it->value();
}


// =====================================

#undef dout_prefix
#define dout_prefix *_dout << "bluestore(" << path << ") "

@@ -6892,91 +6994,6 @@ int BlueStore::_collection_list(
return r;
}

// omap reads

BlueStore::OmapIteratorImpl::OmapIteratorImpl(
CollectionRef c, OnodeRef o, KeyValueDB::Iterator it)
: c(c), o(o), it(it)
{
RWLock::RLocker l(c->lock);
if (o->onode.has_omap()) {
get_omap_key(o->onode.nid, string(), &head);
get_omap_tail(o->onode.nid, &tail);
it->lower_bound(head);
}
}

int BlueStore::OmapIteratorImpl::seek_to_first()
{
RWLock::RLocker l(c->lock);
if (o->onode.has_omap()) {
it->lower_bound(head);
} else {
it = KeyValueDB::Iterator();
}
return 0;
}

int BlueStore::OmapIteratorImpl::upper_bound(const string& after)
{
RWLock::RLocker l(c->lock);
if (o->onode.has_omap()) {
string key;
get_omap_key(o->onode.nid, after, &key);
it->upper_bound(key);
} else {
it = KeyValueDB::Iterator();
}
return 0;
}

int BlueStore::OmapIteratorImpl::lower_bound(const string& to)
{
RWLock::RLocker l(c->lock);
if (o->onode.has_omap()) {
string key;
get_omap_key(o->onode.nid, to, &key);
it->lower_bound(key);
} else {
it = KeyValueDB::Iterator();
}
return 0;
}

bool BlueStore::OmapIteratorImpl::valid()
{
RWLock::RLocker l(c->lock);
return o->onode.has_omap() && it && it->valid() && it->raw_key().second <= tail;
}

int BlueStore::OmapIteratorImpl::next(bool validate)
{
RWLock::RLocker l(c->lock);
if (o->onode.has_omap()) {
it->next();
return 0;
} else {
return -1;
}
}

string BlueStore::OmapIteratorImpl::key()
{
RWLock::RLocker l(c->lock);
assert(it->valid());
string db_key = it->raw_key().second;
string user_key;
decode_omap_key(db_key, &user_key);
return user_key;
}

bufferlist BlueStore::OmapIteratorImpl::value()
{
RWLock::RLocker l(c->lock);
assert(it->valid());
return it->value();
}

int BlueStore::omap_get(
const coll_t& cid, ///< [in] Collection containing oid
const ghobject_t &oid, ///< [in] Object containing omap
@@ -166,6 +166,11 @@ void SnapMapper::clear_snaps(
assert(check(oid));
set<string> to_remove;
to_remove.insert(to_object_key(oid));
if (g_conf->subsys.should_gather(ceph_subsys_osd, 20)) {
for (auto& i : to_remove) {
dout(20) << __func__ << " rm " << i << dendl;
}
}
backend.remove_keys(to_remove, t);
}

@@ -180,6 +185,11 @@ void SnapMapper::set_snaps(
::encode(in, bl);
to_set[to_object_key(oid)] = bl;
dout(20) << __func__ << " " << oid << " " << in.snaps << dendl;
if (g_conf->subsys.should_gather(ceph_subsys_osd, 20)) {
for (auto& i : to_set) {
dout(20) << __func__ << " set " << i.first << dendl;
}
}
backend.set_keys(to_set, t);
}

@@ -214,6 +224,11 @@ int SnapMapper::update_snaps(
to_remove.insert(to_raw_key(make_pair(*i, oid)));
}
}
if (g_conf->subsys.should_gather(ceph_subsys_osd, 20)) {
for (auto& i : to_remove) {
dout(20) << __func__ << " rm " << i << dendl;
}
}
backend.remove_keys(to_remove, t);
return 0;
}
@@ -240,6 +255,11 @@ void SnapMapper::add_oid(
++i) {
to_add.insert(to_raw(make_pair(*i, oid)));
}
if (g_conf->subsys.should_gather(ceph_subsys_osd, 20)) {

This comment has been minimized.

Copy link
@gregsfortytwo

gregsfortytwo Jun 20, 2017

Member

Aside: I think in the past you've done this exclusion with some macro shenanigans, enclosing the whole loop in a dout/dendl pair. This format here is perfectly (perhaps more) readable, but did you switch for any particular reason?

for (auto& i : to_add) {
dout(20) << __func__ << " set " << i.first << dendl;
}
}
backend.set_keys(to_add, t);
}

@@ -259,6 +279,8 @@ int SnapMapper::get_next_objects_to_trim(
while (out->size() < max) {
pair<string, bufferlist> next;
r = backend.get_next(pos, &next);
dout(20) << __func__ << " get_next(" << pos << ") returns " << r
<< " " << next << dendl;
if (r != 0) {
break; // Done
}
@@ -270,6 +292,7 @@ int SnapMapper::get_next_objects_to_trim(

assert(is_mapping(next.first));

dout(20) << __func__ << " " << next.first << dendl;
pair<snapid_t, hobject_t> next_decoded(from_raw(next));
assert(next_decoded.first == snap);
assert(check(next_decoded.second));
@@ -313,6 +336,11 @@ int SnapMapper::_remove_oid(
++i) {
to_remove.insert(to_raw_key(make_pair(*i, oid)));
}
if (g_conf->subsys.should_gather(ceph_subsys_osd, 20)) {
for (auto& i : to_remove) {
dout(20) << __func__ << " rm " << i << dendl;
}
}
backend.remove_keys(to_remove, t);
return 0;
}
@@ -861,14 +861,24 @@ int get_attrs(
cout << "snapset " << snapset << std::endl;
if (!snapset.is_legacy()) {
for (auto& p : snapset.clone_snaps) {
hobject_t clone = hoid.hobj;
clone.snap = p.first;
ghobject_t clone = hoid;
clone.hobj.snap = p.first;
set<snapid_t> snaps(p.second.begin(), p.second.end());
if (!store->exists(coll, clone)) {

This comment has been minimized.

Copy link
@dzafman

dzafman Jun 21, 2017

Member

You could clean-up last_clones since it is not used. last_clones is cleared when we import the head object which would be the point where it could have been useful. It is better to check the store since you might import into a pg that already has existing objects.

This comment has been minimized.

Copy link
@dzafman

dzafman Jun 21, 2017

Member

You can remove last_head too. Just replace "hold == last_head" test with hold.is_head().

This comment has been minimized.

Copy link
@dzafman

dzafman Jun 21, 2017

Member

241f8ad has the change you can cherry-pick here.

This comment has been minimized.

Copy link
@liewegas

liewegas Jun 21, 2017

Author Member

Let's put this in a separate pr, since this one already went through testing and passed?

// no clone, skip. this is probably a cache pool. this works
// because we use a separate transaction per object and clones
// come before head in the archive.
if (debug)
cerr << "\tskipping missing " << clone << " (snaps "
<< snaps << ")" << std::endl;
continue;
}
if (debug)
cerr << "\tsetting " << clone << " snaps " << snaps << std::endl;
cerr << "\tsetting " << clone.hobj << " snaps " << snaps
<< std::endl;
OSDriver::OSTransaction _t(driver.get_transaction(t));
assert(!snaps.empty());
snap_mapper.add_oid(clone, snaps, &_t);
snap_mapper.add_oid(clone.hobj, snaps, &_t);
}
}
} else {
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.