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

V5.7.1 #18

Merged
merged 93 commits into from
Sep 9, 2024
Merged

V5.7.1 #18

merged 93 commits into from
Sep 9, 2024

Conversation

Jo-stfc
Copy link

@Jo-stfc Jo-stfc commented Sep 9, 2024

No description provided.

amadio and others added 30 commits July 2, 2024 11:15
This for the CI to work for a while longer after CentOS 7 EOL.
After the move to cern/cc7-base image, RPMs have an extra .cern
appended to them, which comes from '%{dist}', and this breaks the
artifact upload step, as RPMs are moved to a non-existent directory.
Moving the RPMs into RPMS/ directory directly avoids this problem.
/builddir/build/BUILD/xrootd-5.7.0/src/XrdCl/XrdClClassicCopyJob.cc:722:35: error: ‘virtual XrdCl::XRootDStatus {anonymous}::XRootDSource::GetXAttr(std::vector<std::tuple<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >&)’ was hidden [-Werror=overloaded-virtual=]
  722 |       virtual XrdCl::XRootDStatus GetXAttr( std::vector<XrdCl::xattr_t> &xattrs )
      |                                   ^~~~~~~~
/builddir/build/BUILD/xrootd-5.7.0/src/XrdCl/XrdClClassicCopyJob.cc:1153:35: note:   by ‘virtual XrdCl::XRootDStatus {anonymous}::XRootDSourceZip::GetXAttr(std::vector<XrdCl::XAttr>&)’
 1153 |       virtual XrdCl::XRootDStatus GetXAttr( std::vector<XrdCl::XAttr> &xattrs )
      |                                   ^~~~~~~~
Leave connection open after redirection if keepalive is enabled
The reset function prepares the object for another request, including
clearing the keepalive variable. Save the keepalive value when it's
needed as a return value.
If the file-open failed to finalize (this code path is hit in XCache
when a non-existent file is attempted to be opened), then the `fp`
object leaked.

This eventually fills up the file descriptor table in XrdPosix after
1M file-not-found responses from the cache, a denial of service until
the xrootd server is restarted.
The original commit could return a value < -1 which was incorrect and could
cause additional errors while not actualy deleting the faulty file pointer.
The detailed comments in the code describe the logic that needed to be used.
…ixes xrootd#2288

With the addition of a "hush" flag for ForceDisconnect() this update uses
the flag to suppress the error message generated for each disconnect which
in a proxy server context is perfectly normal to prevent connection reuse.
Fixes the following warning:

XrdApps/XrdAppsCconfig.cc:91:9: warning: ignoring return value of
'ssize_t write(int, const void*, size_t)' declared with attribute
'warn_unused_result' [-Wunused-result]
   91 |    write(fd, cFile.c_str(), cFile.length());

Issue: xrootd#1894
The macros use EXPECT_TRUE internally rather then ASSERT_TRUE, so
having names containing ASSERT is inconsistent with their behaviour.

Rename GTEST_ASSERT_ERRNO       → EXPECT_ERRNO_OK
Rename GTEST_ASSERT_PTHREAD     → EXPECT_PTHREAD_OK
Rename GTEST_ASSERT_XRDST       → EXPECT_XRDST_OK
Rename GTEST_ASSERT_XRDST_NOTOK → EXPECT_XRDST_NOTOK
Example errors seen when a test uses the same file or directory name
as an earlier test and the file system has not synced the removal of
the previous version:

[ERROR] Server responded with an error: [3018] Unable to create directory; it already exists.
[ERROR] Server responded with an error: [3018] Unable to create file; it already exists.
Occasionally tests fail because the servers are not ready:

Example:

[ RUN      ] ZipTest.ExtractTest
/builddir/build/BUILD/xrootd-5.7.0/tests/XrdCl/XrdClZip.cc:58: Failure
Value of: _st.IsOK()
  Actual: false
Expected: true
[WaitFor(OpenArchive(zip_file, archiveUrl, OpenFlags::Read))]: [ERROR] Server responded with an error: [3011] No servers are available to read the file.
The tests uses files with random content created using `openssl rand`.
The sizes of the files are also random and are determined using the
$RANDOM shell varible:

${OPENSSL} rand -out "${TMPDIR}/${i}.ref" $((1024 * $RANDOM))

$RANDOM expands to a random integer between 0 and 32767. However,
requesting a 0 length output is not a valid option for `openssl rand`:

$ openssl rand 0
rand: Use -help for summary.
$ echo $?
1

When $RANDOM returns 0, the above error is triggered and the test
fails. Avoid this by using ($RANDOM + 1) instead.
amadio and others added 29 commits August 29, 2024 14:46
src/Xrd/XrdLink.cc:102:24: warning: overflow in conversion
from 'int' to 'char' changes value from '128' to '-128'
  102 | const char   KillXwt = 0x80;
src/XrdSecgsi/XrdSecProtocolgsi.hh:266:37: warning: format '%p' expects
argument of type 'void*', but argument 4 has type 'XrdCryptoX509Crl*'
  266 |       char k[40]; snprintf(k, 40, "%p", t);
      |                                    ~^   ~
      |                                     |   |
      |                                     |   XrdCryptoX509Crl*
      |                                     void*
It was discovered that the XrdEc tests were 10 times slower than usual.
This slowness came from the fact that for each call to
XrdOucUtils::obfuscateAuth(...) 2 std::regex were instanciated. The
instanciation of those two std::regex objects is expensive as there is
a compilation logic to ensure the regex is correct.
Those 2 regexes are now constructed only once in the lifetime of the
XRootD process by creating a static std::vector<std::regex>
Created XrdOucPrivateUtils.hh in XrdOuc and put the implementation in
XrdOucUtils.cc
This is needed to run the HTTP tests, since they use the
command line tools davix-{get,put,mkdir,mv,rm}.
This makes the starting/stopping of the server configurations
more robust, as well as print out client and server logs when
things fail to make debugging easier. New configurations with
sec.protocol set to host, unix, and sss have been added. Basic
HTTP and FUSE configurations have also been added. The HTTP
configuration tests the server with davix tools, while the FUSE
configuration runs some basic tests for xrootdfs.
FUSE may be installed, but not mounted, and davix commands
may not be available. The two new switches allows disabling
these tests if dependencies are not satisfied.
Since these tests can cause the mount point to hang on failure,
it's best to enable manually only when necessary.
@Jo-stfc Jo-stfc merged commit 46dca9d into v5.7.1withceph Sep 9, 2024
15 of 17 checks passed
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.

10 participants