Added fsyncEnabled option to the config #1425
Conversation
|
Please rebase, |
| @@ -121,6 +121,11 @@ splunkPlatform: | |||
| enabled: false | |||
| storagePath: "/var/addon/splunk/exporter_queue" | |||
|
|
|||
| # Option to disable or enable fsync for filestorage extension used by the agent. Enabling this option will ensure | |||
| # database integrity at the cost of performance. | |||
There was a problem hiding this comment.
Do you know what the performance cost is by any chance?
There was a problem hiding this comment.
Not really, performance is mentioned here:
https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/extension/storage/filestorage#file-storage
I think that this is strange since our filestorage extension sets fsync to false by default (so nosync in bbolt is set to true), but if we dig more into that, the recommendation is to keep nosync flag as false. As seen here:
https://pkg.go.dev/go.etcd.io/bbolt#DB
This makes sense as we now know that this flag caused some problems for our customers.
I think that we should expose this flag, keep its value as is (so fsync is set to false), and maybe do some performance tests to see if we can change it in the future.
There was a problem hiding this comment.
OK so 2 follow-ups:
- one issue in contrib to have performance test benchmarks with and without this setting
- one issue in contrib to have this setting changed to the right default per bbolt.
Would you like to file those issues?
sbylica-splunk
force-pushed
the
add_fsync_flag_config
branch
2 times, most recently
from
f04b848 to
ed1d442
Compare
sbylica-splunk
force-pushed
the
add_fsync_flag_config
branch
from
ed1d442 to
c1d9131
Compare
Description: Added fsyncEnabled flag, it is passed to file storage used by the agent. This is related to the issue:
https://splunk.atlassian.net/browse/ADDON-71899
With this flag enabled we can avoid issues with data corruption in the future.
Testing: Manual testing
Documentation: Flag is described in values.yaml