franz-go icon indicating copy to clipboard operation
franz-go copied to clipboard

Optimistically cache mapped metadata when cluster metadata is periodically refreshed

Open pracucci opened this issue 1 month ago • 0 comments

In Mimir, we have a process creating 1 Client (setting MetadataMinAge = MetadataMaxAge = 10s`) and then using it to do two things:

  1. Consume a partition
  2. Request ListOffsets every 1s

I noticed that such Client issues 2 Metadata requests every MetadataMinAge (remember that we set MetadataMaxAge = MetadataMinAge).

One request is due to the periodic Metadata refresh, and one is due to ListOffsets which need Metadata as well. ListOffsets request calls Client.fetchMappedMetadata() which caches the metadata for MetadataMinAge, so effectively leading to 2 Metadata requests every MetadataMinAge.

In this PR I'm proposing to optimistically cache the mapped metadata when cluster metadata is periodically refreshed.

Notes

  • I'm not an expert of this client, so I may miss something obviously wrong in this PR
  • The fetchMappedMetadata() comment says "this is garbage heavy, so it is only used in one off requests in this package". Part of the the garbage is introduced by caching the mapped metadata, which is now done each time metadata is refreshed. Do you see any issue?
  • I'm not a big fan of passing a callback function to storeCachedMappedMetadata(), but I haven't come up with a better idea which either doesn't make code more complex or introduce more allocations. Thoughts?
  • I've tested it for our use case and the number of Metadata requests dropped from 2 to 1 every MetadataMinAge

pracucci avatar May 09 '24 15:05 pracucci