From f1c4448fc549ed58bb58f920dfa16886e4910315 Mon Sep 17 00:00:00 2001 From: Igor Fedotov Date: Fri, 6 Aug 2021 21:32:23 +0300 Subject: [PATCH] os/bluestore: use non-inclusive comparision against prefer_deferred_size Fixes: https://tracker.ceph.com/issues/52089 Signed-off-by: Igor Fedotov --- src/os/bluestore/BlueStore.cc | 8 +- src/test/objectstore/store_test.cc | 131 ++++++++++++++++++++++++++++- 2 files changed, 132 insertions(+), 7 deletions(-) diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc index d2ef187689d..20bc17b2449 100644 --- a/src/os/bluestore/BlueStore.cc +++ b/src/os/bluestore/BlueStore.cc @@ -13467,7 +13467,7 @@ void BlueStore::_do_write_small( wctx->buffered ? 0 : Buffer::FLAG_NOCACHE); if (!g_conf()->bluestore_debug_omit_block_device_write) { - if (b_len <= prefer_deferred_size) { + if (b_len < prefer_deferred_size) { dout(20) << __func__ << " deferring small 0x" << std::hex << b_len << std::dec << " unused write via deferred" << dendl; bluestore_deferred_op_t *op = _get_deferred_op(txc); @@ -13735,7 +13735,7 @@ bool BlueStore::BigDeferredWriteContext::can_defer( ceph_assert(b_off % chunk_size == 0); ceph_assert(blob_aligned_len() % chunk_size == 0); - res = blob_aligned_len() <= prefer_deferred_size && + res = blob_aligned_len() < prefer_deferred_size && blob_aligned_len() <= ondisk && blob.is_allocated(b_off, blob_aligned_len()); if (res) { @@ -13937,7 +13937,7 @@ void BlueStore::_do_write_big( << std::dec << dendl; offset += l; length -= l; - logger->inc(l_bluestore_write_big_blobs, remaining ? 2 : 1); + logger->inc(l_bluestore_write_big_blobs, remaining ? 2 : 1); logger->inc(l_bluestore_write_big_deferred, remaining ? 2 : 1); continue; } @@ -14305,7 +14305,7 @@ int BlueStore::_do_alloc_write( // queue io if (!g_conf()->bluestore_debug_omit_block_device_write) { - if (l->length() <= prefer_deferred_size.load()) { + if (l->length() < prefer_deferred_size.load()) { dout(20) << __func__ << " deferring 0x" << std::hex << l->length() << std::dec << " write via deferred" << dendl; bluestore_deferred_op_t *op = _get_deferred_op(txc); diff --git a/src/test/objectstore/store_test.cc b/src/test/objectstore/store_test.cc index c77929b0c22..5ecc953d136 100644 --- a/src/test/objectstore/store_test.cc +++ b/src/test/objectstore/store_test.cc @@ -7421,6 +7421,128 @@ TEST_P(StoreTestSpecificAUSize, DeferredOnBigOverwrite) { } } +TEST_P(StoreTestSpecificAUSize, DeferredOnBigOverwrite2) { + + if (string(GetParam()) != "bluestore") + return; + + size_t block_size = 4096; + StartDeferred(block_size); + SetVal(g_conf(), "bluestore_max_blob_size", "65536"); + SetVal(g_conf(), "bluestore_prefer_deferred_size", "65536"); + + g_conf().apply_changes(nullptr); + + int r; + coll_t cid; + ghobject_t hoid(hobject_t("test", "", CEPH_NOSNAP, 0, -1, "")); + + PerfCounters* logger = const_cast(store->get_perf_counters()); + + auto ch = store->create_new_collection(cid); + { + ObjectStore::Transaction t; + t.create_collection(cid, 0); + r = queue_transaction(store, ch, std::move(t)); + ASSERT_EQ(r, 0); + } + { + ObjectStore::Transaction t; + bufferlist bl; + + bl.append(std::string(128 * 1024, 'c')); + + t.write(cid, hoid, 0x1000, bl.length(), bl, CEPH_OSD_OP_FLAG_FADVISE_NOCACHE); + r = queue_transaction(store, ch, std::move(t)); + ASSERT_EQ(r, 0); + ASSERT_EQ(logger->get(l_bluestore_write_big), 1u); + ASSERT_EQ(logger->get(l_bluestore_write_big_bytes), bl.length()); + ASSERT_EQ(logger->get(l_bluestore_write_big_blobs), 2u); + ASSERT_EQ(logger->get(l_bluestore_write_big_deferred), 0u); + ASSERT_EQ(logger->get(l_bluestore_write_deferred), 0u); + } + + logger->reset(); + { + ObjectStore::Transaction t; + bufferlist bl; + + bl.append(std::string(128 * 1024, 'c')); + + t.write(cid, hoid, 0x2000, bl.length(), bl, CEPH_OSD_OP_FLAG_FADVISE_NOCACHE); + r = queue_transaction(store, ch, std::move(t)); + ASSERT_EQ(r, 0); + + ASSERT_EQ(logger->get(l_bluestore_write_big), 1u); + ASSERT_EQ(logger->get(l_bluestore_write_big_bytes), bl.length()); + ASSERT_EQ(logger->get(l_bluestore_write_big_blobs), 4u); + ASSERT_EQ(logger->get(l_bluestore_write_big_deferred), 2u); + ASSERT_EQ(logger->get(l_bluestore_write_deferred), 2u); + } + + { + ObjectStore::Transaction t; + t.remove(cid, hoid); + t.remove(cid, hoid); + t.remove_collection(cid); + cerr << "Cleaning" << std::endl; + r = queue_transaction(store, ch, std::move(t)); + ASSERT_EQ(r, 0); + } +} + +TEST_P(StoreTestSpecificAUSize, DeferredOnBigOverwrite3) { + + if (string(GetParam()) != "bluestore") + return; + + size_t block_size = 4096; + StartDeferred(block_size); + SetVal(g_conf(), "bluestore_max_blob_size", "65536"); + SetVal(g_conf(), "bluestore_prefer_deferred_size", "65536"); + + g_conf().apply_changes(nullptr); + + int r; + coll_t cid; + ghobject_t hoid(hobject_t("test", "", CEPH_NOSNAP, 0, -1, "")); + + PerfCounters* logger = const_cast(store->get_perf_counters()); + + auto ch = store->create_new_collection(cid); + { + ObjectStore::Transaction t; + t.create_collection(cid, 0); + r = queue_transaction(store, ch, std::move(t)); + ASSERT_EQ(r, 0); + } + + logger->reset(); + { + ObjectStore::Transaction t; + bufferlist bl; + + bl.append(std::string(4096 * 1024, 'c')); + + t.write(cid, hoid, 0, bl.length(), bl, CEPH_OSD_OP_FLAG_FADVISE_NOCACHE); + r = queue_transaction(store, ch, std::move(t)); + ASSERT_EQ(r, 0); + + ASSERT_EQ(logger->get(l_bluestore_write_big), 1u); + ASSERT_EQ(logger->get(l_bluestore_write_big_bytes), bl.length()); + ASSERT_EQ(logger->get(l_bluestore_write_big_blobs), 64u); + ASSERT_EQ(logger->get(l_bluestore_write_big_deferred), 0u); + ASSERT_EQ(logger->get(l_bluestore_write_deferred), 0u); + } + { + ObjectStore::Transaction t; + t.remove(cid, hoid); + t.remove_collection(cid); + cerr << "Cleaning" << std::endl; + r = queue_transaction(store, ch, std::move(t)); + ASSERT_EQ(r, 0); + } +} TEST_P(StoreTestSpecificAUSize, DeferredDifferentChunks) { @@ -7429,9 +7551,11 @@ TEST_P(StoreTestSpecificAUSize, DeferredDifferentChunks) { size_t alloc_size = 4096; size_t large_object_size = 1 * 1024 * 1024; + size_t prefer_deferred_size = 65536; StartDeferred(alloc_size); SetVal(g_conf(), "bluestore_max_blob_size", "131072"); - SetVal(g_conf(), "bluestore_prefer_deferred_size", "65536"); + SetVal(g_conf(), "bluestore_prefer_deferred_size", + stringify(prefer_deferred_size).c_str()); g_conf().apply_changes(nullptr); int r; @@ -7447,7 +7571,7 @@ TEST_P(StoreTestSpecificAUSize, DeferredDifferentChunks) { r = queue_transaction(store, ch, std::move(t)); ASSERT_EQ(r, 0); } - for (size_t expected_write_size = 1024; expected_write_size <= 65536; expected_write_size *= 2) { + for (size_t expected_write_size = 1024; expected_write_size <= prefer_deferred_size; expected_write_size *= 2) { //create object with hint ghobject_t hoid(hobject_t("test-"+to_string(expected_write_size), "", CEPH_NOSNAP, 0, -1, "")); { @@ -7483,7 +7607,8 @@ TEST_P(StoreTestSpecificAUSize, DeferredDifferentChunks) { CEPH_OSD_OP_FLAG_FADVISE_NOCACHE); r = queue_transaction(store, ch, std::move(t)); ++exp_bluestore_write_big; - ++exp_bluestore_write_big_deferred; + if (expected_write_size != prefer_deferred_size) + ++exp_bluestore_write_big_deferred; ASSERT_EQ(r, 0); } ASSERT_EQ(logger->get(l_bluestore_write_big), exp_bluestore_write_big); -- 2.39.5