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

os: fix offsets for move_ranges operation #11595

Merged
merged 1 commit into from
Oct 24, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
42 changes: 21 additions & 21 deletions src/os/ObjectStore.h
Original file line number Diff line number Diff line change
Expand Up @@ -977,7 +977,7 @@ class ObjectStore {
void decode_attrset(map<string,bufferlist>& aset) {
::decode(aset, data_bl_p);
}
void decode_move_info(vector<boost::tuple<uint64_t, uint64_t, uint64_t >>& move_info) {
void decode_move_info(vector<std::pair<uint64_t, uint64_t >>& move_info) {
::decode(move_info, data_bl_p);
}
void decode_attrset_bl(bufferlist *pbl) {
Expand Down Expand Up @@ -1347,26 +1347,26 @@ class ObjectStore {
data.ops++;
}

/*
* Move source object to base object.
* Data portion is only copied from source object to base object.
* The copy is done according to the move_info vector of tuple, which
* has information of src_offset, dest_offset and length.
* Finally, the source object is deleted.
*/
void move_ranges_destroy_src(
const coll_t& cid,
const ghobject_t& src_oid,
ghobject_t oid,
const vector<boost::tuple<uint64_t, uint64_t, uint64_t>>& move_info) {
Op* _op = _get_next_op();
_op->op = OP_MERGE_DELETE;
_op->cid = _get_coll_id(cid);
_op->oid = _get_object_id(src_oid);
_op->dest_oid = _get_object_id(oid);
::encode(move_info, data_bl);
data.ops++;
}
/*
* Move source object to base object.
* Data portion is only copied from source object to base object.
* The copy is done according to the move_info vector of tuple, which
* has information of offset and length.
* Finally, the source object is deleted.
*/
void move_ranges_destroy_src(
const coll_t& cid,
const ghobject_t& src_oid,
ghobject_t oid,
const vector<std::pair<uint64_t, uint64_t>>& move_info) {
Op* _op = _get_next_op();
_op->op = OP_MERGE_DELETE;
_op->cid = _get_coll_id(cid);
_op->oid = _get_object_id(src_oid);
_op->dest_oid = _get_object_id(oid);
::encode(move_info, data_bl);
data.ops++;
}

/// Create the collection
void create_collection(const coll_t& cid, int bits) {
Expand Down
29 changes: 14 additions & 15 deletions src/os/bluestore/BlueStore.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7067,7 +7067,7 @@ void BlueStore::_txc_add_transaction(TransContext *txc, Transaction *t)
if (!no) {
no = c->get_onode(noid, true);
}
vector<boost::tuple<uint64_t, uint64_t, uint64_t>> move_info;
vector<std::pair<uint64_t, uint64_t>> move_info;
i.decode_move_info(move_info);
r = _move_ranges_destroy_src(txc, c, o, no, move_info);
}
Expand Down Expand Up @@ -8618,11 +8618,12 @@ int BlueStore::_clone_range(TransContext *txc,
/* Move contents of src object according to move_info to base object.
* Once the move_info is traversed completely, delete the src object.
*/
int BlueStore::_move_ranges_destroy_src(TransContext *txc,
int BlueStore::_move_ranges_destroy_src(
TransContext *txc,
CollectionRef& c,
OnodeRef& srco,
OnodeRef& baseo,
const vector<boost::tuple<uint64_t, uint64_t, uint64_t>> move_info)
const vector<std::pair<uint64_t, uint64_t>> move_info)
{
dout(15) << __func__ << " " << c->cid << " "
<< srco->oid << " -> " << baseo->oid
Expand All @@ -8633,18 +8634,16 @@ int BlueStore::_move_ranges_destroy_src(TransContext *txc,
// Traverse move_info completely, move contents from src object
// to base object.
for (unsigned i = 0; i < move_info.size(); ++i) {
uint64_t srcoff = move_info[i].get<0>();
uint64_t dstoff = move_info[i].get<1>();
uint64_t len = move_info[i].get<2>();

dout(15) << __func__ << " " << c->cid << " " << srco->oid << " -> "
<< baseo->oid << " from 0x" << std::hex << srcoff << "~" << len
<< " to offset 0x" << dstoff << std::dec
<< dendl;

r = _clone_range(txc, c, srco, baseo, srcoff, len, dstoff);
if (r < 0)
goto out;
uint64_t off = move_info[i].first;
uint64_t len = move_info[i].second;

dout(15) << __func__ << " " << c->cid << " " << srco->oid << " -> "
<< baseo->oid << " 0x" << std::hex << off << "~" << len
<< dendl;

r = _clone_range(txc, c, srco, baseo, off, len, off);
if (r < 0)
goto out;
}

// delete the src object
Expand Down
11 changes: 6 additions & 5 deletions src/os/bluestore/BlueStore.h
Original file line number Diff line number Diff line change
Expand Up @@ -2002,11 +2002,12 @@ class BlueStore : public ObjectStore,
OnodeRef& oldo,
OnodeRef& newo,
uint64_t srcoff, uint64_t length, uint64_t dstoff);
int _move_ranges_destroy_src(TransContext *txc,
CollectionRef& c,
OnodeRef& oldo,
OnodeRef& newo,
const vector<boost::tuple<uint64_t, uint64_t, uint64_t>> move_info);
int _move_ranges_destroy_src(
TransContext *txc,
CollectionRef& c,
OnodeRef& oldo,
OnodeRef& newo,
const vector<std::pair<uint64_t,uint64_t>> move_info);
int _rename(TransContext *txc,
CollectionRef& c,
OnodeRef& oldo,
Expand Down
28 changes: 15 additions & 13 deletions src/os/filestore/FileStore.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2707,7 +2707,7 @@ void FileStore::_do_transaction(
coll_t src_cid = i.get_cid(op->cid);
_kludge_temp_object_collection(cid, oid);
_kludge_temp_object_collection(src_cid, src_oid);
vector<boost::tuple<uint64_t, uint64_t, uint64_t>> move_info;
vector<std::pair<uint64_t, uint64_t>> move_info;
i.decode_move_info(move_info);
tracepoint(objectstore, move_ranges_destroy_src_enter, osr_name);
r = _move_ranges_destroy_src(src_cid, src_oid, cid, oid, move_info, spos);
Expand Down Expand Up @@ -3768,13 +3768,16 @@ int FileStore::_clone_range(const coll_t& oldcid, const ghobject_t& oldoid, cons
/*
* Move contents of src object according to move_info to base object. Once the move_info is traversed completely, delete the src object.
*/
int FileStore::_move_ranges_destroy_src(const coll_t& src_cid, const ghobject_t& src_oid, const coll_t& cid, const ghobject_t& oid,
const vector<boost::tuple<uint64_t, uint64_t, uint64_t>> move_info,
const SequencerPosition& spos)
int FileStore::_move_ranges_destroy_src(
const coll_t& src_cid, const ghobject_t& src_oid,
const coll_t& cid, const ghobject_t& oid,
const vector<std::pair<uint64_t, uint64_t>> move_info,
const SequencerPosition& spos)
{
int r = 0;

dout(10) << __func__ << src_cid << "/" << src_oid << " -> " << cid << "/" << oid << dendl;
dout(10) << __func__ << src_cid << "/" << src_oid << " -> "
<< cid << "/" << oid << dendl;

// check replay guard for base object. If not possible to replay, return.
int dstcmp = _check_replay_guard(cid, oid, spos);
Expand All @@ -3795,7 +3798,8 @@ int FileStore::_move_ranges_destroy_src(const coll_t& src_cid, const ghobject_t&

FDRef t;
r = lfn_open(src_cid, src_oid, false, &t);
//If we are replaying, it is possible that we do not find src obj as it is deleted before crashing.
//If we are replaying, it is possible that we do not find src obj as
//it is deleted before crashing.
if (r < 0) {
lfn_close(b);
dout(10) << __func__ << " replaying -->" << replaying << dendl;
Expand All @@ -3808,13 +3812,11 @@ int FileStore::_move_ranges_destroy_src(const coll_t& src_cid, const ghobject_t&
}

for (unsigned i = 0; i < move_info.size(); ++i) {
uint64_t srcoff = move_info[i].get<0>();
uint64_t dstoff = move_info[i].get<1>();
uint64_t len = move_info[i].get<2>();

r = _do_clone_range(**t, **b, srcoff, len, dstoff);
if (r < 0)
break;
uint64_t off = move_info[i].first;
uint64_t len = move_info[i].second;
r = _do_clone_range(**t, **b, off, len, off);
if (r < 0)
break;
}

dout(10) << __func__ << cid << "/" << oid << " " << " = " << r << dendl;
Expand Down
8 changes: 5 additions & 3 deletions src/os/filestore/FileStore.h
Original file line number Diff line number Diff line change
Expand Up @@ -585,9 +585,11 @@ class FileStore : public JournalingObjectStore,
int _clone_range(const coll_t& oldcid, const ghobject_t& oldoid, const coll_t& newcid, const ghobject_t& newoid,
uint64_t srcoff, uint64_t len, uint64_t dstoff,
const SequencerPosition& spos);
int _move_ranges_destroy_src(const coll_t& temp_cid, const ghobject_t& temp_oid, const coll_t& cid, const ghobject_t& oid,
const vector<boost::tuple<uint64_t, uint64_t, uint64_t> > move_info,
const SequencerPosition& spos);
int _move_ranges_destroy_src(
const coll_t& temp_cid, const ghobject_t& temp_oid,
const coll_t& cid, const ghobject_t& oid,
const vector<std::pair<uint64_t, uint64_t> > move_info,
const SequencerPosition& spos);
int _do_clone_range(int from, int to, uint64_t srcoff, uint64_t len, uint64_t dstoff);
int _do_sparse_copy_range(int from, int to, uint64_t srcoff, uint64_t len, uint64_t dstoff);
int _do_copy_range(int from, int to, uint64_t srcoff, uint64_t len, uint64_t dstoff, bool skip_sloppycrc=false);
Expand Down
32 changes: 14 additions & 18 deletions src/os/kstore/KStore.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2393,7 +2393,7 @@ void KStore::_txc_add_transaction(TransContext *txc, Transaction *t)
{
const ghobject_t& boid = i.get_oid(op->dest_oid);
OnodeRef bo = c->get_onode(boid, true);
vector<boost::tuple<uint64_t, uint64_t, uint64_t>> move_info;
vector<std::pair<uint64_t, uint64_t>> move_info;
i.decode_move_info(move_info);
r = _move_ranges_destroy_src(txc, c, o, cvec[op->dest_cid], bo, move_info);
}
Expand Down Expand Up @@ -3211,38 +3211,34 @@ int KStore::_clone_range(TransContext *txc,
/* Move contents of src object according to move_info to base object.
* Once the move_info is traversed completely, delete the src object.
*/
int KStore::_move_ranges_destroy_src(TransContext *txc,
CollectionRef& c,
OnodeRef& srco,
CollectionRef& basec,
OnodeRef& baseo,
vector<boost::tuple<uint64_t, uint64_t, uint64_t>> move_info)
int KStore::_move_ranges_destroy_src(
TransContext *txc,
CollectionRef& c,
OnodeRef& srco,
CollectionRef& basec,
OnodeRef& baseo,
vector<std::pair<uint64_t, uint64_t>> move_info)
{
int r = 0;
bufferlist bl;
baseo->exists = true;
_assign_nid(txc, baseo);

// Traverse move_info completely, move contents from src to base object.
// Traverse move_info completely, move contents from src to base object.
for (unsigned i = 0; i < move_info.size(); ++i) {
uint64_t srcoff;
uint64_t dstoff;
uint64_t len;

srcoff = move_info[i].get<0>();
dstoff = move_info[i].get<1>();
len = move_info[i].get<2>();
uint64_t off = move_info[i].first;
uint64_t len = move_info[i].second;

r = _do_read(srco, srcoff, len, bl, 0);
r = _do_read(srco, off, len, bl, 0);
if (r < 0)
goto out;

r = _do_write(txc, baseo, dstoff, bl.length(), bl, 0);
r = _do_write(txc, baseo, off, bl.length(), bl, 0);
txc->write_onode(baseo);
r = 0;
}

// After for loop ends, remove src obj
// After for loop ends, remove src obj
r = _do_remove(txc, srco);

out:
Expand Down
13 changes: 7 additions & 6 deletions src/os/kstore/KStore.h
Original file line number Diff line number Diff line change
Expand Up @@ -634,12 +634,13 @@ class KStore : public ObjectStore {
OnodeRef& oldo,
OnodeRef& newo,
uint64_t srcoff, uint64_t length, uint64_t dstoff);
int _move_ranges_destroy_src(TransContext *txc,
CollectionRef& c,
OnodeRef& oldo,
CollectionRef& bc,
OnodeRef& newo,
vector<boost::tuple<uint64_t, uint64_t, uint64_t>> move_info);
int _move_ranges_destroy_src(
TransContext *txc,
CollectionRef& c,
OnodeRef& oldo,
CollectionRef& bc,
OnodeRef& newo,
vector<std::pair<uint64_t, uint64_t>> move_info);
int _rename(TransContext *txc,
CollectionRef& c,
OnodeRef& oldo,
Expand Down
35 changes: 17 additions & 18 deletions src/os/memstore/MemStore.cc
Original file line number Diff line number Diff line change
Expand Up @@ -845,7 +845,7 @@ void MemStore::_do_transaction(Transaction& t)
coll_t cid = i.get_cid(op->cid);
ghobject_t oid = i.get_oid(op->oid);
ghobject_t noid = i.get_oid(op->dest_oid);
vector<boost::tuple<uint64_t, uint64_t, uint64_t>> move_info;
vector<std::pair<uint64_t, uint64_t>> move_info;
i.decode_move_info(move_info);
r = _move_ranges_destroy_src(cid, oid, noid, move_info);
}
Expand Down Expand Up @@ -1251,11 +1251,13 @@ int MemStore::_clone_range(const coll_t& cid, const ghobject_t& oldoid,
/* Move contents of src object according to move_info to base object.
* Once the move_info is traversed completely, delete the src object.
*/
int MemStore::_move_ranges_destroy_src(const coll_t& cid, const ghobject_t& srcoid,
const ghobject_t& baseoid,
const vector<boost::tuple<uint64_t, uint64_t, uint64_t> > move_info)
int MemStore::_move_ranges_destroy_src(
const coll_t& cid, const ghobject_t& srcoid,
const ghobject_t& baseoid,
const vector<std::pair<uint64_t, uint64_t> > move_info)
{
dout(10) << __func__ << " " << cid << " " << srcoid << " -> " << baseoid << dendl;
dout(10) << __func__ << " " << cid << " " << srcoid << " -> "
<< baseoid << dendl;
CollectionRef c = get_collection(cid);
if (!c)
return -ENOENT;
Expand All @@ -1266,21 +1268,18 @@ int MemStore::_move_ranges_destroy_src(const coll_t& cid, const ghobject_t& srco
ObjectRef no = c->get_or_create_object(baseoid);

for (unsigned i = 0; i < move_info.size(); ++i) {
uint64_t srcoff = move_info[i].get<0>();
uint64_t dstoff = move_info[i].get<1>();
uint64_t len = move_info[i].get<2>();

if (srcoff >= oo->get_size())
return 0;
if (srcoff + len >= oo->get_size())
len = oo->get_size() - srcoff;

const ssize_t old_size = no->get_size();
no->clone(oo.get(), srcoff, len, dstoff);
used_bytes += (no->get_size() - old_size);
uint64_t off = move_info[i].first;
uint64_t len = move_info[i].second;
if (off >= oo->get_size())
return 0;
if (off + len >= oo->get_size())
len = oo->get_size() - off;
const ssize_t old_size = no->get_size();
no->clone(oo.get(), off, len, off);
used_bytes += (no->get_size() - old_size);
}

// delete the src object
// delete the src object
_remove(cid, srcoid);
return 0;
}
Expand Down
4 changes: 2 additions & 2 deletions src/os/memstore/MemStore.h
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,8 @@ class MemStore : public ObjectStore {
const ghobject_t& newoid,
uint64_t srcoff, uint64_t len, uint64_t dstoff);
int _move_ranges_destroy_src(const coll_t& cid, const ghobject_t& oldoid,
const ghobject_t& newoid,
const vector<boost::tuple<uint64_t, uint64_t, uint64_t> > move_info);
const ghobject_t& newoid,
const vector<pair<uint64_t, uint64_t> > move_info);
int _omap_clear(const coll_t& cid, const ghobject_t &oid);
int _omap_setkeys(const coll_t& cid, const ghobject_t &oid, bufferlist& aset_bl);
int _omap_rmkeys(const coll_t& cid, const ghobject_t &oid, bufferlist& keys_bl);
Expand Down
19 changes: 11 additions & 8 deletions src/test/objectstore/DeterministicOpSequence.cc
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,9 @@ bool DeterministicOpSequence::do_move_ranges_delete_srcobj(rngen_t& gen)
boost::uniform_int<> move_len(1, bl.length());
size = (size_t) move_len(gen);

vector<boost::tuple<uint64_t, uint64_t, uint64_t>> move_info = { boost::make_tuple(0, 0, size)};
vector<std::pair<uint64_t, uint64_t>> move_info = {
make_pair(0, size)
};

dout(0) << "do_move_ranges_delete_srcobj " << coll.to_str() << "/" << orig_obj.oid.name
<< " (0~" << size << ")"
Expand Down Expand Up @@ -561,18 +563,19 @@ void DeterministicOpSequence::_do_write_and_clone_range(coll_t coll,
m_store->apply_transaction(&m_osr, std::move(t));
}

void DeterministicOpSequence::_do_write_and_merge_delete(coll_t coll,
hobject_t& orig_obj,
hobject_t& new_obj,
vector<boost::tuple<uint64_t, uint64_t, uint64_t>> move_info,
bufferlist& bl)
void DeterministicOpSequence::_do_write_and_merge_delete(
coll_t coll,
hobject_t& orig_obj,
hobject_t& new_obj,
vector<std::pair<uint64_t, uint64_t>> move_info,
bufferlist& bl)
{
ObjectStore::Transaction t;

note_txn(&t);
for (unsigned i = 0; i < move_info.size(); ++i) {
uint64_t srcoff = move_info[i].get<0>();
t.write(coll, ghobject_t(orig_obj), srcoff, bl.length(), bl);
uint64_t off = move_info[i].first;
t.write(coll, ghobject_t(orig_obj), off, bl.length(), bl);
}
t.move_ranges_destroy_src(coll, ghobject_t(orig_obj), ghobject_t(new_obj), move_info);
m_store->apply_transaction(&m_osr, std::move(t));
Expand Down