Skip to content

Conversation

joe-elliott
Copy link
Contributor

What this PR does:
Updates the behavior of the query frontend to use round robin when choosing the next tenant to service. For lack of a better name a struct "queueManager" was created to provide round robin access to tenant queues. A combination of a linked list and map were used to provide O(1) for the deleteQueue, getNextQueue and getQueue operations.

Which issue(s) this PR fixes:
Fixes #2431

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

Signed-off-by: Joe Elliott <[email protected]>
Signed-off-by: Joe Elliott <[email protected]>
Signed-off-by: Joe Elliott <[email protected]>
Signed-off-by: Joe Elliott <[email protected]>
Signed-off-by: Joe Elliott <[email protected]>
Signed-off-by: Joe Elliott <[email protected]>
Signed-off-by: Joe Elliott <[email protected]>
Signed-off-by: Joe Elliott <[email protected]>
Signed-off-by: Joe Elliott <[email protected]>
Signed-off-by: Joe Elliott <[email protected]>
Signed-off-by: Joe Elliott <[email protected]>
@joe-elliott joe-elliott changed the title Round robin Update Query Frontend to use Round Robin for Choosing the Next Queue Apr 30, 2020
@joe-elliott
Copy link
Contributor Author

Benchmarks:

Branch

BenchmarkGetNextRequest-8   	   39643	     38105 ns/op	       0 B/op	       0 allocs/op
BenchmarkQueueRequest-8     	    8538	    143534 ns/op	   56613 B/op	     606 allocs/op

BenchmarkGetNextRequest-8   	   39324	     30886 ns/op	       0 B/op	       0 allocs/op
BenchmarkQueueRequest-8     	   15386	    122518 ns/op	   56606 B/op	     606 allocs/op

BenchmarkGetNextRequest-8   	   33900	     32455 ns/op	       0 B/op	       0 allocs/op
BenchmarkQueueRequest-8     	   12225	    101954 ns/op	   56609 B/op	     606 allocs/op

Master

BenchmarkGetNextRequest-8   	    5682	    233817 ns/op	   53162 B/op	     100 allocs/op
BenchmarkQueueRequest-8     	   12513	    101051 ns/op	   52605 B/op	     506 allocs/op

BenchmarkGetNextRequest-8   	    5647	    190832 ns/op	   53200 B/op	     100 allocs/op
BenchmarkQueueRequest-8     	   16616	     93640 ns/op	   52609 B/op	     506 allocs/op

BenchmarkGetNextRequest-8   	    5505	    225738 ns/op	   53122 B/op	     100 allocs/op
BenchmarkQueueRequest-8     	   12760	     91275 ns/op	   52606 B/op	     506 allocs/op

@joe-elliott
Copy link
Contributor Author

@tomwilkie if you could review when you get a chance, i would appreciate it.

Copy link
Contributor

@pstibrany pstibrany left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job, very clean code. I miss at least short comment explaining queueManager, also name is too generic imho.

Thank you! (And sorry for so long delay)

userID string
}

type queueManager struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Short comment about this struct would be nice.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a comment and went with queueIterator. Let me know if you'd like any other changes.

Thanks for the review!

Copy link
Contributor

@pracucci pracucci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job @joe-elliott! LGTM 🎉

@pracucci pracucci merged commit 43926c4 into cortexproject:master May 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Query Frontend should use Round Robin to Choose the Next Tenant to Service
3 participants