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

Remove content length from upload to enable chunked #208

Merged
merged 1 commit into from
Dec 1, 2023

Conversation

djw8605
Copy link
Contributor

@djw8605 djw8605 commented Oct 10, 2023

Removing the content length from a put changes the upload style from full file upload to a chunked encoded upload. A full file upload causes go to read in the entire file in memory while uploading, chunked does not.

NOTE chunked uploads only work after xrootd has merged and put into production xrootd/xrootd#2102

Corresponds to htcondor/osdf-client#88

@bbockelm
Copy link
Collaborator

@djw8605 - is there a way to check for server compatibility? What happens when you try a chunk encoded PUT against an older server?

@djw8605
Copy link
Contributor Author

djw8605 commented Oct 11, 2023

I suppose you could do a separate request to the xrootd server to determine the version.

If you do a chunked encoded PUT against an older (well, current as of now) server, it may or may not work. It all depends on timing, buffers, bandwidth... The larger the file, the more of a chance of failure. Though, even small files frequently failed in my tests.

@bbockelm bbockelm added the enhancement New feature or request label Oct 29, 2023
@bbockelm bbockelm added this to the v7.3.0 milestone Oct 29, 2023
@bbockelm
Copy link
Collaborator

@djw8605 - I'm tempted to merge this one. The relevant server version has been released for about a month and I haven't seen any major complaints. If we merge now, it'd make it into the v7.3.0 (early December) release, giving a few weeks for any straggling origins to upgrade.

Thoughts?

@djw8605 djw8605 marked this pull request as ready for review November 27, 2023 17:24
@djw8605
Copy link
Contributor Author

djw8605 commented Nov 27, 2023

Sounds good to me, setting for ready for review.

@turetske
Copy link
Collaborator

turetske commented Nov 29, 2023

I suggest doing a rebase on main and pushing in order to run all the tests again with updated code. It's been over two months and many things have changed since then and I'd like the extra sanity check before merging in (I doubt it would fail, but just in case).

Removing the content length from a put changes the upload style from full file upload to a chunked encoded upload. A full file upload causes go to read in the entire file in memory while uploading, chunked does not.

**NOTE** chunked uploads only work after xrootd has merged and put into production xrootd/xrootd#2102

Corresponds to htcondor/osdf-client#88
Copy link
Collaborator

@bbockelm bbockelm left a comment

Choose a reason for hiding this comment

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

LGTM. We'll coordinate with existing origins to ensure they're at 5.6.3.

@bbockelm bbockelm merged commit 8fcc766 into PelicanPlatform:main Dec 1, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants