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]: storage::log interface self-honesty #12596

Merged
merged 10 commits into from
Aug 7, 2023

Conversation

dotnwat
Copy link
Member

@dotnwat dotnwat commented Aug 4, 2023

We've been hiding behind dynamic_cast instead of being honest with ourselves about the unpleasant parts of the log interface. This PR changes that by surfacing the external interfaces to storage::log (it's really not so bad). In any case, the disk_log_builder is now the only remaining user of dynamic_cast<disk_log_impl> as it uses a bunch of low-level interfaces.

This change is important as we look towards evolving the local log implementation--we need to be able to clearly see the interface we are trying to support.

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.2.x
  • v23.1.x
  • v22.3.x

Release Notes

  • none

dotnwat added 9 commits August 3, 2023 16:33
Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
Exposes the segment set through the log interface, and removes the
dynamic_cast to disk_log_impl. The segment is forward declared as its
header includes some bits that wreck compilation due to naming conflicts
in other parts of the tree. This will be addressed later.

Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
Exposes the probe. The probe is forward declared because the probe.h
header brings a long some bits that wreck compilation in a few places
with naming clases, which will be fixed up later.

Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
@dotnwat
Copy link
Member Author

dotnwat commented Aug 4, 2023

Comment on lines 415 to 417
vlog(
archival_log.warn,
"Upload policy find next compacted segment: cannot find log for ntp: "
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this warning message change, as the condition has changed?

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks. fixed

Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
@dotnwat dotnwat requested a review from abhijat August 4, 2023 15:24
@dotnwat
Copy link
Member Author

dotnwat commented Aug 4, 2023

@dotnwat dotnwat merged commit 868c76e into redpanda-data:dev Aug 7, 2023
ztlpn added a commit that referenced this pull request Oct 3, 2023
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.

3 participants