Bugfix/537

python_tests/conftest.py

@@ -20,6 +20,9 @@ def __extract_param(request, key, default):
 
 @pytest.fixture()
 def db0_fixture(request):
+    if 'D' in db0.build_flags():
+        db0.enable_storage_validation(__extract_param(request, "storage_validation", False))
+
     if os.path.exists(DB0_DIR):
         shutil.rmtree(DB0_DIR)    
     os.mkdir(DB0_DIR)
@@ -34,12 +37,14 @@ def db0_fixture(request):
     yield db0
     gc.collect()
     db0.close()
+    if 'D' in db0.build_flags():
+        db0.enable_storage_validation(False)
     if os.path.exists(DB0_DIR):
         shutil.rmtree(DB0_DIR)
 
 
 @pytest.fixture()
-def db0_no_default_fixture():
+def db0_no_default_fixture():        
     if os.path.exists(DB0_DIR):
         shutil.rmtree(DB0_DIR)
     # create empty directory

python_tests/test_storage.py

@@ -69,3 +69,22 @@ def rand_str():
     # make sure storage overhead is < 25%
     assert dp_size_total < 1.25 * total_bytes
     assert file_size < 1.25 * total_bytes
+
+
+@pytest.mark.parametrize("db0_fixture", [{"storage_validation": True}], indirect=True)
+def test_generate_data_with_storage_validation(db0_fixture):
+    if 'D' in db0.build_flags():
+        # very small cache size to trigger lots of IO operations
+        db0.set_cache_size(128 << 10)
+        buf = db0.list()
+
+        def rand_str():
+            return ''.join(random.choice(string.ascii_letters) for i in range(1000))    
+
+        total_bytes = 0
+        for _ in range(50):
+            for _ in range(100):
+                str = rand_str()
+                total_bytes += len(str)
+                buf.append(MemoTestClass(str))
+            db0.commit()

src/dbzero/bindings/python/PyAPI.cpp

@@ -1457,6 +1457,21 @@ namespace db0::python
         // memo_obj->ext().getType().startDebug();
         Py_RETURN_NONE;
     }
+
+    PyObject *PyAPI_enableStorageValidation(PyObject *, PyObject *args, PyObject *kwargs)
+    {
+        bool enable = true;
+        static const char *kwlist[] = {"enable", NULL};
+        if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|p", const_cast<char**>(kwlist), &enable)) {            
+            return NULL;
+        }
+
+        PY_API_FUNC
+        // FIXME: log
+        std::cout << "Storage validation " << (enable ? "enabled" : "disabled") << std::endl;
+        db0::Settings::__storage_validation = enable;
+        Py_RETURN_NONE;
+    }
 #endif
 
     PyObject *PyAPI_assign(PyObject *, PyObject *args, PyObject *kwargs)

src/dbzero/bindings/python/PyAPI.hpp

@@ -182,6 +182,7 @@ namespace db0::python
     PyObject *PyAPI_crashFromCommit(PyObject *self, PyObject *const *args, Py_ssize_t nargs);    
 
     PyObject *PyAPI_breakpoint(PyObject *self, PyObject *const *args, Py_ssize_t nargs);
+    PyObject *PyAPI_enableStorageValidation(PyObject *, PyObject *args, PyObject *kwargs);
 #endif
 
     template <typename T> db0::object_model::StorageClass getStorageClass();

src/dbzero/bindings/python/dbzero.cpp

@@ -115,6 +115,7 @@ static PyMethodDef dbzero_methods[] =
     {"get_base_lock_usage", &py::getResourceLockUsage, METH_VARARGS, "Debug function, retrieves total memory occupied by ResourceLocks"},
     {"get_dram_io_map", (PyCFunction)&py::getDRAM_IOMap, METH_VARARGS | METH_KEYWORDS, "Get page_num -> state_num mapping related with a specific DRAM_Prefix"},    
     {"breakpoint", (PyCFunction)&py::PyAPI_breakpoint, METH_FASTCALL, "Testing & debugging function "},
