travisjeffery / jocko

Kafka implemented in Golang with built-in coordination (No ZK dep, single binary install, Cloud Native)
https://twitter.com/travisjeffery
MIT License
4.94k stars 363 forks source link

bug in commitlog? #145

Open mdubbyap opened 6 years ago

mdubbyap commented 6 years ago

Wrote a test for it, haven't determined if it's an issue with the index, or the what is written to the disk. If i'm doing something wrong, please let me know. Thanks!

func TestFailureInCommitLog(t *testing.T) {
    ff := func (err error) {
        if err != nil {
            panic(err.Error())
        }
    }
    path, err := ioutil.TempDir("", "commitlogtest")
    ff(err)
    os.RemoveAll(path)

    l := setupWithOptions(t, commitlog.Options{
        MaxSegmentBytes: 1024,
        MaxLogBytes:     -1,
    })
    defer cleanup(t, l)

    var offsets []int64
    var size int32
    for i := 0; i < 100; i++ {
        s := fmt.Sprintf("%d", i)
        bytes := []byte(s)

        ms := commitlog.NewMessageSet(uint64(i), commitlog.NewMessage(bytes))
        size = ms.Size()
        offset, err := l.Append(ms)
        offsets = append(offsets, offset)
        require.Equal(t, int64(i), offset)
        ff(err)
    }

    fmt.Println("about to read")
    b := make([]byte, size)
    for _, i := range offsets {
        rdr, err := l.NewReader(i, 0)
        ff(err)
        _, err = rdr.Read(b)
        ff(err)
        size := commitlog.MessageSet(b).Size()
        act := commitlog.MessageSet(b[:size])
        fmt.Println(i, act.Offset(), string(act.Payload()))
        require.Equal(t, int64(i),  act.Offset())
    }
}
mdubbyap commented 6 years ago

Used proper message encodings and kafka tools to print out the contents of the disk and things looks fine there. It appears the logic here is messed up. https://github.com/travisjeffery/jocko/blob/master/commitlog/segment.go#L220

The || e.Offset == 0 i think is trying to differentiate from unused portions, but basically makes offset 1, always fail. Moreover, the error return of s.Index.ReadEntryAtFileOffset is not being checked, and so e is not actually getting updated when seeking past where you've written to. With these two changes i can't break it anymore. I'll send you a PR.

travisjeffery commented 6 years ago

@mdubbyap did you ever work on the PR?