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

XrdTpc: Add a crude timeout mechanism. #1392

Merged
merged 2 commits into from
Feb 4, 2021

Conversation

bbockelm
Copy link
Contributor

@bbockelm bbockelm commented Feb 3, 2021

This does not enforce a minimum bandwidth (N MB/s) but rather
aborts the transfer if zero data has moved within a given timeframe.
By default, the transfer has 120 seconds to move the first byte of
data and it must transfer at least 1 byte every 60 seconds thereafter.

This purposely does not do bandwidth minimums as that's built into
future versions of curl and I'd prefer to not re-implement it.

The default timeouts can be overridden by setting:

tpc.timeout timeout_val [first_byte_timeout_val]

where the default is equivalent to:

tpc.timeout 60

If the first_byte_timeout_val is not set, it defaults to 2x the
timeout_val.

(Note: I am not targeting the 5.1.0 release for this)

This does not enforce a minimum bandwidth (N MB/s) but rather
aborts the transfer if zero data has moved within a given timeframe.
By default, the transfer has 120 seconds to move the first byte of
data and it must transfer at least 1 byte every 60 seconds thereafter.

This purposely does not do bandwidth minimums as that's built into
future versions of `curl` and I'd prefer to not re-implement it.

The default timeouts can be overridden by setting:

```
tpc.timeout timeout_val [first_byte_timeout_val]
```

where the default is equivalent to:

```
tpc.timeout 60
```

If the `first_byte_timeout_val` is not set, it defaults to 2x the
`timeout_val`.
@abh3
Copy link
Member

abh3 commented Feb 3, 2021

@bbockelm There is an error in this pull request that needs to be corrected:
/home/travis/build/xrootd/xrootd/src/XrdTpc/XrdTpcMultistream.cc: In member function ‘int TPC::TPCHandler::RunCurlWithStreamsImpl(XrdHttpExtReq&, TPC::State&, size_t, std::vectorTPC::State*, TPC::TPCHandler::TPCLogRecord&)’:
/home/travis/build/xrootd/xrootd/src/XrdTpc/XrdTpcMultistream.cc:415:39: error: ‘mres’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
<< curl_multi_strerror(mres);
^
cc1plus: all warnings being treated as errors
src/CMakeFiles/XrdHttpTPC-5.dir/build.make:75: recipe for target 'src/CMakeFiles/XrdHttpTPC-5.dir/XrdTpc/XrdTpcMultistream.cc.o' failed

This triggered a compiler warning about potential use of an uninitialized
variable; I don't think that was possible but it would be impossible
for the compiler to know that!
@bbockelm
Copy link
Contributor Author

bbockelm commented Feb 3, 2021

It's a reasonable call by the compiler ... not likely to have ever been triggered but reasonable to fix!

@abh3
Copy link
Member

abh3 commented Feb 3, 2021

Thanks Brian, waiting for the CI to finish.

@abh3 abh3 merged commit 8147a4a into xrootd:master Feb 4, 2021
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

2 participants