+    {"enable_storage_validation", (PyCFunction)&py::PyAPI_enableStorageValidation, METH_VARARGS | METH_KEYWORDS, "Enable full storage validation for testing"},
 #endif
     {NULL} // Sentinel
 };

src/dbzero/core/collections/SGB_Tree/SGB_CompressedLookupTree.hpp

@@ -9,6 +9,7 @@
 #include <dbzero/core/memory/AccessOptions.hpp>
 #include <dbzero/core/compiler_attributes.hpp>
 #include <dbzero/core/metaprog/misc_utils.hpp>
+#include <dbzero/core/memory/config.hpp>
 
 namespace db0
 
@@ -93,8 +94,8 @@ DB0_PACKED_BEGIN
             // remove elements including "at"
             this->m_size -= len;
         }
-
-        template <typename op_src> void append_sorted(const o_sgb_compressed_lookup_tree_node &from, 
+        
+        template <typename op_src> void append_sorted(const o_sgb_compressed_lookup_tree_node &from,
             const_iterator begin, const_iterator end, const HeapCompT &comp)
         {
             if (this->is_reversed()) {
@@ -131,7 +132,7 @@ DB0_PACKED_BEGIN
                     *out = this_head.compress(from_head.uncompress(*begin));
                     op_src::next(begin);
                     op::next(out);
-                }
+                }                
                 this->m_size += len;
             }
         }
@@ -246,7 +247,7 @@ DB0_PACKED_END
                 return;
             }
 
-            base_t::modify().m_sgb_size++;
+            ++base_t::modify().m_sgb_size;
             // Find the node by uncompressed key / item
             auto node = base_t::lower_equal_bound(item);
             if (node == base_t::end()) {
@@ -284,7 +285,7 @@ DB0_PACKED_END
             }
 
             // node will be sorted if needed (only if in READ/WRITE mode)
-            if (this->m_access_type == AccessType::READ_WRITE) {
+            if (this->m_access_type == AccessType::READ_WRITE) {                
                 this->onNodeLookup(node);
             }
 
@@ -308,7 +309,7 @@ DB0_PACKED_END
             }
 
             // node will be sorted if needed (only if opened as READ/WRITE)
-            if (this->m_access_type == AccessType::READ_WRITE) {
+            if (this->m_access_type == AccessType::READ_WRITE) {                
                 this->onNodeLookup(node);
             }
 
@@ -340,21 +341,21 @@ DB0_PACKED_END
             }
 
             // node will be sorted if needed (only if opened as READ/WRITE)
-            if (this->m_access_type == AccessType::READ_WRITE) {
+            if (this->m_access_type == AccessType::READ_WRITE) {                
                 this->onNodeLookup(node);
             }
             // within the node look up by compressed key
             const CompressedItemT *item_ptr = nullptr;     
             if (node->header().canFit(key)) {
                 item_ptr = node->upper_equal_bound(node->header().compress(key), this->m_heap_comp);
             }
-            
+
             if (!item_ptr) {
                 // pick first item from the next node otherwise
                 ++node;                
                 if (node == base_t::end()) {
                     return std::nullopt;
-                }
+                }                
                 item_ptr = node->find_min();
                 assert(item_ptr);
             }
@@ -412,9 +413,9 @@ DB0_PACKED_END
         ItemCompT m_raw_item_comp;
 
         template <typename... Args> void insert_into(sg_tree_const_iterator &node, int recursion, const ItemT &item)
