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

storage: add log_reader option to translate offsets #17384

Merged
merged 1 commit into from
Mar 29, 2024

Conversation

andrwng
Copy link
Contributor

@andrwng andrwng commented Mar 26, 2024

Previously offset translation on the read path was done in the Kafka layer, with the storage layer providing primitives to help translate. As we clarify the requirements of the storage log implementation, offset translation has made its way into the purview of storage.

To allow the storage layer to own offset translation end-to-end on the read path, and allow for future log implementations that translate offsets differently (e.g. by storing translated offsets), this adds an option to the log reader to translate. The corresponding code is removed from the Kafka layer.

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v23.3.x
  • v23.2.x

Release Notes

  • none

@vbotbuildovich
Copy link
Collaborator

vbotbuildovich commented Mar 26, 2024

new failures in https://buildkite.com/redpanda/redpanda/builds/46771#018e79be-3214-40fa-b530-9da74449b1e6:

"rptest.tests.data_transforms_test.DataTransformsTest.test_identity.transactional=True"
"rptest.tests.data_transforms_test.DataTransformsLoggingMetricsTest.test_logger_metrics_values"

new failures in https://buildkite.com/redpanda/redpanda/builds/46771#018e79be-3211-4e52-ad63-917d85052941:

"rptest.tests.data_transforms_test.DataTransformsTest.test_identity.transactional=False"

new failures in https://buildkite.com/redpanda/redpanda/builds/46771#018e79be-320e-4313-8d42-adfeacb48e14:

"rptest.tests.data_transforms_test.DataTransformsLeadershipChangingTest.test_leadership_changing_randomly"

new failures in https://buildkite.com/redpanda/redpanda/builds/46771#018e79be-3217-41d6-ac3b-2a4429ae34c5:

"rptest.tests.data_transforms_test.DataTransformsChainingTest.test_multiple_transforms_chained_together"

new failures in https://buildkite.com/redpanda/redpanda/builds/46771#018e79d0-002c-4cec-be97-2d1befcdef74:

"rptest.tests.data_transforms_test.DataTransformsChainingTest.test_multiple_transforms_chained_together"

new failures in https://buildkite.com/redpanda/redpanda/builds/46771#018e79d0-0024-4d45-8afa-7ac0caa312ae:

"rptest.tests.data_transforms_test.DataTransformsLeadershipChangingTest.test_leadership_changing_randomly"

new failures in https://buildkite.com/redpanda/redpanda/builds/46771#018e79d0-0027-4768-9931-04d2a3feb466:

"rptest.tests.data_transforms_test.DataTransformsTest.test_identity.transactional=False"

new failures in https://buildkite.com/redpanda/redpanda/builds/46771#018e79d0-0029-4b1f-8b55-7018f42f1f43:

"rptest.tests.data_transforms_test.DataTransformsTest.test_identity.transactional=True"
"rptest.tests.data_transforms_test.DataTransformsLoggingMetricsTest.test_logger_metrics_values"

@andrwng andrwng force-pushed the storage-log-reader-translation branch from 1e1fa1f to 30b77e9 Compare March 28, 2024 03:29
@andrwng andrwng requested review from dotnwat, mmaslankaprv and ztlpn and removed request for dotnwat March 28, 2024 07:44
src/v/storage/types.h Outdated Show resolved Hide resolved
Previously offset translation on the read path was done in the Kafka
layer, with the storage layer providing primitives to help translate.
As we clarify the requirements of the storage log implementation, offset
translation has made its way into the purview of storage.

To allow the storage layer to own offset translation end-to-end on the
read path, and allow for future log implementations that translate
offsets differently (e.g. by storing translated offsets), this adds an
option to the log reader to translate. The corresponding code is removed
from the Kafka layer.
@andrwng andrwng force-pushed the storage-log-reader-translation branch from 30b77e9 to b4f54b1 Compare March 28, 2024 20:50
@andrwng andrwng merged commit 67e446b into redpanda-data:dev Mar 29, 2024
17 checks passed
Comment on lines +441 to +448
if (_config.translate_offsets) {
vassert(
_translator, "Expected offset translactor to be initialized");
for (auto& b : batches) {
b.header().base_offset = _translator->from_log_offset(
b.base_offset());
}
}
Copy link
Member

Choose a reason for hiding this comment

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

nice. i wonder if at some point this could become cheap enough that'd we could always translate the offset into a new b.translated_offset()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah in the future once getting the translated offset is a matter of reading deserializing the metadata from an entry, it seems plausible that that offset gets put into the ctx as well, along with the term.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants