diff --git a/Tests/LibSQL/TestSqlHeap.cpp b/Tests/LibSQL/TestSqlHeap.cpp index b2ac4de150..c47b2bda47 100644 --- a/Tests/LibSQL/TestSqlHeap.cpp +++ b/Tests/LibSQL/TestSqlHeap.cpp @@ -127,3 +127,43 @@ TEST_CASE(heap_reuse_freed_blocks_after_storage_trim) auto heap_size_after_second_storage = MUST(heap->file_size_in_bytes()); EXPECT(heap_size_after_second_storage <= original_heap_size); } + +TEST_CASE(heap_reuse_freed_blocks_after_reopening_file) +{ + ScopeGuard guard([]() { MUST(Core::System::unlink(db_path)); }); + + size_t original_heap_size = 0; + StringBuilder builder; + MUST(builder.try_append_repeated('x', SQL::Block::DATA_SIZE * 4)); + auto long_string = builder.string_view(); + + { + auto heap = create_heap(); + + // First, write storage spanning 4 blocks + auto first_index = heap->request_new_block_index(); + TRY_OR_FAIL(heap->write_storage(first_index, long_string.bytes())); + MUST(heap->flush()); + original_heap_size = MUST(heap->file_size_in_bytes()); + + // Then, overwrite the first storage and reduce it to 2 blocks + builder.clear(); + MUST(builder.try_append_repeated('x', SQL::Block::DATA_SIZE * 2)); + long_string = builder.string_view(); + TRY_OR_FAIL(heap->write_storage(first_index, long_string.bytes())); + MUST(heap->flush()); + auto heap_size_after_reduction = MUST(heap->file_size_in_bytes()); + EXPECT(heap_size_after_reduction <= original_heap_size); + } + + // Reopen the database file; we expect the heap to support reading back free blocks somehow. + // Add the second storage spanning 2 blocks - heap should not have grown compared to the original storage. + { + auto heap = create_heap(); + auto second_index = heap->request_new_block_index(); + TRY_OR_FAIL(heap->write_storage(second_index, long_string.bytes())); + MUST(heap->flush()); + auto heap_size_after_second_storage = MUST(heap->file_size_in_bytes()); + EXPECT(heap_size_after_second_storage <= original_heap_size); + } +} diff --git a/Userland/Libraries/LibSQL/Heap.cpp b/Userland/Libraries/LibSQL/Heap.cpp index 12a7c5460c..b373d78081 100644 --- a/Userland/Libraries/LibSQL/Heap.cpp +++ b/Userland/Libraries/LibSQL/Heap.cpp @@ -68,7 +68,16 @@ ErrorOr Heap::open() return open(); } - dbgln_if(SQL_DEBUG, "Heap file {} opened; number of blocks = {}", name(), m_highest_block_written); + // Perform a heap scan to find all free blocks + // FIXME: this is very inefficient; store free blocks in a persistent heap structure + for (Block::Index index = 1; index <= m_highest_block_written; ++index) { + auto block_data = TRY(read_raw_block(index)); + auto size_in_bytes = *reinterpret_cast(block_data.data()); + if (size_in_bytes == 0) + TRY(m_free_block_indices.try_append(index)); + } + + dbgln_if(SQL_DEBUG, "Heap file {} opened; number of blocks = {}; free blocks = {}", name(), m_highest_block_written, m_free_block_indices.size()); return {}; } @@ -159,8 +168,8 @@ ErrorOr Heap::read_raw_block(Block::Index index) VERIFY(m_file); VERIFY(index < m_next_block); - if (auto data = m_write_ahead_log.get(index); data.has_value()) - return data.value(); + if (auto wal_entry = m_write_ahead_log.get(index); wal_entry.has_value()) + return wal_entry.value(); TRY(m_file->seek(index * Block::SIZE, SeekMode::SetPosition)); auto buffer = TRY(ByteBuffer::create_uninitialized(Block::SIZE)); @@ -211,6 +220,7 @@ ErrorOr Heap::write_block(Block const& block) { VERIFY(block.index() < m_next_block); VERIFY(block.next_block() < m_next_block); + VERIFY(block.size_in_bytes() > 0); VERIFY(block.data().size() <= Block::DATA_SIZE); auto size_in_bytes = block.size_in_bytes();