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

Upload chunks as separate files #981

Merged
merged 1 commit into from
Nov 11, 2020

Conversation

lubosmj
Copy link
Member

@lubosmj lubosmj commented Oct 23, 2020

closes #4498

@pulpbot
Copy link
Member

pulpbot commented Oct 23, 2020

Attached issue: https://pulp.plan.io/issues/4498

@lubosmj lubosmj force-pushed the not-onlys3-chunked-uploads-4498-2 branch from 8d1a128 to 9a199f4 Compare October 23, 2020 16:56
@lubosmj
Copy link
Member Author

lubosmj commented Oct 23, 2020

This pull request is the same as #914. The only difference here is that the settings file is not changed and therefore no compatibility issues should persist (#973).

@daviddavis
Copy link
Contributor

So this would store chunks in /var/lib/pulp/upload/ by default?

@lubosmj
Copy link
Member Author

lubosmj commented Oct 23, 2020

@daviddavis, yes, to the place where uploads were stored before. Do you suggest me to create a new setting called CHUNKED_UPLOAD_DIR_NEW which will point to /var/lib/pulp/chunks/?

@daviddavis
Copy link
Contributor

No I think /var/lib/pulp/upload is good.

@@ -0,0 +1 @@
Made uploaded chunks to be stored as separated files in the default storage.
Copy link
Member

Choose a reason for hiding this comment

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

@daviddavis Should we mention, that this finally lifts the requirement that the api-nodes have a shared storage?

@lubosmj lubosmj force-pushed the not-onlys3-chunked-uploads-4498-2 branch 3 times, most recently from 2ac170e to 2bd206a Compare October 30, 2020 09:38
@lubosmj lubosmj requested a review from mdellweg October 30, 2020 10:07
@@ -0,0 +1,3 @@
Made uploaded chunks to be stored as separate files in the default storage. This feature allows
Pulp nodes to have a shared storage when dealing with clustered deployment. Each node can now
access all uploaded chunks as they were uploaded directly to that node.
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 it is: This feature removes the need for a share storage of pulp api nodes, as the chunks are now stored individually in the shared storage and are therefore accessible by all nodes.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, okay, I will reword it. Thank you!

@lubosmj lubosmj force-pushed the not-onlys3-chunked-uploads-4498-2 branch from 2bd206a to b35c4ab Compare October 30, 2020 13:18
@lubosmj lubosmj requested a review from mdellweg October 30, 2020 13:44
@@ -0,0 +1,3 @@
Made uploaded chunks to be stored as separate files in the default storage. This feature removes
the need for a share storage of pulp api nodes, as the chunks are now stored individually in the
shared storage and are therefore accessible by all nodes.
Copy link
Member

Choose a reason for hiding this comment

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

👍

@@ -22,7 +25,14 @@ def commit(upload_id, sha256):
log.info(_("The upload was not found. Nothing to do."))
return

file = files.PulpTemporaryUploadedFile.from_file(upload.file.file)
chunks = models.UploadChunk.objects.filter(upload=upload).order_by("offset")
Copy link
Member

Choose a reason for hiding this comment

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

Probably not in this pr, but can we add a check, that the chunks uploaded neither overlap nor fail to cover the whole file?
The overlap check can be added to where we save the upload chunk, and then a "sum of all chunk sizes == expected file size" should be sufficient.


def delete(self, *args, **kwargs):
"""
Delete UploadChunk model and the file associated with the model
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@daviddavis daviddavis merged commit 1b6c736 into pulp:master Nov 11, 2020
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.

None yet

4 participants