Skip to content

Conversation

@PeteGillinElastic
Copy link
Member

This adds the option to have a geoip cache with a byte-based rather than a count-based capacity.

@PeteGillinElastic PeteGillinElastic force-pushed the ES-7713-geoip-cache-byte-capacity branch from 3692033 to 6fb2ec6 Compare June 10, 2025 15:37
This adds the option to have a geoip cache with a byte-based rather
than a count-based capacity.
@PeteGillinElastic PeteGillinElastic force-pushed the ES-7713-geoip-cache-byte-capacity branch from 6fb2ec6 to 2ca311c Compare June 10, 2025 15:51
…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).
*/
@Nullable
<RESPONSE> RESPONSE getResponse(String ipAddress, CheckedBiFunction<Reader, String, RESPONSE, Exception> responseProvider);
<RESPONSE extends GeoIpCache.CacheableValue> RESPONSE getResponse(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not have any changes to this file, and eat the subsequent consequences.

… them public, just so we can use them in integration tests. Make the tests use their own dummy city and country objects instead.
@PeteGillinElastic PeteGillinElastic changed the title [WIP] Add option for byte-based capacity for geoip cache [WIP] ES-11278 Add option for byte-based capacity for geoip cache Jun 17, 2025
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.

3 participants