refactor(general): remove no-longer used functions and tests (#3924)

Followup cleanup for #3919

All the changes are in the `epoch` package

Summary:
- Removal of now unused code and tests.
- Refactoring to simplify test and remove a linter annotation.
- Fix a typo
This commit is contained in:
Julio López
2024-06-18 12:24:56 -07:00
committed by GitHub
parent 55c750f291
commit 4786ab3cbb
2 changed files with 34 additions and 396 deletions

View File

@@ -115,8 +115,6 @@ func (r closedIntRange) isEmpty() bool {
return r.length() == 0
}
var errNonContiguousRange = errors.New("non-contiguous range")
// constants from the standard math package.
const (
//nolint:mnd
@@ -126,42 +124,6 @@ func (r closedIntRange) isEmpty() bool {
minInt = -1 << (intSize - 1)
)
// Returns a range for the keys in m. It returns an empty range when m is empty.
func getKeyRange[E any](m map[int]E) closedIntRange {
if len(m) == 0 {
return closedIntRange{lo: 0, hi: -1}
}
lo, hi := maxInt, minInt
for k := range m {
if k < lo {
lo = k
}
if k > hi {
hi = k
}
}
return closedIntRange{lo: lo, hi: hi}
}
// Returns a contiguous range for the keys in m.
// When the range is not continuous an error is returned.
func getContiguousKeyRange[E any](m map[int]E) (closedIntRange, error) {
r := getKeyRange(m)
// r.hi and r.lo are from unique map keys, so for the range to be continuous
// then the range length must be exactly the same as the size of the map.
// For example, if lo==2, hi==4, and len(m) == 3, the range must be
// contiguous => {2, 3, 4}
if r.length() != uint(len(m)) {
return closedIntRange{-1, -2}, errors.Wrapf(errNonContiguousRange, "[lo: %d, hi: %d], length: %d", r.lo, r.hi, len(m))
}
return r, nil
}
func getFirstContiguousKeyRange[E any](m map[int]E) closedIntRange {
if len(m) == 0 {
return closedIntRange{lo: 0, hi: -1}

View File

@@ -78,243 +78,11 @@ func TestGroupByEpochNumber(t *testing.T) {
}
}
func TestGetContiguosKeyRange(t *testing.T) {
invalidEmptyRange := closedIntRange{-1, -2}
cases := []struct {
input map[int]bool
want closedIntRange
shouldErr bool
length uint
isEmpty bool
}{
{
isEmpty: true,
want: closedIntRange{0, -1},
},
{
input: map[int]bool{0: true},
want: closedIntRange{lo: 0, hi: 0},
length: 1,
},
{
input: map[int]bool{-5: true},
want: closedIntRange{lo: -5, hi: -5},
length: 1,
},
{
input: map[int]bool{-5: true, -4: true},
want: closedIntRange{lo: -5, hi: -4},
length: 2,
},
{
input: map[int]bool{0: true},
want: closedIntRange{lo: 0, hi: 0},
length: 1,
},
{
input: map[int]bool{5: true},
want: closedIntRange{lo: 5, hi: 5},
length: 1,
},
{
input: map[int]bool{0: true, 1: true},
want: closedIntRange{lo: 0, hi: 1},
length: 2,
},
{
input: map[int]bool{8: true, 9: true},
want: closedIntRange{lo: 8, hi: 9},
length: 2,
},
{
input: map[int]bool{1: true, 2: true, 3: true, 4: true, 5: true},
want: closedIntRange{lo: 1, hi: 5},
length: 5,
},
{
input: map[int]bool{8: true, 10: true},
want: invalidEmptyRange,
shouldErr: true,
isEmpty: true,
},
{
input: map[int]bool{1: true, 2: true, 3: true, 5: true},
want: invalidEmptyRange,
shouldErr: true,
isEmpty: true,
},
{
input: map[int]bool{-5: true, -7: true},
want: invalidEmptyRange,
shouldErr: true,
isEmpty: true,
},
{
input: map[int]bool{0: true, minInt: true},
want: invalidEmptyRange,
shouldErr: true,
isEmpty: true,
},
{
input: map[int]bool{0: true, maxInt: true},
want: invalidEmptyRange,
shouldErr: true,
isEmpty: true,
},
{
input: map[int]bool{maxInt: true, minInt: true},
want: invalidEmptyRange,
shouldErr: true,
isEmpty: true,
},
{
input: map[int]bool{minInt: true},
want: closedIntRange{lo: minInt, hi: minInt},
length: 1,
},
{
input: map[int]bool{maxInt - 1: true},
want: closedIntRange{lo: maxInt - 1, hi: maxInt - 1},
length: 1,
},
{
input: map[int]bool{maxInt: true},
want: closedIntRange{lo: maxInt, hi: maxInt},
length: 1,
},
}
for i, tc := range cases {
t.Run(fmt.Sprint("case: ", i), func(t *testing.T) {
got, err := getContiguousKeyRange(tc.input)
if tc.shouldErr {
require.Error(t, err, "input: %v", tc.input)
}
require.Equal(t, tc.want, got, "input: %#v", tc.input)
require.Equal(t, tc.length, got.length())
require.Equal(t, tc.isEmpty, got.isEmpty())
})
}
}
func TestAssertMinMaxIntConstants(t *testing.T) {
require.Equal(t, math.MinInt, minInt)
require.Equal(t, math.MaxInt, maxInt)
}
func TestGetKeyRange(t *testing.T) {
cases := []struct {
input map[int]bool
want closedIntRange
length uint
isEmpty bool
}{
{
isEmpty: true,
want: closedIntRange{lo: 0, hi: -1},
},
{
input: map[int]bool{0: true},
want: closedIntRange{lo: 0, hi: 0},
length: 1,
},
{
input: map[int]bool{-5: true},
want: closedIntRange{lo: -5, hi: -5},
length: 1,
},
{
input: map[int]bool{-5: true, -4: true},
want: closedIntRange{lo: -5, hi: -4},
length: 2,
},
{
input: map[int]bool{0: true},
want: closedIntRange{lo: 0, hi: 0},
length: 1,
},
{
input: map[int]bool{5: true},
want: closedIntRange{lo: 5, hi: 5},
length: 1,
},
{
input: map[int]bool{0: true, 1: true},
want: closedIntRange{lo: 0, hi: 1},
length: 2,
},
{
input: map[int]bool{8: true, 9: true},
want: closedIntRange{lo: 8, hi: 9},
length: 2,
},
{
input: map[int]bool{1: true, 2: true, 3: true, 4: true, 5: true},
want: closedIntRange{lo: 1, hi: 5},
length: 5,
},
{
input: map[int]bool{8: true, 10: true},
want: closedIntRange{lo: 8, hi: 10},
length: 3,
},
{
input: map[int]bool{1: true, 2: true, 3: true, 5: true},
want: closedIntRange{lo: 1, hi: 5},
length: 5,
},
{
input: map[int]bool{-5: true, -7: true},
want: closedIntRange{lo: -7, hi: -5},
length: 3,
},
{
input: map[int]bool{0: true, minInt: true},
want: closedIntRange{lo: minInt, hi: 0},
length: -minInt + 1,
},
{
input: map[int]bool{0: true, maxInt: true},
want: closedIntRange{lo: 0, hi: maxInt},
length: maxInt + 1,
},
{
input: map[int]bool{maxInt: true, minInt: true},
want: closedIntRange{lo: minInt, hi: maxInt},
length: 0, // corner case, not representable :(
isEmpty: true,
},
{
input: map[int]bool{minInt: true},
want: closedIntRange{lo: minInt, hi: minInt},
length: 1,
},
{
input: map[int]bool{maxInt - 1: true},
want: closedIntRange{lo: maxInt - 1, hi: maxInt - 1},
length: 1,
},
{
input: map[int]bool{maxInt: true},
want: closedIntRange{lo: maxInt, hi: maxInt},
length: 1,
},
}
for i, tc := range cases {
t.Run(fmt.Sprint("case: ", i), func(t *testing.T) {
got := getKeyRange(tc.input)
require.Equal(t, tc.want, got, "input: %#v", tc.input)
require.Equal(t, tc.length, got.length())
require.Equal(t, tc.isEmpty, got.isEmpty())
})
}
}
//nolint:maintidx
func TestOldestUncompactedEpoch(t *testing.T) {
cases := []struct {
input CurrentSnapshot
@@ -408,136 +176,64 @@ func TestOldestUncompactedEpoch(t *testing.T) {
{
input: CurrentSnapshot{
LongestRangeCheckpointSets: []*RangeMetadata{
{
MinEpoch: 0,
MaxEpoch: 2,
Blobs: []blob.Metadata{
{BlobID: rangeCheckpointBlobPrefix(0, 2) + "foo-0-2"},
},
},
},
LongestRangeCheckpointSets: makeLongestRange(0, 2),
},
expectedEpoch: 3,
},
{
input: CurrentSnapshot{
LongestRangeCheckpointSets: []*RangeMetadata{
{
MinEpoch: 0,
MaxEpoch: 2,
Blobs: []blob.Metadata{
{BlobID: rangeCheckpointBlobPrefix(0, 2) + "foo-0-2"},
},
},
},
SingleEpochCompactionSets: makeSingleCompactionEpochSets([]int{0, 1}),
LongestRangeCheckpointSets: makeLongestRange(0, 2),
SingleEpochCompactionSets: makeSingleCompactionEpochSets([]int{0, 1}),
},
expectedEpoch: 3,
},
{
input: CurrentSnapshot{
LongestRangeCheckpointSets: []*RangeMetadata{
{
MinEpoch: 0,
MaxEpoch: 2,
Blobs: []blob.Metadata{
{BlobID: rangeCheckpointBlobPrefix(0, 2) + "foo-0-2"},
},
},
},
SingleEpochCompactionSets: makeSingleCompactionEpochSets([]int{1, 2}),
LongestRangeCheckpointSets: makeLongestRange(0, 2),
SingleEpochCompactionSets: makeSingleCompactionEpochSets([]int{1, 2}),
},
expectedEpoch: 3,
},
{
input: CurrentSnapshot{
LongestRangeCheckpointSets: []*RangeMetadata{
{
MinEpoch: 0,
MaxEpoch: 2,
Blobs: []blob.Metadata{
{BlobID: rangeCheckpointBlobPrefix(0, 2) + "foo-0-2"},
},
},
},
SingleEpochCompactionSets: makeSingleCompactionEpochSets([]int{1}),
LongestRangeCheckpointSets: makeLongestRange(0, 2),
SingleEpochCompactionSets: makeSingleCompactionEpochSets([]int{1}),
},
expectedEpoch: 3,
},
{
input: CurrentSnapshot{
LongestRangeCheckpointSets: []*RangeMetadata{
{
MinEpoch: 0,
MaxEpoch: 2,
Blobs: []blob.Metadata{
{BlobID: rangeCheckpointBlobPrefix(0, 2) + "foo-0-2"},
},
},
},
SingleEpochCompactionSets: makeSingleCompactionEpochSets([]int{4, 5}),
LongestRangeCheckpointSets: makeLongestRange(0, 2),
SingleEpochCompactionSets: makeSingleCompactionEpochSets([]int{4, 5}),
},
expectedEpoch: 3,
},
{
input: CurrentSnapshot{
LongestRangeCheckpointSets: []*RangeMetadata{
{
MinEpoch: 0,
MaxEpoch: 2,
Blobs: []blob.Metadata{
{BlobID: rangeCheckpointBlobPrefix(0, 2) + "foo-0-2"},
},
},
},
SingleEpochCompactionSets: makeSingleCompactionEpochSets([]int{2, 3}),
LongestRangeCheckpointSets: makeLongestRange(0, 2),
SingleEpochCompactionSets: makeSingleCompactionEpochSets([]int{2, 3}),
},
expectedEpoch: 4,
},
{
input: CurrentSnapshot{
LongestRangeCheckpointSets: []*RangeMetadata{
{
MinEpoch: 0,
MaxEpoch: 2,
Blobs: []blob.Metadata{
{BlobID: rangeCheckpointBlobPrefix(0, 2) + "foo-0-2"},
},
},
},
SingleEpochCompactionSets: makeSingleCompactionEpochSets([]int{3, 4}),
LongestRangeCheckpointSets: makeLongestRange(0, 2),
SingleEpochCompactionSets: makeSingleCompactionEpochSets([]int{3, 4}),
},
expectedEpoch: 5,
},
{
input: CurrentSnapshot{
LongestRangeCheckpointSets: []*RangeMetadata{
{
MinEpoch: 1,
MaxEpoch: 2,
Blobs: []blob.Metadata{
{BlobID: rangeCheckpointBlobPrefix(1, 2) + "foo-1-2"},
},
},
},
SingleEpochCompactionSets: makeSingleCompactionEpochSets([]int{3, 4}),
LongestRangeCheckpointSets: makeLongestRange(1, 2),
SingleEpochCompactionSets: makeSingleCompactionEpochSets([]int{3, 4}),
},
expectedEpoch: -1,
wantErr: errInvalidCompactedRange,
},
{
input: CurrentSnapshot{
LongestRangeCheckpointSets: []*RangeMetadata{
{
MinEpoch: 0,
MaxEpoch: 2,
Blobs: []blob.Metadata{
{BlobID: rangeCheckpointBlobPrefix(0, 2) + "foo-0-2"},
},
},
},
LongestRangeCheckpointSets: makeLongestRange(0, 2),
// non-contiguous single epoch compaction set
SingleEpochCompactionSets: makeSingleCompactionEpochSets([]int{3, 5}),
},
@@ -545,15 +241,7 @@ func TestOldestUncompactedEpoch(t *testing.T) {
},
{
input: CurrentSnapshot{
LongestRangeCheckpointSets: []*RangeMetadata{
{
MinEpoch: 0,
MaxEpoch: 7,
Blobs: []blob.Metadata{
{BlobID: rangeCheckpointBlobPrefix(0, 7) + "foo-0-7"},
},
},
},
LongestRangeCheckpointSets: makeLongestRange(0, 7),
// non-contiguous single epoch compaction set, but most of the set overlaps with the compacted range
SingleEpochCompactionSets: makeSingleCompactionEpochSets([]int{0, 1, 2, 4, 6, 9}),
},
@@ -561,30 +249,14 @@ func TestOldestUncompactedEpoch(t *testing.T) {
},
{
input: CurrentSnapshot{
LongestRangeCheckpointSets: []*RangeMetadata{
{
MinEpoch: 0,
MaxEpoch: 7,
Blobs: []blob.Metadata{
{BlobID: rangeCheckpointBlobPrefix(0, 7) + "foo-0-7"},
},
},
},
SingleEpochCompactionSets: makeSingleCompactionEpochSets([]int{9, 10}),
LongestRangeCheckpointSets: makeLongestRange(0, 7),
SingleEpochCompactionSets: makeSingleCompactionEpochSets([]int{9, 10}),
},
expectedEpoch: 8,
},
{
input: CurrentSnapshot{
LongestRangeCheckpointSets: []*RangeMetadata{
{
MinEpoch: 0,
MaxEpoch: 7,
Blobs: []blob.Metadata{
{BlobID: rangeCheckpointBlobPrefix(0, 7) + "foo-0-7"},
},
},
},
LongestRangeCheckpointSets: makeLongestRange(0, 7),
// non-contiguous single epoch compaction set
SingleEpochCompactionSets: makeSingleCompactionEpochSets([]int{8, 10}),
},
@@ -592,15 +264,7 @@ func TestOldestUncompactedEpoch(t *testing.T) {
},
{
input: CurrentSnapshot{
LongestRangeCheckpointSets: []*RangeMetadata{
{
MinEpoch: 0,
MaxEpoch: 7,
Blobs: []blob.Metadata{
{BlobID: rangeCheckpointBlobPrefix(0, 7) + "foo-0-7"},
},
},
},
LongestRangeCheckpointSets: makeLongestRange(0, 7),
// non-contiguous single epoch compaction set
SingleEpochCompactionSets: makeSingleCompactionEpochSets([]int{8, 9, 12}),
},
@@ -632,7 +296,19 @@ func makeSingleCompactionEpochSets(epochs []int) map[int][]blob.Metadata {
return es
}
func TestGetFirstContiguosKeyRange(t *testing.T) {
func makeLongestRange(minEpoch, maxEpoch int) []*RangeMetadata {
return []*RangeMetadata{
{
MinEpoch: minEpoch,
MaxEpoch: maxEpoch,
Blobs: []blob.Metadata{
{BlobID: blob.ID(fmt.Sprintf("%sfoo-%v-%v", rangeCheckpointBlobPrefix(minEpoch, maxEpoch), minEpoch, maxEpoch))},
},
},
}
}
func TestGetFirstContiguousKeyRange(t *testing.T) {
cases := []struct {
input map[int]bool
want closedIntRange