feat(topics): make topic queue length configurable #2661
Conversation
alert/topics.go
Outdated
| } | ||
|
|
||
| func NewTopics() *Topics { | ||
| func NewTopics(bufferSize int) *Topics { | ||
| if bufferSize <= 0 { |
There was a problem hiding this comment.
Do we want a larger minimum, like 1K or something? I cold see a 3-byte buffer causing problems.
alert/topics.go
Outdated
| @@ -385,10 +389,13 @@ type bufHandler struct { | |||
| wg sync.WaitGroup | |||
| } | |||
|
|
|||
| func newHandler(h Handler) *bufHandler { | |||
| func newHandler(h Handler, bufferSize int) *bufHandler { | |||
| if bufferSize == 0 { | |||
There was a problem hiding this comment.
Why the difference in safety checks: above you use <= 0
There was a problem hiding this comment.
+1 -- this is actually a bug imho since bufferSize is an int. this would panic 100% if someone passed in, say, -1.
docmerlin
force-pushed
the
topicqueueleng
branch
from
7dab35a to
5a8696e
Compare
alert/topics.go
Outdated
|
|
||
| collected *expvar.Int | ||
| statsKey string | ||
|
|
||
| handlers []*bufHandler | ||
| } | ||
|
|
||
| func newTopic(id string) *Topic { | ||
| func newTopic(id string, bufferLength int) *Topic { |
There was a problem hiding this comment.
Sorry, this was the one I thought should be a method of the Topics struct, so bufferLength is magically available. My mistake on NewTopics() being a method of the thing that it created.
There was a problem hiding this comment.
That's actually a good idea.
| @@ -385,10 +391,13 @@ type bufHandler struct { | |||
| wg sync.WaitGroup | |||
| } | |||
|
|
|||
| func newHandler(h Handler) *bufHandler { | |||
| func newHandler(h Handler, bufferSize int) *bufHandler { | |||
| if bufferSize < MinimumEventBufferSize { | |||
docmerlin
force-pushed
the
topicqueueleng
branch
from
5a8696e to
faedd69
Compare
|
@docmerlin -- docs will need the name of the config parameter, along with its default value....and where it appears in the config file. Essentially, the config documentation needs to be updated to describe how customers can set this. |
This is a tuning param to make the topic queue length configurable. This is useful if very large amounts of alerts must go off due to scheduled maintenance etc such as a windows update.