Skip to content
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

kafka replay speed: use franz-go fork #9203

Conversation

dimitarvdimitrov
Copy link
Contributor

@dimitarvdimitrov dimitarvdimitrov commented Sep 4, 2024

Using a fork

Starts using a fork of franz-go until we can get a version of twmb/franz-go#803 merged upstream. The maintainer was ok with the approach, so I think using a fork won't diverge us in the long run.

As a result this PR also deletes all the copied code we have from franz-go for partition parsing.

Metrics

Using a fork allows us to just use the metrics that kprom exposes instead of tracking them ourselves.

  • cortex_ingest_storage_reader_fetches_compressed_bytes_total which was a copy of kprom's cortex_ingest_storage_reader_fetch_compressed_bytes_total
  • cortex_ingest_storage_reader_fetched_bytes_total which was a copy of kprom's cortex_ingest_storage_reader_fetch_bytes_total

Clean up fetching

  • Properly close spans when we double-fetch a record. Also emit a span event to show that we've ignored the record.

  • We also assert on the response we get instead of indexing into an array and hoping it contains what we asked for. I don't think this check should ever fail in normal operations. It's there as a stopgap to consuming wrong data in case there's a bug somewhere.

Checklist

  • Tests updated.
  • Documentation added.
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX].
  • about-versioning.md updated with experimental features.

Signed-off-by: Dimitar Dimitrov <[email protected]>
Signed-off-by: Dimitar Dimitrov <[email protected]>
Signed-off-by: Dimitar Dimitrov <[email protected]>
Copy link
Contributor

@gotjosh gotjosh left a comment

Choose a reason for hiding this comment

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

LGTM

@dimitarvdimitrov dimitarvdimitrov merged commit a09738b into dimitar/ingester/consume-latency-push-sharding Sep 19, 2024
29 checks passed
@dimitarvdimitrov dimitarvdimitrov deleted the dimitar/ingester/consume-latency-push-sharding-fixups/use-fork-of-franz-go branch September 19, 2024 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants