-
Notifications
You must be signed in to change notification settings - Fork 76
Remove min cache size and cache size allocation #828
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove min cache size and cache size allocation #828
Conversation
Good point. The division is due to how the disk buffering config works, though I think it should be straightforward to allow for a different size per folder over there, so I created this issue to work on it. |
| } | ||
|
|
||
| // Divides the available cache size by 3 (for each signal's folder) | ||
| val calculatedSize = requestedSize / 3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably we would no longer need to store this calculation since it's not a heavy one anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@deividasstr want to pick this up in this PR or the next?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! I will do it in this one.
Just in case, I noticed that this value here is used by another sdk, disk buffering. There shouldn't be no negative backwards compatibility consequence to change cache size on existing files, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be fine. The size config number is read before writing to ensure that we won't take more space than needed, and when the used space plus the one that is about to be written goes over said number, disk buffering clears the oldest data before doing the writing. So to your question on what would happen if the cache size changes, if the new value is higher, then the existing oldest data will remain there longer, and if it's shorter, then more older data will be cleared in the next write (unless it gets exported first).
LikeTheSalad
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
breedx-splk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
As per discussions here, it was decided to remove min cache size check along with disk space allocation.
New thought emerged - is dividing requested cache size to three different signals really necessary?
In my own use case, I only want to track metrics for now, maybe later - logs, and traces - not likely. So I would expect that all of my requested cache size is used by signals I need. Or maybe there is some optimization under the hood which handles this?