Fix leaked executor in raw_exec #5248
Conversation
| select { | ||
| case <-c.flushCh: | ||
| default: | ||
| close(c.flushCh) |
There was a problem hiding this comment.
This is a neat pattern, didn't know of it before
There was a problem hiding this comment.
Use with care! It does not support concurrent callers without extra coordination.
|
|
||
| func (c *circbufWriter) flushLoop() { | ||
| // Check buffer every 100ms in case a flush from Write was missed | ||
| ticker := time.NewTicker(time.Millisecond * 100) |
There was a problem hiding this comment.
May need to defer ticker.Stop() here well. Not sure how ticker works if not stopped.
There was a problem hiding this comment.
Yeah I think we always use timer and manually reset the timer which I was missing before. I may switch back to that as its a little more obvious
| // circleBufferSize is the size of the in memory ring buffer used for | ||
| // go-plugin logging to stderr. When the buffer exceeds this size before | ||
| // flushing it will begin overwriting data | ||
| circleBufferSize = 64 * 1024 |
There was a problem hiding this comment.
Is this size related to #4765 for example? we log up to 200kb log lines in some cases
There was a problem hiding this comment.
Nope! This is for the logs of the executor itself, whats found in executor.out. We're piping these logs up to the Nomad client's logger. Completely different path than task logs.
There was a problem hiding this comment.
okay, thanks for clarifying @nickethier
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
No description provided.