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: interrupt downloading segment #5847

Merged
merged 1 commit into from
Aug 11, 2022

Conversation

LenaAn
Copy link
Contributor

@LenaAn LenaAn commented Aug 4, 2022

Cover letter

cloud_storage::remote may download a big segment and redpanda will not
be stopped until the segment is fully downloaded. This commit adds an
abort source that is passed to the offset_translator.
partition_recovery_manager may interrupt segment download with this
abort source.

Backport Required

  • not a bug fix
  • papercut/not impactful enough to backport
  • v22.2.x
  • v22.1.x
  • v21.11.x

UX changes

Describe in plain language how this PR affects an end-user. What topic flags, configuration flags, command line flags, deprecation policies etc are added/changed.

Release notes

  • none

@LenaAn LenaAn marked this pull request as ready for review August 4, 2022 20:25
@piyushredpanda piyushredpanda requested review from abhijat and removed request for VadimPlh August 5, 2022 03:18
src/v/storage/parser.cc Outdated Show resolved Hide resolved
@LenaAn LenaAn force-pushed the stream_copy branch 2 times, most recently from 91fd714 to 70447cf Compare August 5, 2022 13:08
@@ -386,13 +391,15 @@ class copy_helper {
ss::output_stream<char> _output;
record_batch_transform_predicate _pred;
model::record_batch_header _header{};
opt_abort_source_t _as;
};

ss::future<result<size_t>> transform_stream(
Copy link
Member

Choose a reason for hiding this comment

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

I think that in the end the abort_source is not passed in any of the transform_streams calls

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's passed in offset_translator::copy_stream

@mmaslankaprv
Copy link
Member

Please clean up not needed includes. Also would be great to add a test where we drop all packets going out to minio and stop redpanda during this period.

@LenaAn
Copy link
Contributor Author

LenaAn commented Aug 8, 2022

Please clean up not needed includes. Also would be great to add a test where we drop all packets going out to minio and stop redpanda during this period.

it doesn't stop just yet, will investigate further why

UPD: it should stop after #5905

cloud_storage::remote may download a big segment and redpanda will not
be stopped until the segment is fully downloaded. This commit adds an
abort source that is passed to the offset_translator.
partition_recovery_manager may interrupt segment download with this
abort source.
@LenaAn
Copy link
Contributor Author

LenaAn commented Aug 10, 2022

Waiting until we cut 22.2.x to merge this

@LenaAn
Copy link
Contributor Author

LenaAn commented Aug 10, 2022

CI failure is #5079

@LenaAn
Copy link
Contributor Author

LenaAn commented Aug 11, 2022

CI failure is #5358

@LenaAn LenaAn merged commit 7b465d0 into redpanda-data:dev Aug 11, 2022
@LenaAn
Copy link
Contributor Author

LenaAn commented Aug 11, 2022

/backport v21.11.x

@vbotbuildovich
Copy link
Collaborator

Failed to run cherry-pick command. I executed the below command:

git cherry-pick -x af30270f16604ec0b90d3ba497edea94b6110456

Workflow run logs.

@LenaAn
Copy link
Contributor Author

LenaAn commented Aug 11, 2022

/backport v22.1.x

@vbotbuildovich
Copy link
Collaborator

Failed to run cherry-pick command. I executed the below command:

git cherry-pick -x af30270f16604ec0b90d3ba497edea94b6110456

Workflow run logs.

@LenaAn
Copy link
Contributor Author

LenaAn commented Aug 11, 2022

will backport to 22.2 after we publish release

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.

4 participants