diff --git a/pkg/querier/tripperware/queryrange/limits_test.go b/pkg/querier/tripperware/queryrange/limits_test.go index c1e5954421..d5d4a6b230 100644 --- a/pkg/querier/tripperware/queryrange/limits_test.go +++ b/pkg/querier/tripperware/queryrange/limits_test.go @@ -2,7 +2,6 @@ package queryrange import ( "context" - "github.com/cortexproject/cortex/pkg/util/validation" "testing" "time" @@ -13,6 +12,7 @@ import ( "github.com/cortexproject/cortex/pkg/querier/tripperware" "github.com/cortexproject/cortex/pkg/util" + "github.com/cortexproject/cortex/pkg/util/validation" ) func TestLimitsMiddleware_MaxQueryLookback(t *testing.T) { diff --git a/pkg/scheduler/queue/user_queues.go b/pkg/scheduler/queue/user_queues.go index b0ae8069ac..25f562ee02 100644 --- a/pkg/scheduler/queue/user_queues.go +++ b/pkg/scheduler/queue/user_queues.go @@ -2,7 +2,6 @@ package queue import ( "math/rand" - "reflect" "sort" "time" @@ -136,7 +135,7 @@ func (q *queues) getOrAddQueue(userID string, maxQueriers int) userRequestQueue uq := q.userQueues[userID] priorityEnabled := q.limits.QueryPriority(userID).Enabled maxOutstanding := q.limits.MaxOutstandingPerTenant(userID) - priorityList := getPriorityList(q.limits.QueryPriority(userID), len(q.queriers)) + priorityList := getPriorityList(q.limits.QueryPriority(userID), maxQueriers) if uq == nil { uq = &userQueue{ @@ -180,7 +179,7 @@ func (q *queues) getOrAddQueue(userID string, maxQueriers int) userRequestQueue uq.queriers = shuffleQueriersForUser(uq.seed, maxQueriers, q.sortedQueriers, nil) } - if priorityEnabled && !reflect.DeepEqual(uq.priorityList, priorityList) { + if priorityEnabled && hasPriorityListChanged(uq.priorityList, priorityList) { reservedQueriers := make(map[string]int64) i := 0 @@ -406,6 +405,18 @@ func getPriorityList(queryPriority validation.QueryPriority, totalQuerierCount i return priorityList } +func hasPriorityListChanged(old, new []int64) bool { + if len(old) != len(new) { + return true + } + for i := range old { + if old[i] != new[i] { + return true + } + } + return false +} + // MockLimits implements the Limits interface. Used in tests only. type MockLimits struct { MaxOutstanding int diff --git a/pkg/scheduler/queue/user_queues_test.go b/pkg/scheduler/queue/user_queues_test.go index ae812aac6a..ded597baa0 100644 --- a/pkg/scheduler/queue/user_queues_test.go +++ b/pkg/scheduler/queue/user_queues_test.go @@ -399,6 +399,7 @@ func TestGetOrAddQueueShouldUpdateProperties(t *testing.T) { assert.Equal(t, 3, q.userQueues["userID"].maxQueriers) assert.Equal(t, 5, len(q.queriers)) assert.Equal(t, 3, len(q.userQueues["userID"].queriers)) + assert.Equal(t, 2, len(q.userQueues["userID"].reservedQueriers)) assert.IsType(t, &PriorityRequestQueue{}, queue) assert.Equal(t, 1, queue.length()) assert.ElementsMatch(t, []int64{1, 1}, q.userQueues["userID"].priorityList) @@ -406,6 +407,38 @@ func TestGetOrAddQueueShouldUpdateProperties(t *testing.T) { assert.Subset(t, getKeys(q.queriers), getKeys(q.userQueues["userID"].queriers)) assert.Subset(t, getKeys(q.userQueues["userID"].queriers), getKeys(q.userQueues["userID"].reservedQueriers)) + limits.QueryPriorityVal = validation.QueryPriority{Enabled: true, Priorities: []validation.PriorityDef{ + { + Priority: 1, + ReservedQueriers: 0.5, + }, + }} + q.limits = limits + _ = q.getOrAddQueue("userID", 3) + + assert.Equal(t, 10, q.userQueues["userID"].maxOutstanding) + assert.Equal(t, 3, q.userQueues["userID"].maxQueriers) + assert.Equal(t, 5, len(q.queriers)) + assert.Equal(t, 3, len(q.userQueues["userID"].queriers)) + assert.Equal(t, 2, len(q.userQueues["userID"].reservedQueriers)) + assert.ElementsMatch(t, []int64{1, 1}, q.userQueues["userID"].priorityList) + + limits.QueryPriorityVal = validation.QueryPriority{Enabled: true, Priorities: []validation.PriorityDef{ + { + Priority: 1, + ReservedQueriers: 10, + }, + }} + q.limits = limits + _ = q.getOrAddQueue("userID", 3) + + assert.Equal(t, 10, q.userQueues["userID"].maxOutstanding) + assert.Equal(t, 3, q.userQueues["userID"].maxQueriers) + assert.Equal(t, 5, len(q.queriers)) + assert.Equal(t, 3, len(q.userQueues["userID"].queriers)) + assert.Equal(t, 0, len(q.userQueues["userID"].reservedQueriers)) + assert.ElementsMatch(t, []int64{}, q.userQueues["userID"].priorityList) + limits.QueryPriorityVal.Enabled = false q.limits = limits queue = q.getOrAddQueue("userID", 3) @@ -582,69 +615,11 @@ func TestShuffleQueriersCorrectness(t *testing.T) { } } -func TestShuffleQueriers_WithReservedQueriers(t *testing.T) { - //allQueriers := []string{"a", "b", "c", "d", "e"} - // - //queriers, reservedQueriers := shuffleQueriersForUser(12345, 0, allQueriers, 0, nil) - //require.Nil(t, queriers) - //require.Equal(t, 0, len(reservedQueriers)) - // - //queriers, reservedQueriers = shuffleQueriersForUser(12345, 0, allQueriers, 0.5, nil) - //require.Nil(t, queriers) - //require.Equal(t, 3, len(reservedQueriers)) - // - //queriers, reservedQueriers = shuffleQueriersForUser(12345, 0, allQueriers, 1, nil) - //require.Nil(t, queriers) - //require.Equal(t, 1, len(reservedQueriers)) - // - //queriers, reservedQueriers = shuffleQueriersForUser(12345, 0, allQueriers, 100, nil) - //require.Nil(t, queriers) - //require.Equal(t, 5, len(reservedQueriers)) - // - //queriers, reservedQueriers = shuffleQueriersForUser(12345, 3, allQueriers, 0, nil) - //require.Equal(t, 3, len(queriers)) - //require.Equal(t, 0, len(reservedQueriers)) - // - //queriers, reservedQueriers = shuffleQueriersForUser(12345, 3, allQueriers, 0.5, nil) - //require.Equal(t, 3, len(queriers)) - //require.Equal(t, 2, len(reservedQueriers)) - // - //queriers, reservedQueriers = shuffleQueriersForUser(12345, 3, allQueriers, 1, nil) - //require.Equal(t, 3, len(queriers)) - //require.Equal(t, 1, len(reservedQueriers)) - // - //queriers, reservedQueriers = shuffleQueriersForUser(12345, 3, allQueriers, 100, nil) - //require.Equal(t, 3, len(queriers)) - //require.Equal(t, 3, len(reservedQueriers)) - // - //queriers, reservedQueriers = shuffleQueriersForUser(12345, 100, allQueriers, 0, nil) - //require.Nil(t, queriers) - //require.Equal(t, 0, len(reservedQueriers)) - // - //queriers, reservedQueriers = shuffleQueriersForUser(12345, 100, allQueriers, 0.5, nil) - //require.Nil(t, queriers) - //require.Equal(t, 3, len(reservedQueriers)) - // - //queriers, reservedQueriers = shuffleQueriersForUser(12345, 100, allQueriers, 1, nil) - //require.Nil(t, queriers) - //require.Equal(t, 1, len(reservedQueriers)) - // - //queriers, reservedQueriers = shuffleQueriersForUser(12345, 100, allQueriers, 100, nil) - //require.Nil(t, queriers) - //require.Equal(t, 5, len(reservedQueriers)) -} - -func TestShuffleQueriers_WithReservedQueriers_Correctness(t *testing.T) { - //allQueriers := []string{"a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l", "m", "n"} - // - //prevQueriers, prevReservedQueriers := shuffleQueriersForUser(12345, 10, allQueriers, 5, nil) - //for i := 0; i < 100; i++ { - // queriers, reservedQueriers := shuffleQueriersForUser(12345, 10, allQueriers, 5, nil) - // require.Equal(t, prevQueriers, queriers) - // require.Equal(t, prevReservedQueriers, reservedQueriers) - // prevQueriers = queriers - // prevReservedQueriers = reservedQueriers - //} +func TestHasPriorityListChanged(t *testing.T) { + require.True(t, hasPriorityListChanged([]int64{1, 2}, []int64{1, 3})) + require.False(t, hasPriorityListChanged([]int64{1, 2}, []int64{1, 2})) + require.True(t, hasPriorityListChanged([]int64{1, 2}, []int64{1})) + require.False(t, hasPriorityListChanged([]int64{}, []int64{})) } func TestGetPriorityList(t *testing.T) {