Skip to content

Conversation

@PeteGillinElastic
Copy link
Member

A variant on #129207 to illustrate a point.

PeteGillinElastic and others added 21 commits June 10, 2025 16:51
This adds the option to have a geoip cache with a byte-based rather
than a count-based capacity.
…nd `GeoIpCache.CacheableValue`. This will be needed to implement the `sizeInBytes()` method on `Result`.

The code in `GeoIpTestUtils` has to change a little bit, because previously we had `Result<CityResponse>` and we don't own `CityResponse` so we can't make it implement the interface (and the same for `CountryResponse`). This change makes the test helper act a bit more like the production code, which is probably a good thing.
This version of the code does *not* affect the default behaviour. I'm
not sure whether that would be considered a breaking change. (We could
override the default in serverless, which I am sure we're free to do.)
It would obviously be simple to flip the logic around so that we do
change the default to, say, 1% of heap (like we do with enrich).
… them public, just so we can use them in integration tests. Make the tests use their own dummy city and country objects instead.
…ires us to make sure that we only ever call getResponse() with types which satisfy the constraint. It allows us to not touch IpDatabase.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants