From fe2ed4e9b095ec67a43fe0a625d484bbcce826a2 Mon Sep 17 00:00:00 2001 From: Igor Fedotov Date: Tue, 17 May 2016 18:22:15 +0300 Subject: [PATCH] os/bluestore: Fixes some issues when using Buffer Cache from _do_read and improves test coverage Signed-off-by: Igor Fedotov --- src/os/bluestore/BlueStore.cc | 13 +++++-- src/os/bluestore/BlueStore.h | 2 +- src/test/objectstore/store_test.cc | 58 +++++++++++++++++++++++++----- 3 files changed, 60 insertions(+), 13 deletions(-) diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc index 41b804d69063e..c9f2047cb5fad 100644 --- a/src/os/bluestore/BlueStore.cc +++ b/src/os/bluestore/BlueStore.cc @@ -2804,6 +2804,7 @@ int BlueStore::_do_read( ready_regions_t ready_regions_in_cache, ready_regions; interval_set ready_intervals_in_cache; o->bc.read(off, length, ready_regions_in_cache, ready_intervals_in_cache); + dout(20) << __func__ << " regions in cache " << ready_regions_in_cache.size() << " " << ready_intervals_in_cache << dendl; //build blob list to read blobs2read_t blobs2read; @@ -2887,12 +2888,13 @@ int BlueStore::_do_read( auto rr0_end = ready_regions_in_cache.end(); off = offset; - while (rr_it != rr_end || rr0_it != rr0_end) { + while ((rr_it != rr_end || rr0_it != rr0_end) && off < offset + length) { ready_regions_t::iterator it; if (rr_it != rr_end && (rr0_it == rr0_end || rr_it->first < rr0_it->first)) { - uint64_t r_off = 0; uint64_t r_len = rr_it->second.length(); + dout(30) << __func__ << " read region " << rr_it->first << "~" << rr_it->second.length() << + " off " << off << " " << dendl; if (off > rr_it->first + r_len) { ++rr_it; continue; @@ -2921,17 +2923,22 @@ int BlueStore::_do_read( bl.claim_append(tmp); } off = rr_it->first + r_off + r_len; + dout(30) << __func__ << " used read region " << rr_it->first + r_off << + "~" << r_len << " new off " << off << dendl; ++rr_it; } else if(rr0_it != rr0_end) { if (off < rr0_it->first) bl.append_zero(rr0_it->first + off); - bl.claim_append(rr0_it->second); + dout(30) << __func__ << " used from cached buffer " << + rr0_it->first << "~" << rr0_it->second.length() << dendl; off = rr0_it->first + rr0_it->second.length(); + bl.claim_append(rr0_it->second); ++rr0_it; } } assert(offset + length >= off); bl.append_zero(offset + length - off); + assert(bl.length() == length); r = bl.length(); return r; diff --git a/src/os/bluestore/BlueStore.h b/src/os/bluestore/BlueStore.h index 56e3eb971068c..425ba97e31459 100644 --- a/src/os/bluestore/BlueStore.h +++ b/src/os/bluestore/BlueStore.h @@ -270,7 +270,7 @@ public: } } - void read(uint64_t offset, uint64_t length, BlueStore::ready_regions_t& res, interval_set res_intervals) { + void read(uint64_t offset, uint64_t length, BlueStore::ready_regions_t& res, interval_set& res_intervals) { res.clear(); auto i = _data_lower_bound(offset); uint64_t end = offset + length; diff --git a/src/test/objectstore/store_test.cc b/src/test/objectstore/store_test.cc index 855c8ac3b0cdf..8d9a1a5bb8123 100644 --- a/src/test/objectstore/store_test.cc +++ b/src/test/objectstore/store_test.cc @@ -572,16 +572,27 @@ TEST_P(StoreTest, BufferCacheReadTest) { r = apply_transaction(store, &osr, std::move(t)); ASSERT_EQ(r, 0); - newdata.clear(); - r = store->read(cid, hoid, 0, 5, newdata); - ASSERT_EQ(r, 5); - ASSERT_TRUE(newdata.contents_equal(bl)); + r = store->read(cid, hoid, 0, 15, newdata); + ASSERT_EQ(r, 15); + { + bufferlist expected; + expected.append(bl); + expected.append_zero(5); + expected.append(bl); + ASSERT_TRUE(newdata.contents_equal(expected)); + } + } + //overwrite over the same extents + { + ObjectStore::Transaction t; + bufferlist bl, newdata; + bl.append("edbca"); + t.write(cid, hoid, 0, 5, bl); + t.write(cid, hoid, 10, 5, bl); + cerr << "TwinWrite" << std::endl; + r = apply_transaction(store, &osr, std::move(t)); + ASSERT_EQ(r, 0); - newdata.clear(); - r = store->read(cid, hoid, 10, 15, newdata); - ASSERT_EQ(r, 5); - ASSERT_TRUE(newdata.contents_equal(bl)); - newdata.clear(); r = store->read(cid, hoid, 0, 15, newdata); ASSERT_EQ(r, 15); { @@ -592,6 +603,35 @@ TEST_P(StoreTest, BufferCacheReadTest) { ASSERT_TRUE(newdata.contents_equal(expected)); } } + + //additional write to an unused region of some blob and partial owerite over existing extents + { + ObjectStore::Transaction t; + bufferlist bl, bl2, bl3, newdata; + bl.append("DCB"); + bl2.append("1234567890"); + bl3.append("CB"); + + t.write(cid, hoid, 20, bl2.length(), bl2); + t.write(cid, hoid, 1, bl.length(), bl); + t.write(cid, hoid, 13, bl3.length(), bl3); + cerr << "TripleWrite" << std::endl; + r = apply_transaction(store, &osr, std::move(t)); + ASSERT_EQ(r, 0); + + r = store->read(cid, hoid, 0, 30, newdata); + ASSERT_EQ(r, 15); + { + bufferlist expected; + expected.append("eDCBa"); + expected.append_zero(5); + expected.append("edCBa"); + expected.append_zero(5); + expected.append(bl2); + + ASSERT_TRUE(newdata.contents_equal(expected)); + } + } } TEST_P(StoreTest, SimpleObjectTest) { -- 2.39.5