In read mode, request sincedb disk flush on each read loop iteration. #196
Conversation
The actual write occurs at the `sincedb_write interval`. Note: the chunk size vs the events-per-second affect how often a request is made. e.g. a large chunk size (1MB), lines of say 128 bytes and 256 events/s means each iteration takes 32 seconds, and an eps of 32768 takes 250 ms.
| @@ -28,6 +28,9 @@ def handle_specifically(watched_file) | |||
| watched_file.file_close | |||
| # unset_watched_file will set sincedb_value.position to be watched_file.bytes_read | |||
| sincedb_collection.unset_watched_file(watched_file) | |||
| # we know we're done, we should not wait until the discovery of a | |||
| # new file (or LS shutdown) to write out the sincedb | |||
| sincedb_collection.immediate_write | |||
There was a problem hiding this comment.
since we're now guaranteed to write the sincedb at exit, should we still keep this change of forcing sincedb writing at EOF?
There was a problem hiding this comment.
Yes I think so.
If not then consider the case where one has say 5 small files and a default sincedb write interval of 15 seconds:
- Each file has 10 read loops, 10
request_disk_flushwere made and none are granted as all files are processed in less than 15 seconds. - The 11th loop is an immediate EOF.
- No further write requests are made, only LS exit will force the write. This is a vulnerable situation, say in a docker container (with persistent disks) that is killed (because we don't have the
read all files and exitfeature yet). - The user is forced to understand enough to set a very low
sincedb write interval - The discover/stat loop continues to discover and stat though .
Before I had a nagging feeling about changing the request_disk_flush into setting a flag to true before checking the interval. This flag is set to false when the file is written. Then adding a call in the iterate_on_state method to disk_flush_if_needed (if the flag is set and the interval is exceeded).
I could not see the logic before but now after these changes and the SDH I feel this is a better change.
WDYT?
There was a problem hiding this comment.
I understand the scenario, but IMO it is an edge case that could be worked around by terminating logstash gracefully before destroying the docker container.
Thinking the other way around, what about the downside to this? Calling a sincedb write on every EOF can be slow in scenarios of multiple small files, creating a large sincedb that needs to be rewritten on each EOF.
Do think the tradeoff is worth it?
There was a problem hiding this comment.
Well my alternate suggestion (the nagging feeling) would not call immediate_write at EOF.
How about I implement it and you can critique again?
|
There seems to be a failing spec: |
|
I don't know why that test failed. I re-ran it and it is OK now |
The actual write occurs at the
sincedb_write interval.Note: the chunk size vs the events-per-second affect how often a request is made., e.g. a large chunk size (1MB), lines of say 128 bytes and 256 events/s means each iteration takes 32 seconds, and an eps of 32768 takes 250 ms.
With the default chunk size of 32768 bytes and an average line size of 128 bytes a pipeline running at 256 events/s will iterate every 1 second - but with a 32768 event/s the loop takes 7.8125 ms or 128 noop calls to
request_disk_flushevery second.Fixes #195