Experimental: move prev_count to the map itself

src/blockstore/blockstore_heap.cpp

@@ -191,14 +191,11 @@ uint32_t heap_entry_t::calc_crc32c()
 {
     auto old_crc32c = crc32c;
     auto old_prev_pos = prev_pos;
-    auto old_prev_count = prev_count;
     crc32c = 0;
     prev_pos = 0;
-    prev_count = 0;
     uint32_t res = ::crc32c(0, (uint8_t*)this, size);
     crc32c = old_crc32c;
     prev_pos = old_prev_pos;
-    prev_count = old_prev_count;
     return res;
 }
 
@@ -326,18 +323,18 @@ int blockstore_heap_t::load_blocks(uint64_t disk_offset, uint64_t size, uint8_t
         }
         entries_loaded++;
         auto & inode_idx = block_index[get_pg_id(wr->inode, wr->stripe)][wr->inode];
-        auto obj_it = inode_idx.find(wr->stripe);
+        auto & idx = inode_idx[wr->stripe];
         const uint64_t wr_pos = (uint8_t*)wr - buf + disk_offset + 1;
-        if (obj_it == inode_idx.end())
+        idx.refcnt++;
+        if (!idx.pos)
         {
-            inode_idx[wr->stripe] = wr_pos;
+            idx.pos = wr_pos;
         }
         else
         {
-            auto & idx = inode_idx[wr->stripe];
+            auto prev_wr = (idx.pos - idx.pos % dsk->meta_block_size) == disk_offset
-            auto prev_wr = (idx - idx % dsk->meta_block_size) == disk_offset
+                ? (heap_entry_t*)(buf + (idx.pos % dsk->meta_block_size) - 1)
-                ? (heap_entry_t*)(buf + (idx % dsk->meta_block_size) - 1)
+                : entry_from_pos(idx.pos, true);
-                : entry_from_pos(idx, true);
             if (!prev_wr || prev_wr->is_before(wr))
             {
                 if (wr->is_overwrite())
@@ -356,10 +353,10 @@ int blockstore_heap_t::load_blocks(uint64_t disk_offset, uint64_t size, uint8_t
                 }
                 else
                 {
-                    wr->prev_pos = idx;
+                    wr->prev_pos = idx.pos;
                 }
                 // Insert <wr> on top
-                idx = wr_pos;
+                idx.pos = wr_pos;
             }
             else
             {
@@ -426,7 +423,7 @@ void blockstore_heap_t::fill_recheck_queue()
         {
             for (auto & op: ip.second)
             {
-                auto wr = entry_from_pos(op.second);
+                auto wr = entry_from_pos(op.second.pos);
                 bool prev_intent = false;
                 while (wr)
                 {
@@ -451,7 +448,7 @@ void blockstore_heap_t::mark_used_blocks()
             for (auto & op: ip.second)
             {
                 bool added = false;
-                auto wr = entry_from_pos(op.second);
+                auto wr = entry_from_pos(op.second.pos);
                 while (wr)
                 {
                     if (wr->type() == BS_HEAP_SMALL_WRITE)
@@ -485,7 +482,7 @@ void blockstore_heap_t::recheck_buffer(heap_entry_t *cwr, uint8_t *buf)
     {
         // write entry is invalid, erase it and all newer entries
         auto & inode_idx = block_index[get_pg_id(cwr->inode, cwr->stripe)][cwr->inode];
-        auto wr_pos = inode_idx[cwr->stripe];
+        auto wr_pos = inode_idx[cwr->stripe].pos;
         auto wr = entry_from_pos(wr_pos);
         int rolled_back = 0;
         auto free_entry = [&]()
@@ -509,11 +506,12 @@ void blockstore_heap_t::recheck_buffer(heap_entry_t *cwr, uint8_t *buf)
             free_entry();
         }
         assert(wr == cwr);
+        // FIXME refcnt
         if (wr->prev_pos)
         {
             fprintf(stderr, "Notice: %u unfinished writes to %jx:%jx v%jx since lsn %ju, rolling back\n",
                 rolled_back+1, wr->inode, wr->stripe, prev(wr)->version, prev(wr)->lsn);
-            inode_idx[wr->stripe] = wr->prev_pos;
+            inode_idx[wr->stripe].pos = wr->prev_pos;
         }
         else
         {
@@ -833,8 +831,8 @@ heap_entry_t *blockstore_heap_t::read_entry(object_id oid)
     {
         return NULL;
     }
-    heap_entry_t *obj = entry_from_pos(stripe_it->second);
+    heap_entry_t *obj = entry_from_pos(stripe_it->second.pos);
-    assert(obj->inode == oid.inode && obj->stripe == oid.stripe);
+    assert(!obj || obj->inode == oid.inode && obj->stripe == oid.stripe);
     return obj;
 }
 
@@ -907,16 +905,16 @@ void blockstore_heap_t::defragment_block(uint32_t block_num)
             continue;
         }
         auto & idx = block_index[get_pg_id(oid.inode, oid.stripe)][oid.inode][oid.stripe];
-        auto new_it = remap.find(idx);
+        auto new_it = remap.find(idx.pos);
         heap_entry_t *wr = NULL;
         if (new_it != remap.end())
         {
-            idx = new_it->second.new_pos;
+            idx.pos = new_it->second.new_pos;
             wr = (heap_entry_t*)(new_data + (new_it->second.new_pos % dsk->meta_block_size) - 1); // like entry_from_pos
             new_it->second.new_pos = 0;
         }
         else
-            wr = entry_from_pos(idx);
+            wr = entry_from_pos(idx.pos);
         while (wr)
         {
             assert(!(wr->prev_pos & GARBAGE_BIT));
@@ -1137,10 +1135,9 @@ int blockstore_heap_t::add_entry(uint32_t wr_size, heap_entry_t *old_head, uint3
         (new_wr->is_overwrite() ? HEAP_INFLIGHT_COMPACTED : 0) |
         (new_wr->is_compactable() ? HEAP_INFLIGHT_COMPACTABLE : 0));
     const uint64_t new_pos = entry_pos(block_num, offset);
-    if (old_head && defragmented)
+    auto & idx = block_index[get_pg_id(oid.inode, oid.stripe)][oid.inode][oid.stripe];
-    {
+    idx.refcnt++;
-        old_head = read_entry(oid);
+    old_head = entry_from_pos(idx.pos);
-    }
     if (old_head && !old_head->is_before(new_wr))
     {
         // BIG_WRITE may be inserted into the middle of the sequence during compaction
@@ -1149,7 +1146,6 @@ int blockstore_heap_t::add_entry(uint32_t wr_size, heap_entry_t *old_head, uint3
         auto next_wr = old_head;
         while (true)
         {
-            next_wr->prev_count++;
             auto nn = prev(next_wr);
             if (!nn || nn->is_before(new_wr))
                 break;
@@ -1157,31 +1153,16 @@ int blockstore_heap_t::add_entry(uint32_t wr_size, heap_entry_t *old_head, uint3
         }
         auto prev_wr = prev(next_wr);
         // <prev_wr> may be an identical big_write entry when we "punch holes" in the bitmap
-        assert(prev_wr->type() != BS_HEAP_DELETE &&
+        assert(prev_wr && prev_wr->type() != BS_HEAP_DELETE &&
             (prev_wr->type() != BS_HEAP_BIG_WRITE || prev_wr->version == new_wr->version));
         // Insert <new_wr> between <next_wr> and <prev_wr>
         new_wr->prev_pos = next_wr->prev_pos;
-        new_wr->prev_count = prev_wr->prev_count + 1;
         next_wr->prev_pos = new_pos;
     }
     else
     {
-        auto & idx = block_index[get_pg_id(oid.inode, oid.stripe)][oid.inode][oid.stripe];
+        new_wr->prev_pos = idx.pos;
-        new_wr->prev_pos = idx;
+        idx.pos = new_pos;
-        new_wr->prev_count = (old_head ? old_head->prev_count+1 : 0);
-        if (old_head)
-            new_wr->prev_count = old_head->prev_count+1;
-        else
-        {
-            auto del_it = deref_deletes.find(oid);
-            if (del_it != deref_deletes.end())
-            {
-                // An inflight garbage-collected delete entry is still potentially on disk, reflect it
-                new_wr->prev_count = 1;
-                deref_deletes.erase(del_it);
-            }
-        }
-        idx = new_pos;
     }
     new_wr->size = wr_size;
     new_wr->crc32c = new_wr->calc_crc32c();
@@ -1379,7 +1360,8 @@ int blockstore_heap_t::add_punch_holes(heap_entry_t *obj, uint64_t to_lsn, uint6
         return ENOENT;
     }
     auto & idx = block_index[get_pg_id(obj->inode, obj->stripe)][obj->inode][obj->stripe];
-    uint32_t block_num = idx / dsk->meta_block_size;
+    assert(idx.pos);
+    uint32_t block_num = idx.pos / dsk->meta_block_size;
     auto & inf = block_info.at(block_num);
     if (inf.is_writing)
     {
@@ -1805,7 +1787,7 @@ int blockstore_heap_t::list_objects(uint32_t pg_num, object_id min_oid, object_i
             {
                 continue;
             }
-            heap_entry_t *obj = entry_from_pos(stripe_pair.second);
+            heap_entry_t *obj = entry_from_pos(stripe_pair.second.pos);
             assert(obj->inode == oid.inode && obj->stripe == oid.stripe);
             uint64_t stable_version = 0;
             auto first_wr = obj;
@@ -2056,29 +2038,29 @@ void blockstore_heap_t::apply_inflight()
     }
     else if (inflight.flags & HEAP_INFLIGHT_GC)
     {
-        // Remove 1 prev_count from the object refcount in the DB or in deref_deletes
+        // Decrement the object's refcount
         auto & oid = inflight.oid;
         auto & inode_idx = block_index[get_pg_id(oid.inode, oid.stripe)][oid.inode];
         auto idx_it = inode_idx.find(oid.stripe);
-        if (idx_it == inode_idx.end())
+        assert(idx_it != inode_idx.end());
+        auto & idx = idx_it->second;
+        idx.refcnt--;
+        if (idx.refcnt == 1)
         {
-            // delete is dereferenced
+            heap_entry_t *obj = entry_from_pos(idx.pos);
-            int del = deref_deletes.erase(oid);
+            if (obj->entry_type == (BS_HEAP_DELETE|BS_HEAP_STABLE))
-            assert(del > 0);
-        }
-        else
-        {
-            heap_entry_t *newer_obj = entry_from_pos(idx_it->second);
-            newer_obj->prev_count--;
-            if (newer_obj->prev_count == 0 && newer_obj->entry_type == (BS_HEAP_DELETE|BS_HEAP_STABLE))
             {
-                // free BS_HEAP_DELETEs when their prev_count becomes 0
+                // free BS_HEAP_DELETEs when their refcount becomes 1
-                // but remember that they have a 'temporary' dereferenced prev_count of 1
+                //free_entry(idx.pos / dsk->meta_block_size, obj);
-                mark_garbage(idx_it->second / dsk->meta_block_size, newer_obj, UINT32_MAX);
+                mark_garbage(idx.pos / dsk->meta_block_size, obj, UINT32_MAX);
-                deref_deletes.insert(oid);
+                idx.pos = 0;
-                inode_idx.erase(idx_it);
+                //deref_deletes.insert(oid);
             }
         }
+        else if (!idx.refcnt)
+        {
+            inode_idx.erase(idx_it);
+        }
     }
     inflight_lsn.pop_front();
     first_inflight_lsn++;

src/blockstore/blockstore_heap.h

@@ -44,8 +44,7 @@ struct __attribute__((__packed__)) heap_entry_t
     uint64_t inode;
     uint64_t stripe;
     uint64_t version;
-    uint64_t prev_pos;   // ALWAYS invalid on disk and skipped in checksum calculation
+    uint64_t prev_pos; // ALWAYS invalid on disk and skipped in checksum calculation
-    uint32_t prev_count; // ALWAYS invalid on disk and skipped in checksum calculation
 
     // uint8_t[] external_bitmap
     // uint8_t[] internal_bitmap
@@ -108,21 +107,21 @@ struct heap_inflight_lsn_t
     uint64_t compact_lsn;
 };
 
-struct heap_deref_prev_t
-{
-    uint32_t block_num;
-    object_id oid;
-};
-
 struct heap_compact_t
 {
     uint64_t compact_lsn, compact_version;
     uint64_t clean_lsn, clean_version, clean_loc;
 };
 
+struct heap_object_ptr_t
+{
+    uint64_t pos;
+    uint32_t refcnt;
+};
+
 using i64hash_t = robin_hood::hash<uint64_t>;
 using heap_block_index_t = robin_hood::unordered_flat_map<uint64_t,
-    robin_hood::unordered_flat_map<inode_t, robin_hood::unordered_flat_map<uint64_t, uint64_t, i64hash_t, std::equal_to<uint64_t>, 88>, i64hash_t>, i64hash_t>;
+    robin_hood::unordered_flat_map<inode_t, robin_hood::unordered_flat_map<uint64_t, heap_object_ptr_t, i64hash_t, std::equal_to<uint64_t>, 88>, i64hash_t>, i64hash_t>;
 using heap_mvcc_map_t = robin_hood::unordered_flat_map<object_id, heap_object_mvcc_t>;
 
 class blockstore_heap_t
@@ -160,8 +159,6 @@ class blockstore_heap_t
     uint64_t completed_lsn = 0;
     uint64_t fsynced_lsn = 0;
     std::deque<object_id> compact_queue;
-    std::vector<heap_deref_prev_t> deref_prev;
-    robin_hood::unordered_flat_set<object_id> deref_deletes;
 
     bool marked_used_blocks = false;
     bool recheck_queue_filled = false;

src/test/test_heap.cpp

@@ -245,7 +245,6 @@ void test_delete(bool csum)
         assert(obj);
         assert(count_writes(heap, obj) == 1);
         assert(obj->entry_type == (BS_HEAP_DELETE|BS_HEAP_STABLE));
-        assert(obj->prev_count == 1);
 
         assert(space.at(INODE_WITH_POOL(1, 1)) == 0x20000);
         assert(heap.get_data_used_space() == 0x20000);
@@ -257,7 +256,6 @@ void test_delete(bool csum)
         assert(obj);
         assert(count_writes(heap, obj) == 1);
         assert(obj->entry_type == (BS_HEAP_BIG_WRITE|BS_HEAP_STABLE));
-        assert(obj->prev_count == 2);
 
         // Delete it again...
         res = heap.add_delete(obj, &mblock);
@@ -268,7 +266,6 @@ void test_delete(bool csum)
 
         obj = heap.read_entry(oid);
         assert(obj);
-        assert(obj->prev_count == 3);
 
         // Now the trickiest part - check that the delete entry itself disappears
         // when all previous entries disappear from the disk too. It happens only
@@ -299,21 +296,50 @@ void test_defrag_block()
 
     uint32_t big_write_size = heap.get_big_entry_size();
     uint32_t small_write_size = heap.get_small_entry_size(0, 4096);
-    assert(big_write_size == 192);
+    assert(big_write_size == 188);
-    assert(small_write_size == 76);
+    assert(small_write_size == 72);
-    uint32_t nwr = dsk.meta_block_size/(big_write_size+small_write_size);
+    uint32_t nwr = 0;
+    bool add = false;
+    if ((dsk.meta_block_size % (big_write_size+small_write_size)) >= big_write_size)
+    {
+        nwr = (dsk.meta_block_size / (big_write_size+small_write_size)) +
+            (dsk.meta_block_size-small_write_size) / (big_write_size+small_write_size);
+        add = (dsk.meta_block_size - small_write_size -
+            (dsk.meta_block_size-small_write_size) % (big_write_size+small_write_size)) >= big_write_size;
+    }
+    else
+        nwr = dsk.meta_block_size/(big_write_size+small_write_size);
 
     {
-        for (uint32_t i = 0; i < nwr*2-1; i++)
+        uint32_t used = 0;
+        uint32_t expected_block = 0;
+        for (uint32_t i = 0; i < nwr; i++)
         {
-            _test_big_write(heap, dsk, 1, i*0x20000, 1, i*0x20000, true, 0, 0, buffer_area.data(), (i < nwr ? 0 : 1));
+            _test_big_write(heap, dsk, 1, i*0x20000, 1, i*0x20000, true, 0, 0, buffer_area.data(), expected_block);
-            _test_small_write(heap, dsk, 1, i*0x20000, 2, 0, 4096, i*4096, true, buffer_area.data()+i*4096, false, (i < nwr ? 0 : 1));
+            used += big_write_size;
+            if (dsk.meta_block_size-used < small_write_size)
+            {
+                used = 0;
+                expected_block++;
+            }
+            _test_small_write(heap, dsk, 1, i*0x20000, 2, 0, 4096, i*4096, true, buffer_area.data()+i*4096, false, expected_block);
+            used += small_write_size;
+            if (dsk.meta_block_size-used < big_write_size)
+            {
+                used = 0;
+                expected_block++;
+            }
+        }
+        if (add)
+        {
+            _test_big_write(heap, dsk, 1, nwr*0x20000, 1, nwr*0x20000, true, 0, 0, buffer_area.data(), 1);
+            used += big_write_size;
         }
         // The next write should be rejected because allowing it would block compaction
-        assert(_test_do_big_write(heap, dsk, 1, (nwr*2)*0x20000, 1, (nwr*2)*0x20000, true, 0, 0, buffer_area.data()) == ENOSPC);
+        assert(_test_do_big_write(heap, dsk, 1, (nwr+1)*0x20000, 1, (nwr+1)*0x20000, true, 0, 0, buffer_area.data()) == ENOSPC);
         // Compact all small writes
         uint32_t mblock = 999999;
-        for (uint32_t i = 0; i < nwr*2-1; i++)
+        for (uint32_t i = 0; i < nwr; i++)
         {
             auto obj = heap.read_entry((object_id){ .inode = INODE_WITH_POOL(1, 1), .stripe = i*0x20000 });
             assert(obj);
@@ -1047,7 +1073,7 @@ void test_full_alloc()
     std::map<std::string, std::string> config;
     config["data_csum_type"] = "crc32c";
     dsk.parse_config(config);
-    dsk.data_device_size = 8*1024*1024;
+    dsk.data_device_size = 64*1024*1024;
     dsk.meta_device_size = 5*4096;
     dsk.journal_device_size = 4*1024*1024;
     dsk.data_device = "data";
@@ -1062,28 +1088,19 @@ void test_full_alloc()
 
     uint32_t big_write_size = heap.get_big_entry_size();
     uint32_t small_write_size = heap.get_small_entry_size(0, 4096);
-    assert(big_write_size == 192);
+    assert(big_write_size == 188);
-    assert(small_write_size == 76);
+    assert(small_write_size == 72);
-    uint32_t b_4s = (big_write_size + 4*small_write_size);
+    uint32_t epb = dsk.meta_block_size/big_write_size;
-    uint32_t epb = dsk.meta_block_size/b_4s;
     for (int j = 0; j < 4; j++)
     {
         assert(heap.get_meta_nearfull_blocks() == j);
-        for (int i = j*epb; i < j*epb+epb; i++)
+        for (int i = j*epb; i < j*epb+epb-(j == 3); i++)
         {
             _test_big_write(heap, dsk, 1, i*0x20000, 1, i*0x20000, true, 0, 0, buffer_area.data(), j);
-            _test_small_write(heap, dsk, 1, i*0x20000, 2, 8192, 4096, i*16384, true, buffer_area.data(), false, j);
+            assert(heap.get_meta_block_used_space(0) == (i < epb ? i+1 : epb)*big_write_size);
-            _test_small_write(heap, dsk, 1, i*0x20000, 3, 8192, 4096, i*16384+4096, true, buffer_area.data(), false, j);
+            assert(heap.get_meta_block_used_space(1) == (i < epb ? 0 : (i < 2*epb ? i+1-epb : epb)*big_write_size));
-            _test_small_write(heap, dsk, 1, i*0x20000, 4, 8192, 4096, i*16384+2*4096, true, buffer_area.data(), false, j);
+            assert(heap.get_meta_block_used_space(2) == (i < 2*epb ? 0 : (i < 3*epb ? i+1-2*epb : epb)*big_write_size));
-            if (i < 4*epb-1)
+            assert(heap.get_meta_block_used_space(3) == (i < 3*epb ? 0 : (i < 4*epb ? i+1-3*epb : epb)*big_write_size));
-            {
-                // Don't write the last entry
-                _test_small_write(heap, dsk, 1, i*0x20000, 5, 8192, 4096, i*16384+3*4096, true, buffer_area.data(), false, j);
-            }
-            assert(heap.get_meta_block_used_space(0) == (i < epb ? i+1 : epb)*b_4s);
-            assert(heap.get_meta_block_used_space(1) == (i < epb ? 0 : (i < 2*epb ? i+1-epb : epb)*b_4s));
-            assert(heap.get_meta_block_used_space(2) == (i < 2*epb ? 0 : (i < 3*epb ? i+1-2*epb : epb)*b_4s));
-            assert(heap.get_meta_block_used_space(3) == (i < 3*epb ? 0 : (i < 4*epb ? i+1-3*epb : epb)*b_4s) - (i < 4*epb-1 ? 0 : small_write_size));
         }
     }
 
@@ -1091,12 +1108,12 @@ void test_full_alloc()
     assert(ENOSPC == _test_do_big_write(heap, dsk, 1, epb*4*0x20000, 1, epb*4*0x20000, true, 0, 0, buffer_area.data(), 0));
 
     // We can still do some more overwrites into 3 of 4 nearfull blocks
-    int rest_fit = (dsk.meta_block_size-b_4s*(dsk.meta_block_size/b_4s))/small_write_size * 3;
+    int rest_fit = (dsk.meta_block_size % big_write_size)/small_write_size * 4;
     for (int i = 0; i < rest_fit; i++)
     {
-        _test_small_write(heap, dsk, 1, (4*epb-1)*0x20000, 5+i, 8192, 4096, (4*epb-1)*16384+3*4096+i*4096, true, buffer_area.data(), false, UINT32_MAX /*any block*/);
+        _test_small_write(heap, dsk, 1, 1*0x20000, 5+i, 8192, 4096, (4*epb-1)*16384+3*4096+i*4096, true, buffer_area.data(), false, UINT32_MAX /*any block*/);
     }
-    assert(ENOSPC == _test_do_small_write(heap, dsk, 1, (4*epb-1)*0x20000, 5+rest_fit, 8192, 4096, (4*epb-1)*16384+3*4096+rest_fit*4096, true, buffer_area.data(), false, 0));
+    assert(ENOSPC == _test_do_small_write(heap, dsk, 1, 1*0x20000, 5+rest_fit, 8192, 4096, (4*epb-1)*16384+3*4096+rest_fit*4096, true, buffer_area.data(), false, 0));
 
     printf("OK test_full_alloc\n");
 }