Skip to content
This repository has been archived by the owner on Sep 6, 2018. It is now read-only.

Pad snapshot filenames with 0s #237

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Pad snapshot filenames with 0s #237

wants to merge 1 commit into from

Conversation

otoolep
Copy link
Contributor

@otoolep otoolep commented Oct 8, 2014

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

@otoolep
Copy link
Contributor Author

otoolep commented Oct 8, 2014

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

@otoolep
Copy link
Contributor Author

otoolep commented Oct 8, 2014

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

@otoolep
Copy link
Contributor Author

otoolep commented Oct 8, 2014

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

Allowing up to 24 0s is sufficient for each 64-bit unsigned int.
@otoolep
Copy link
Contributor Author

otoolep commented Oct 8, 2014

@otoolep
Copy link
Contributor Author

otoolep commented Oct 8, 2014

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

@bcwaldon
Copy link
Contributor

bcwaldon commented Oct 8, 2014

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
Copy link
Contributor

bcwaldon commented Oct 8, 2014

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
Copy link
Contributor Author

otoolep commented Oct 8, 2014

Cute. :-)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants