-
Notifications
You must be signed in to change notification settings - Fork 24.7k
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
Fix geo ip database file leak when processing IP arrays #93177
Conversation
Pinging @elastic/es-data-management (Team:Data Management) |
Hi @jbaiera, I've created a changelog YAML for you. |
@elasticmachine run elasticsearch-ci/part-1 |
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.
LGTM. Good catch! It seems odd to me that the call to preLookup
happens (we hope) in the supplier (DatabaseReaderLazyLoader lazyLoader = this.supplier.get();
) but the call to postLookup
is explicit. Since the supplier is given to the constructor, it seems possible that a supplier might not ever even call preLookup
. We probably ought to fix that at some point. But that is preexisting weirdness not introduced by this PR (and out of scope for it).
💚 Backport successful
|
This PR moves the postLookup call out of the lookup methods in order to correctly decrement the instance counter a single time at the end of the processor call.
The
DatabaseReaderLazyLoader
class keeps track of how many clients are using it at a time. This allows the reader to be cleaned up as soon as the last client using it is complete. This tracking logic takes place by balancing apreLookup
method with apostLookup
method on each Geo IP processor invocation. The problem arises when processing an array of IPs. Currently thepostLookup
method is called for each IP in the array, which can cause the reader to decrement incorrectly. If a new database is installed during processor execution the reader can get stuck open, leaving database files on disk when they should be cleaned up.This PR moves the
postLookup
call out of the lookup methods in order to correctly decrement the instance counter a single time at the end of the processor call.