Cache has no eviction policy - when it fills up, it never changes

logstash-plugins / logstash-filter-geoip

Apache License 2.0

64 stars 80 forks source link

Cache has no eviction policy - when it fills up, it never changes #102

Open prehor opened 7 years ago

prehor commented 7 years ago

It seems that geoip plugin has inconsistent cache size option:

Docs says that we should use option lru_cache_size only (see https://www.elastic.co/guide/en/logstash/current/plugins-filters-geoip.html)
Geoip plugin declares two options for cache size setting: cache_size and lru_cache_size (see https://github.com/logstash-plugins/logstash-filter-geoip/blob/master/lib/logstash/filters/geoip.rb#L107)
Geoip plugin uses only cache_size option (see https://github.com/logstash-plugins/logstash-filter-geoip/blob/master/lib/logstash/filters/geoip.rb#L143), lru_cache_size option is not used anywhere.

joewreschnig commented 7 years ago

This seems to have come to pass because the cache is no longer an LRU cache for GeoIP2. So cache_size in lieu of lru_cache_size is probably correct and the documentation is incorrect. But there's also still an underlying problem of how to evict entries.

I've opened an issue with MaxMind to see if it's possible to implement an eviction policy in their default cache.

prehor commented 5 years ago

The question which cache size argument we should use was resolved by @jsvd in #149.

We still have to solve the cache eviction, so I changed the subject of this issue.