logthrdestdrv: process flush result after worker thread exits #2402
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The messages are ACKed by the _process_result() method. UnACKed messages are move back to the queue to be sent by syslog-ng later. Problems the patch fixes in batching mode: 1) when a queue contains unsent items during a stop request, worker threads are blocked until the batch timer not elapsed 2) when the queue contains items during reload (less then a batch_size), then we perform a flush after the worker thread exits, but as the result was not processed, these messages are never ACKed, so they were move back to the queue, and could be sent multiple times Signed-off-by: Laszlo Budai <laszlo.budai@balabit.com>
MrAnno
approved these changes
Nov 9, 2018
|
Build SUCCESS |
Kokan
reviewed
Nov 10, 2018
| @@ -639,7 +639,8 @@ _worker_thread(gpointer arg) | |||
| _schedule_restart(self); | |||
| iv_main(); | |||
|
|
|||
| log_threaded_dest_worker_flush(self); | |||
| worker_insert_result_t result = log_threaded_dest_worker_flush(self); | |||
| _process_result(self, result); | |||
There was a problem hiding this comment.
Using the _process_result has a few drawback, as it could trigger _disconnect or _rewind_batch. Although they are fine, as both of them should handle consecutive calls.
But it has also a tricky side effect about logs. The logs in the _process_result was not composed to use in this case.
For example, if the destination is not connected while the thread is in shutdown, this is going to print on info level the following: Server disconnected while preparing messages for sending, trying again. That could be confusing to see.
I did not requested change, as I think the issue it solves outweights the above small inconveniences.
|
@kira-syslogng test this please test branch=pzolee-splunk; |
|
Build SUCCESS |
Kokan
approved these changes
Nov 13, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The messages are ACKed by the
_process_result()method.UnACKed messages are move back to the queue to be sent by syslog-ng
later.
Problems the patch fixes in batching mode:
when a queue contains unsent items during a stop request, worker
threads are blocked until the batch timer not elapsed
when the queue contains items during reload (less then a
batch_lines), then we perform a flush after the worker thread exits, but as the result was not processed, these messages are never ACKed, so they were moved back to the queue, and could be sent multiple timesSigned-off-by: Laszlo Budai laszlo.budai@balabit.com