-        {
+        {            
             // Split node if full or unable to fit item
-            if (node->isFull() || ((node->size() > 2) && (recursion < 2) && !node->header().canFit(item))) {
+            if (node->isFull() || ((node->size() > 2) && (recursion < 2) && !node->header().canFit(item))) {                
                 // erase the max element and create the new node
                 auto item_ptr = node.modify().find_middle(this->m_heap_comp);
                 auto new_node = super_t::insert_equal(node->header().uncompress(*item_ptr), this->m_node_capacity, this->m_heap_comp);
@@ -424,13 +425,12 @@ DB0_PACKED_END
                 if (!this->m_raw_item_comp(item, new_node->keyItem())) {
                     insert_into(new_node, recursion + 1, item);
                     return;
-                }
+                }                
             }
 
             if (!node->header().canFit(item)) {
                 // must insert a new node to be able to fit the new item
-                auto new_node = super_t::insert_equal(item, this->m_node_capacity, this->m_heap_comp);
-                new_node->begin();                
+                super_t::insert_equal(item, this->m_node_capacity, this->m_heap_comp);                
             } else {
                 node.modify().append(this->m_heap_comp, item);
             }

src/dbzero/core/collections/SGB_Tree/SGB_LookupTree.hpp

@@ -102,13 +102,13 @@ DB0_PACKED_BEGIN
             return const_cast<ItemT*>(this->cend());
         }
 
-        const_iterator clast() const 
+        const_iterator clast() const
         {
             if (!is_reversed()) {
                 return super_t::clast();                
             }
             // reversed last
-            return super_t::cbegin() - 1;
+            return super_t::cbegin() - 1;            
         }
 
         iterator last() {
@@ -174,7 +174,7 @@ DB0_PACKED_BEGIN
         inline int step() const {
             return this->header().m_flags[LookupHeaderFlags::reversed] ? -1 : 1;
         }
-
+        
         template <typename KeyT> const_iterator lower_equal_bound(const KeyT &key, const HeapCompT &comp) const
         {
             const_iterator result = nullptr;
@@ -295,12 +295,9 @@ DB0_PACKED_BEGIN
             return this->begin() + (this->size() >> 1) * this->step();
         }
 
-        const_iterator find_min() const
-        {
-            if (is_sorted()) {
-                return this->cbegin();
-            }
-            return super_t::find_min();
+        const_iterator find_min() const {
+            // First item is always minimum, either sorted or heap-sorted
+            return this->cbegin();
         }
 
         const_iterator find_max(const HeapCompT &comp) const
@@ -429,12 +426,12 @@ DB0_PACKED_END
             }
 
             // node will be sorted if needed (only if in READ/WRITE mode)
-            if (m_access_type == AccessType::READ_WRITE) {
+            if (m_access_type == AccessType::READ_WRITE) {                
                 this->onNodeLookup(node);
             }
             return { node->lower_equal_bound(key, this->m_heap_comp), node };
         }
-
+        
         AddressT getAddress() const {
             return base_t::getAddress();
         }

src/dbzero/core/memory/DP_Lock.cpp

@@ -3,6 +3,9 @@
 #include <cstring>
 #include <cassert>
 #include <dbzero/core/storage/BaseStorage.hpp>
+#ifndef NDEBUG
+#include <dbzero/core/storage/BDevStorage.hpp>
+#endif
 
 namespace db0
 
@@ -63,24 +66,35 @@ namespace db0
                     storage.write(m_address, m_state_num, this->size(), m_data.data());
                 } else {
                     assert(flush_method == FlushMethod::diff);
-                    auto cow_ptr = getCowPtr();
+                    auto cow_ptr = getCowPtr();                    
                     if (!cow_ptr) {
-                        // unable to diff-flush
+                        // unable to diff-flush                        
                         return false;
                     }
+
                     std::vector<std::uint16_t> diffs;
                     if (!this->getDiffs(cow_ptr, diffs)) {
-                        // unable to diff-flush
+                        // unable to diff-flush (too many diffs)
                         return false;
                     }
+
                     // NOTE: DP needs not to be flushed if there are no diffs
                     if (!diffs.empty()) {
-                        storage.writeDiffs(m_address, m_state_num, this->size(), m_data.data(), diffs);
+                        if (!storage.tryWriteDiffs(m_address, m_state_num, this->size(), m_data.data(), diffs)) {
+                            // unable to diff-flush
+                            return false;
+                        }
                     }
+
+#ifndef NDEBUG                    
+                    if (Settings::__storage_validation) {
+                        // write full contents for validation
+                        storage.asFile().writeForValidation(m_address, m_state_num, this->size(), m_data.data());
+                    }
+#endif                    
                 }
 
                 m_diffs.clear();
-
                 // reset the dirty flag
                 lock.commit_reset();
             }

