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

wip: delete pieces when served when streaming #241

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

cocool97
Copy link

@cocool97 cocool97 commented Sep 20, 2024

This PR is a work in progress to allow storage to delete pieces when they have been served in streaming context.

Related to issue #224

@cocool97
Copy link
Author

@ikatson Can you confirm if expected changes seem good to you ? I'll implement it afterwards

@cocool97 cocool97 changed the title wip: add explanations about work to do wip: delete pieces when served when streaming Sep 20, 2024
@ikatson
Copy link
Owner

ikatson commented Sep 20, 2024

I won't feel comfortable merging this. As I think I mentioned previously, streaming with garbage collection is a major design change and can't be easily hacked into the codebase.

For example, given your comment in this PR - it's not good for separation of concerns. Streaming.rs shouldn't be deleting anything or knowing anything about piece garbage collection.

@ikatson
Copy link
Owner

ikatson commented Sep 20, 2024

Also:

on_piece_completed() could thus be replaced by delete_chunk()

Same comment here. librqbit shouldn't be deleting chunks. At most you could provide enough integration points so that your custom Box implementation (e.g. Box, although that won't be enough) can do deletion without the core knowing anything about it, but also not breaking.

@ikatson
Copy link
Owner

ikatson commented Sep 20, 2024

One way to try to make this work is:

  • add "streaming_only: bool" to the torrent.
  • in all the "stream then delete" places check for "streaming_only", and branch accordingly.

At least this would make clear what needs to be changed and where. I'm not saying I'll merge it if it's done this way, but at least will give more room for discussion. Piece-meal tiny PRs aren't enough to show the full picture of how it's all going to work in the end.

Writing up a design doc upfront would also help not waste time.

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.

2 participants