graylog2-server
graylog2-server copied to clipboard
Search Cleanup Job: Fix interval checking, skip static referenced searches
Description
The SearchCleanupJob
previously used a fixed previous duration (set at in constructor at injection time), which resulted in the periodical only ever cleaning up expired searches once at server startup time (thanks to @ryan-carroll-graylog for noticing this). The migration is intended to perform the cleanup once per hour. This PR fixes that, so that the expired reports threshold mustBeOlderThan
is recalculated on each run, which allows newly expired non-referenced searches to be deleted each time the job runs.
Also adds support for not deleting static referenced search ids. See https://github.com/Graylog2/graylog-plugin-enterprise/pull/7107 for context.
/nocl fixes/adjusts backend process that does not affect visible user functionality
How Has This Been Tested?
Manual testing by overriding the initial delay and run intervals in the migration to get it to execute quicker. Verifying that appropriate static IDs are skipped, unit tests.
Screenshots (if appropriate):
Types of changes
- [x] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [x] Refactoring (non-breaking change)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
Checklist:
- [x] My code follows the code style of this project.
- [ ] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly.
- [ ] I have read the CONTRIBUTING document.
- [x] I have added tests to cover my changes.
Thanks for fixing this @danotorrey! And really like the approach of using the StaticReferencedSearch
binding to preserve searches we don't want cleaned up.
This all looks awesome but just a heads up, I checked out this PR without the enterprise counterpart and I'm getting a Guice error that looks to be because we're never calling PluginModule.addStaticReferencedSearch
and therefore nothing is bound to Set<StaticReferencedSearch> staticReferencedSearches
for the SearchesCleanUpJob
class:
com.google.inject.CreationException: Unable to create injector, see the following errors:
1) [Guice/MissingImplementation]: No implementation for Set<StaticReferencedSearch> was bound.
Requested by:
1 : SearchesCleanUpJob.<init>(SearchesCleanUpJob.java:50)
\_ for 5th parameter
at PluginModule.addPeriodical(PluginModule.java:102)
Not sure if this would be a problem for instances without the security module, maybe we can call staticReferencedSearchBinder
somewhere in graylog2-server where we know it'll always get bound?
Not sure if this would be a problem for instances without the security module, maybe we can call staticReferencedSearchBinder somewhere in graylog2-server where we know it'll always get bound?
@ryan-carroll-graylog @danotorrey I've run into something similar before when working on the Illuminate lookup tables. We should be able to do exactly that somewhere in an open module, similar to here for the new SystemEntity lookup stuff.
@ryan-carroll-graylog @kingzacko1 Nice find! Thanks for the tip. Just added that fix, and it seems to work.
Just force-pushed the same changeset with a fixed email address to resolve the CLA-signing check failure.
@dennisoelkers @moesterheld Do you have any concerns or see any issues with this? Just wanted to check-in before merging.