guycipher / k4

High-performance open-source, durable, transactional embedded storage engine designed for low-latency, and optimized read and write efficiency.
https://pkg.go.dev/github.com/guycipher/k4
BSD 3-Clause "New" or "Revised" License
155 stars 4 forks source link

Issue with flushing with bloomfilter v1.5.0b #7

Closed guycipher closed 1 day ago

guycipher commented 1 day ago

It seems from version 1.4.0-1.5.0 there is a bug with the bloomfilter and writing and reading from pager I believe. It's hard to say currently.
For the test below

func TestMemtableFlush(t *testing.T) {
    dir := setup(t)
    defer teardown(dir)

    k4, err := Open(dir, 2764/2, 60, false, false)
    if err != nil {
        t.Fatalf("Failed to open K4: %v", err)
    }

    for i := 0; i < 100; i++ {
        key := []byte("key" + fmt.Sprintf("%d", i))
        value := []byte("value" + fmt.Sprintf("%d", i))

        err = k4.Put(key, value, nil)
        if err != nil {
            k4.Close()
            t.Fatalf("Failed to put key-value: %v", err)
        }
    }

    k4.Close()

    k4, err = Open(dir, 1024*1024, 2, false, false)
    if err != nil {
        t.Fatalf("Failed to reopen K4: %v", err)
    }
    defer k4.Close()

    // get all keys
    for i := 0; i < 100; i++ {
        key := []byte("key" + fmt.Sprintf("%d", i))
        value := []byte("value" + fmt.Sprintf("%d", i))

        got, err := k4.Get(key)
        if err != nil {
            t.Fatalf("Failed to get key: %v", err)
        }

        if !bytes.Equal(got, value) {
            t.Fatalf("Expected value %s, got %s", value, got)
        }
    }
}
=== RUN   TestMemtableFlush
    k4_test.go:172: Expected value value0, got 
--- FAIL: TestMemtableFlush (0.06s)

We are failing on specifically

if !bf.Check(key) {
    return nil, nil
}

Within the function func (sstable *SSTable) get(key []byte) ([]byte, error)

This was working fine over here: https://github.com/guycipher/k4/tree/a852e946d6c32ab5911f6dc96bf15fa6945aba93

Doing code review to find this bug, it's an odd one. Feel free to assist, an extra pair of eyes and brain would be awesome!

guycipher commented 1 day ago

I've reviewed majority of the flushing logic, we are creating the bloom filter, writing keys to the bloom filter, serializing and writing it to sstable, nothing changed there. The compression logic was introduced not to long ago but I did not add anything that would truly change the original logic just adding to it.

guycipher commented 1 day ago

Still investigating. Narrowed down to this commit https://github.com/guycipher/k4/commit/32ff5db6ab8b3fd81ef13dce6810e13a145c2813#diff-ca5e6b75d800b93cb0f5df5bbd62d9de4695638e22aa3114df827c372b080ae7

guycipher commented 1 day ago

Found it. It's the bytes.Compare(value, []byte(TOMBSTONE_VALUE)) == 0 Woops, I must be a bit tired. Fixing up.