HS: Prune stale cache files from disk v2 #14212
Conversation
To have a system-level overview of when was the last time the file was used, update the file modification timestamp to to the current time. This is needed to remove stale cache files of the system. Access time is not used as it may be, on the system level, disabled. Ticket: 7830
Ticket: 7830
Hyperscan MPM can cache the compiled contexts to files. This however grows as rulesets change and leads to bloating the system. This addition prunes the stale cache files based on their modified file timestamp. Ticket: 7830
lukashino
requested review from
a team,
jasonish and
victorjulien
as code owners
| base64 = "~0.22.1" | ||
| bendy = { version = "~0.3.3", default-features = false } | ||
| asn1-rs = { version = "~0.6.2" } | ||
| parse_duration = "~2.1.1" |
There was a problem hiding this comment.
I don't think this is maintained anymore. Look at humantime instead.
There was a problem hiding this comment.
yup, would need to be replaced.
There was a problem hiding this comment.
I didn't know but cargo audit told me so :D
Thanks for the suggestion
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #14212 +/- ##
==========================================
- Coverage 84.17% 84.14% -0.03%
==========================================
Files 1013 1013
Lines 262316 262494 +178
==========================================
+ Hits 220800 220877 +77
- Misses 41516 41617 +101
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
| filename, sizeof(filename), "%020" PRIu64 "%s", hs_db_hash, hash_file_path_suffix); | ||
| if (r != (uint64_t)(20 + strlen(hash_file_path_suffix))) | ||
| filename, sizeof(filename), "%s_%s%s", sensor_name, hs_db_hash, hash_file_path_suffix); | ||
| if (r != (uint64_t)(strlen(sensor_name) + 1 + strlen(hs_db_hash) + | ||
| strlen(hash_file_path_suffix))) |
There was a problem hiding this comment.
What happens if my sensor name is "foo/bar"? Or "../../../../../etc/"? :) I don't think we have any restrictions on the sensor-name, but if using in a path we probably need to. One option is to create a hash from the name.
There was a problem hiding this comment.
Good point, that would need to be addressed. I could take the sensor name as one of the things I hash with SHA256 so we wouldn't even need to extend the naming scheme (to <sensor_name>__v1.hs)
|
Information: QA ran without warnings. Pipeline = 28174 |
|
Decision made: Sensor name won't be part of the cache file name. |
|
new in #14270 |
4 participants
Follow-up of #13850
Link to ticket: https://redmine.openinfosecfoundation.org/issues/7830
Describe changes:
v2:
v1:
The logic to determine a stale file is currently based on the modification timestamp in the file systems. The accessed time stamp was not used as it may be switched off. Alternatively, we could use a local DB/notekeeping file of the last used files/caches but this approach seemed simpler.
I can also add GitHub CI tests, I thought of some scenarios.
Decision required:
Multiple instances, 1 cache folder
In #13850 we ran into a potential problem, where when multiple instances share one cache folder, one might be just creating the HS cache and the other instance will attempt to load it. Or we might have multiple writers to one file.
This can be easily mitigated by either specifying unique sensor names or not sharing the same folder with each instance. These mitigations, however, miss out on the use case where 1 folder shared by many instances can save the load time because other instances hit the precompiled cache files. The question here is whether we want to allow it.
I looked at solving this, and we can draw inspiration from one of the package managers.
Jason then also suggested the use of the
flock()system call, which is also promising, but we need to ensure there is 100% compatibility across operating systems and file systems. It is not supported by Windows, but Windows has an alternative way of doing this.My current idea was to (when the cached file is not present):
Perhaps the - meaning random - file should also have its lock file in the very unlikely case when multiple instances come up with the same name
If we want to proceed with this deployment scenario, we need a ticket.