Skip to content

Support streaming writes against HTTP backends#104

Merged
bbockelm merged 10 commits intoPelicanPlatform:mainfrom
whwjiang:main
Aug 23, 2025
Merged

Support streaming writes against HTTP backends#104
bbockelm merged 10 commits intoPelicanPlatform:mainfrom
whwjiang:main

Conversation

@whwjiang
Copy link
Contributor

@whwjiang whwjiang commented Jun 26, 2025

Remaining work:

  • Write unit tests
  • Figure out if we need to explicitly close the connection ("flush") on close (the file size tells curl to close the connection, but XRootD may not know the file size in which case we do need to close explicitly)

Previously, writing files to HTTP backends didn't really work. With this addition, we should be able to support a multitude of writes, including:

  • 0 byte files
  • Files that occupy a single XRootD write buffer length (approximately 1 MB)
  • Files that occupy more than one write buffer

To achieve the last item, we have a single libcurl operation that we keep alive for the lifetime of the file. That is to say, if XRootD issues something like open() -> write() -> write() -> ... -> write() -> close(), each of the writes in the chain correspond to a single HTTP PUT call, and the connection to the other side is persisted until the file has been completely sent. This is different from our implementation for S3 PUTs, in which case each "chunk" constitutes its own HTTP PUT request.

This work was done as a precursor to supporting Globus backends in Pelican.

@rw2
Copy link
Collaborator

rw2 commented Jun 27, 2025

Hi @whwjiang, Justin asked me to take a look at this. Which I will soon. Until then, let me know if you need any help with the unit testing part. It's proven very useful, but isn't always super easy or intuitive to get running.

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.

Various small nitpicks. The biggest change would be to determine whether you keep the final flag (and, if not, adjust the error conditions in Open appropriately).

.gitignore Outdated
@@ -1 +1,2 @@
build/
.vscode/ No newline at end of file
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please do this as a separate logical commit (and add in the EOL).

}

bool HTTPRequest::SendHTTPRequest(const std::string &payload) {
bool HTTPRequest::SendHTTPRequest(const std::string &payload, bool final) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unclear if the final is needed since libcurl knows how large the request should be. Please give it a try without now that we've got more experience with the branch.

src/HTTPFile.cc Outdated
if (m_write && !m_write_offset) {
HTTPUpload upload(m_hostUrl, m_object, m_log, m_oss->getToken());
if (!upload.SendRequest(nullptr)) {
m_log.Emsg("HTTPFile::Close",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Switch to Log here as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@whwjiang whwjiang force-pushed the main branch 2 times, most recently from 561c768 to e1a3ed6 Compare July 2, 2025 19:56
@whwjiang whwjiang requested a review from bbockelm July 7, 2025 21:50
@whwjiang whwjiang marked this pull request as ready for review July 7, 2025 21:55
@bbockelm
Copy link
Collaborator

@whwjiang - can you please rebase with the various formatting fixes noted above included?

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@whwjiang whwjiang mentioned this pull request Aug 6, 2025
4 tasks
whwjiang and others added 2 commits August 8, 2025 14:25
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

We're good! Let's go...

@bbockelm bbockelm merged commit 08a8edc into PelicanPlatform:main Aug 23, 2025
3 checks passed
turetske added a commit to turetske/xrootd-s3-http that referenced this pull request Oct 20, 2025
This reverts commit 08a8edc, reversing
changes made to d8c59a8.
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.

3 participants