goraft / raft

UNMAINTAINED: A Go implementation of the Raft distributed consensus protocol.
MIT License
2.43k stars 479 forks source link

Pad snapshot filenames with 0s #237

Open otoolep opened 10 years ago

otoolep commented 10 years ago

Allowing up to 24 0s is sufficient for a 64-bit unsigned int.

otoolep commented 10 years ago

OK, what about this trivial change to address issue 235? This will not fix busted systems, but systems going forward should be better.

otoolep commented 10 years ago

I ran the unit test suite with this change in place. It passed.

otoolep commented 10 years ago

I guess to be fully functional, the patch would need to pad last index too.

otoolep commented 10 years ago

http://play.golang.org/p/B5dVjbT37o

otoolep commented 10 years ago

This is, of course, not backwards compatible with snapshots that already exist.

bcwaldon commented 10 years ago

Handling backwards-compatibility could look like so:

func NewSnapshotFileNames(names []string) ([]SnapshotFileName, error) {
    s := make([]SnapshotFileName, 0)
    for _, n := range names {
        trimmed := strings.TrimSuffix(n, ".ss")
        if trimmed == n {
            return nil, fmt.Errorf("file %q does not have .ss extension", n)
        }

        parts := strings.SplitN(trimmed, "_", 2)
        if len(parts) != 2 {
            return nil, fmt.Errorf("unrecognized file name format %q", n)
        }

        fn := SnapshotFileName{FileName: n}

        var err error
        fn.Term, err = strconv.ParseUint(parts[0], 10, 64)
        if err != nil {
            return nil, fmt.Errorf("unable to parse term from filename %q: %v", err)
        }

        fn.Index, err = strconv.ParseUint(parts[1], 10, 64)
        if err != nil {
            return nil, fmt.Errorf("unable to parse index from filename %q: %v", err)
        }

        s = append(s, fn)
    }

    sortable := SnapshotFileNames(s)
    sort.Sort(&sortable)
    return s, nil
}

type SnapshotFileNames []SnapshotFileName
type SnapshotFileName struct {
    FileName string
    Term     uint64
    Index    uint64
}

func (n *SnapshotFileNames) Less(i, j int) bool {
    iTerm, iIndex := (*n)[i].Term, (*n)[i].Index
    jTerm, jIndex := (*n)[j].Term, (*n)[j].Index
    return iTerm < jTerm || (iTerm == jTerm && iIndex < jIndex)
}

func (n *SnapshotFileNames) Swap(i, j int) {
    (*n)[i], (*n)[j] = (*n)[j], (*n)[i]
}

func (n *SnapshotFileNames) Len() int {
    return len([]SnapshotFileName(*n))
}
bcwaldon commented 10 years ago

And if we've already established one filename format out there, I question why we want to change that if we still need to deal with backwards-compatibility.

otoolep commented 10 years ago

Cute. :-)