Repository navigation
Add dedicated thread option for log dequeue - #6309
tiagohudler wants to merge 1 commit into
Conversation
* feat: IDispatcher interface * feat: add timer dispatcher - modify IDispatcher to include stop method - modify set method to only define the callback * feat: add dedicated thread dispatcher * feat: modify AsyncTargetWrapper to use new dispatchers * feat: add user-facing config for dispatcher type
|
| /// Milliseconds between invocations. Zero (0) or a negative number disables periodic signaling. | ||
| /// </param> | ||
| /// <returns>true if the change was applied; false if the dispatcher has been stopped.</returns> | ||
| bool Change(int dueTime, int period); |
There was a problem hiding this comment.
This schedule interface seems very leaky, and directly exposes the timer-logic (that only applies to one implementation instance)
There was a problem hiding this comment.
The reason for this was that I tried to copy the timer logic in order to change as little of the wrapper as possible. The dedicated thread also behaves like a timer, since it only triggers when an instant Change() is called or when it was set with a period greater than zero.
| /// </summary> | ||
| /// <param name="callback">The method that gets called by the dispatcher when it triggers.</param> | ||
| /// <returns>true if the setup was successful.</returns> | ||
| bool Set(Action callback); |
There was a problem hiding this comment.
Any reason for having this instead of a constructor-parameter on the implementation-instance?
There was a problem hiding this comment.
I made this like this so that a client would be able to change the callback, but looking back on it maybe this not only is not necessary, it also increases complexity for the client and makes the flow more error-prone. Can I try to improve on this?
2 participants
This change refers to issue #5478
It adds two so called dispatchers, which take the place of the previously used timer for dequeueing log messages in the AsyncTargetWrapper:
The user can select which one of the two to use by adding the "dispatcher" attribute on the wrapper element in the config, like so:
This solves the problem of threadpool starvation when the target was running on overflowAction="Block", because the DedicatedThreadDispatcher does not depend on the threadpool, as verified by this local test run:
For future work, this can maybe be rewritten as a signal-based consumer-producer flow. For now I tried to change as little of the AsyncTargetWrapper logic as possible.