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

cloud_storage: various non-functional changes #16886

Merged
merged 3 commits into from
Mar 5, 2024

Conversation

andrwng
Copy link
Contributor

@andrwng andrwng commented Mar 4, 2024

Some non-functional changes that I noticed while reading through the read path:

  • renames segment readers to _seg_reader, rather than _parent or _reader
  • removes some dead code

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

Within the remote partition reader, we maintain a segment reader
previously named `_reader`. Since partition reader is also a reader,
it can be confusing to read the code without that context.

This renames it to `_seg_reader`.
@andrwng andrwng changed the title cloud_storage: clarify segment reader variable name cloud_storage: various non-functional changes Mar 4, 2024
@@ -1130,7 +1130,7 @@ class remote_segment_batch_consumer : public storage::batch_consumer {
const model::ntp& ntp,
retry_chain_node& rtc)
: _config(conf)
, _parent(parent)
, _seg_reader(parent)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think the c-tor argument should also be renamed for consistency?

@Lazin
Copy link
Contributor

Lazin commented Mar 5, 2024

could you also please backport this, it touches a lot of code so there could be merge conflicts when we will backport future PRs

@andrwng andrwng merged commit f9af041 into redpanda-data:dev Mar 5, 2024
19 checks passed
@andrwng
Copy link
Contributor Author

andrwng commented Mar 5, 2024

could you also please backport this, it touches a lot of code so there could be merge conflicts when we will backport future PRs

will do

@vbotbuildovich
Copy link
Collaborator

/backport v23.3.x

@vbotbuildovich
Copy link
Collaborator

/backport v23.2.x

@vbotbuildovich
Copy link
Collaborator

Failed to create a backport PR to v23.2.x branch. I tried:

git remote add upstream https://github.com/redpanda-data/redpanda.git
git fetch --all
git checkout -b backport-pr-16886-v23.2.x-459 remotes/upstream/v23.2.x
git cherry-pick -x d3ab4d7356a7434bad840e6c1ef9b0e50b11c7f9 b0166ffb0e7b5e8869ec026c028202b42ea0713e 65478db60eeb83193702c99a37480bfb3649e183

Workflow run logs.

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

4 participants