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

kv: add a rate limiter to control the maximum op request frequency #33

Merged
merged 1 commit into from
Mar 15, 2017

Conversation

spencerkimball
Copy link
Member

@spencerkimball spencerkimball commented Mar 15, 2017

Specify --max-rate=<frequency> in order to activate a rate limiter.

Fixes #32


This change is Reviewable

@spencerkimball
Copy link
Member Author

+cc @BramGruneir

@BramGruneir
Copy link
Member

LGTM

@petermattis
Copy link
Contributor

:lgtm:


Review status: 0 of 1 files reviewed at latest revision, 2 unresolved discussions.


kv/main.go, line 190 at r1 (raw file):

	if *maxRate > 0 {
		// Create a limiter using maxRate specified on the command line and
		// with allowed burst of 10s at the maximum allowed rate.

Where is the allowed burst of 10s happening? Looks like the burst size is 1.


kv/main.go, line 191 at r1 (raw file):

		// Create a limiter using maxRate specified on the command line and
		// with allowed burst of 10s at the maximum allowed rate.
		b.limiter = rate.NewLimiter(rate.Limit(*maxRate), 1)

Don't you want to share a rate limiter amongst all of the blockers?


Comments from Reviewable

Specify `--max-rate=<frequency>` in order to activate a rate limiter.

Fixes #32
@spencerkimball spencerkimball force-pushed the spencerkimball/kv-rate-limiter branch from f37609a to cb590ed Compare March 15, 2017 19:57
@spencerkimball
Copy link
Member Author

Review status: 0 of 1 files reviewed at latest revision, 2 unresolved discussions.


kv/main.go, line 190 at r1 (raw file):

Previously, petermattis (Peter Mattis) wrote…

Where is the allowed burst of 10s happening? Looks like the burst size is 1.

Done.


kv/main.go, line 191 at r1 (raw file):

Previously, petermattis (Peter Mattis) wrote…

Don't you want to share a rate limiter amongst all of the blockers?

Done.


Comments from Reviewable

@spencerkimball spencerkimball merged commit cb98415 into master Mar 15, 2017
@spencerkimball spencerkimball deleted the spencerkimball/kv-rate-limiter branch March 15, 2017 19:58
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.

3 participants