src/dbzero/core/memory/DirtyCache.cpp

@@ -65,7 +65,7 @@ namespace db0
         assert(m_dirty_meter_ptr);
         std::unique_lock<std::mutex> lock(m_mutex);
         std::size_t flushed = 0;
-        auto it = m_locks.begin();
+        auto it = m_locks.begin();        
         while (flushed < limit && it != m_locks.end()) {
             // only flush locks with use_count below 2 
             // i.e. - owned by the DirtyCache and possibly by the CacheRecycler

src/dbzero/core/memory/MetaAllocator.cpp

@@ -311,9 +311,11 @@ namespace db0
 
     void MetaAllocator::Realm::commit() const
     {
-        m_slab_defs.commit();
-        m_capacity_items.commit();
+        // NOTE: slab manager must commit first (important!)
+        // this is because it may perform modifications to the slab defs and capacity items
         m_slab_manager->commit();
+        m_slab_defs.commit();
+        m_capacity_items.commit();        
     }
 
     void MetaAllocator::Realm::detach() const
@@ -340,7 +342,7 @@ namespace db0
     SlabRecycler *MetaAllocator::getSlabRecyclerPtr() const {
         return m_recycler_ptr;
     }
-        
+
     void MetaAllocator::forAllSlabs(std::function<void(const SlabAllocator &, std::uint32_t)> f) const {
         m_realms.forAllSlabs(f);        
     }

src/dbzero/core/memory/ResourceLock.cpp

@@ -133,7 +133,7 @@ namespace db0
     void ResourceLock::freeze() {
         atomicCheckAndSetFlags(m_resource_flags, db0::RESOURCE_FREEZE);
     }
-
+    
     void ResourceLock::setDirty(std::uint64_t at, std::uint64_t end)
     {
         assert(at >= m_address);

src/dbzero/core/memory/SlabManager.cpp

@@ -531,7 +531,7 @@ namespace db0
             return m_realm_id;
         }
     }
-
+    
     std::optional<Address> SlabManager::tryAlloc(std::size_t size, std::uint32_t slot_num, bool aligned,
         bool unique, std::uint16_t &instance_id, unsigned char locality)
     {

src/dbzero/core/memory/WideLock.cpp

@@ -129,7 +129,12 @@ namespace db0
                             }
                             // NOTE: DP needs not to be flushed if there are no diffs
                             if (!diffs.empty()) {
-                                storage.writeDiffs(m_address + (page_ptr - m_data.data()), m_state_num, page_size, page_ptr, diffs);
+                                if (!storage.tryWriteDiffs(
+                                    m_address + (page_ptr - m_data.data()), m_state_num, page_size, page_ptr, diffs)) 
+                                {
+                                    // write as full-DP if unable to write diffs
+                                    storage.write(m_address + (page_ptr - m_data.data()), m_state_num, page_size, page_ptr);                                    
+                                }
                             }
                             first_write = false;
                         } else {

src/dbzero/core/memory/config.cpp

@@ -6,6 +6,7 @@ namespace db0
 
 #ifndef NDEBUG
     bool Settings::__dbg_logs = false;
+    bool Settings::__storage_validation = false;
 #endif 
 
 }

src/dbzero/core/memory/config.hpp

@@ -15,13 +15,15 @@ namespace db0
     // Currently we use uint32_t as the state number which allows enumerating 4B sequential transactions
     // assuming 1s per transaction that gives us 136 years of continuous operation until the state number wraps
     using StateNumType = std::uint32_t;
-        
+
     class Settings
     {
     public:
 #ifndef NDEBUG        
         static bool __dbg_logs;
+        // performs storage full read / write validation (with in-memory mirroring)
+        static bool __storage_validation;
 #endif
     };
-        
+
 }