Remove reference to global config in monitor.go #251
Conversation
This updates MakeMonitor() to take the channel size as a parameter, instead of reading it from the global `config` object. Unfortunately, the caller of MakeMonitor() doesn't actually have access to the global, since it's in a different package (bin vs the root package). Luckily, there doesn't appear to be a reason to have a buffer in this channel. This updates the caller to pass a hardcoded size of 1.
Generally, I think it is best to have as small of a channel size as needed (seems like larger is more likely to mask a sync issue where an empty buffer causes deadlock, etc.), so I like this change. In this particular case, the I don't see any comments or commit messages in the git history that explains why it is I guess one test would be to see if it affects throughput, which I could see being the case if we have workers waiting on the monitor goroutine. |
The goroutine running the monitor isn't actually closed. This PR updates the API to allow that Goroutine to properly block program exit. This can be leveraged as we continue to make the configuration non-global.
|
@codyprime I have not tested against a large number of hosts yet. I had the same worry as you, but it should be pretty fast. I think that having a buffer size of 1 effectively forces this to be scheduled before the next scan finishes. |
This updates MakeMonitor() to take the channel size as a parameter,
instead of reading it from the global
configobject. Unfortunately,the caller of MakeMonitor() doesn't actually have access to the global,
since it's in a different package (bin vs the root package). Luckily,
there doesn't appear to be a reason to have a buffer in this channel.
This updates the caller to pass a hardcoded size of 1.
Notes & Caveats
@codyprime what do you think about channel size here?