For async calls always store XrdCl::FileSystem with the response-handler. #35481

smuzaffar · 2021-09-30T07:00:20Z

backport of #35455

This is a continuation of #34700.

XrdCl's internal response handlers (before user-supplied handler is called) access URL string that is part of the FileSystem object. As FileSystemObject was created on the stack, this gets destroyed while async request are still pending.

This PR moves FileSystem object into our response-handlers so it's lifetime is the same as that of the response. All response handlers auto-destruct as needed.

…ler.

smuzaffar · 2021-09-30T07:02:27Z

test parameters:

pull_request = XrootD - backport 531 from devel to master cmsdist#7233, Backport XrootD 5.3.1 PR to master from DEVEL #34991
enable_test = threading

cmsbuild · 2021-09-30T07:08:22Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35481/25645

This PR adds an extra 8KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File Utilities/XrdAdaptor/src/XrdRequestManager.cc modified in PR(s): Refactored code in StorageFactory #35435, Backport XrootD 5.3.1 PR to master from DEVEL #34991

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35481/25645/code-format.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35481/25645/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

cmsbuild · 2021-09-30T07:19:11Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35481/25646

This PR adds an extra 8KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File Utilities/XrdAdaptor/src/XrdRequestManager.cc modified in PR(s): Refactored code in StorageFactory #35435, Backport XrootD 5.3.1 PR to master from DEVEL #34991

smuzaffar · 2021-09-30T07:19:29Z

please test

cmsbuild · 2021-09-30T07:19:32Z

A new Pull Request was created by @smuzaffar (Malik Shahzad Muzaffar) for master.

It involves the following packages:

Utilities/XrdAdaptor (core)

@makortel, @smuzaffar, @cmsbuild, @Dr15Jones can you please review it and eventually sign? Thanks.
@wddgit this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

Backported from For async calls always store XrdCl::FileSystem with the response-handler. #35455

cmsbuild · 2021-09-30T13:52:14Z

-1

Failed Tests: ClangBuild
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1dd22a/19266/summary.html
COMMIT: 7798261
CMSSW: CMSSW_12_1_X_2021-09-29-2300/slc7_amd64_gcc900
Additional Tests: THREADING
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/35481/19266/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1dd22a/19266/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1dd22a/19266/git-merge-result

Clang Build

I found compilation warning while trying to compile with clang. Command used:

USER_CUDA_FLAGS='--expt-relaxed-constexpr' USER_CXXFLAGS='-Wno-register -fsyntax-only' scram build -k -j 32 COMPILER='llvm compile'

See details on the summary page.

smuzaffar · 2021-09-30T20:23:56Z

@osschar , clang complains

Utilities/XrdAdaptor/src/XrdRequestManager.cc:90:3: warning: explicitly defaulted default constructor is implicitly deleted [-Wdefaulted-function-deleted]

see the log https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1dd22a/19266/clang-logs/Utilities/XrdAdaptor/log.html . Can you please fix this warning?

cmsbuild · 2021-12-09T22:26:33Z

-1

Failed Tests: UnitTests
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1dd22a/21117/summary.html
COMMIT: cb2c26e
CMSSW: CMSSW_12_3_X_2021-12-08-2300/slc7_amd64_gcc900
Additional Tests: THREADING
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/35481/21117/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found errors in the following unit tests:

---> test testCondToolsSiStripBuildersReaders had ERRORS

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 2 differences found in the comparisons
DQMHistoTests: Total files compared: 42
DQMHistoTests: Total histograms compared: 3250608
DQMHistoTests: Total failures: 5
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 3250580
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.004 KiB( 41 files compared)
DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
Checked 177 log files, 37 edm output root files, 42 DQM output files
TriggerResults: no differences found

smuzaffar · 2021-12-17T08:35:22Z

please test

cmsbuild · 2021-12-17T20:49:38Z

-1

Failed Tests: UnitTests
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1dd22a/21351/summary.html
COMMIT: cb2c26e
CMSSW: CMSSW_12_3_X_2021-12-16-2300/slc7_amd64_gcc10
Additional Tests: THREADING
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/35481/21351/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

@cms-sw [DD4hep] Update to v01-19 cmsdist#7514

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1dd22a/21351/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1dd22a/21351/git-merge-result

Unit Tests

I found errors in the following unit tests:

---> test testCondToolsSiStripBuildersReaders had ERRORS

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 2853 differences found in the comparisons
DQMHistoTests: Total files compared: 42
DQMHistoTests: Total histograms compared: 3250719
DQMHistoTests: Total failures: 2522
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3248175
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 41 files compared)
Checked 177 log files, 37 edm output root files, 42 DQM output files
TriggerResults: found differences in 1 / 41 workflows

smuzaffar · 2022-01-08T12:48:56Z

please test

cmsbuild · 2022-01-09T01:08:21Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1dd22a/21570/summary.html
COMMIT: cb2c26e
CMSSW: CMSSW_12_3_X_2022-01-08-1100/slc7_amd64_gcc10
Additional Tests: THREADING
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/35481/21570/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 3 differences found in the comparisons
DQMHistoTests: Total files compared: 43
DQMHistoTests: Total histograms compared: 3461659
DQMHistoTests: Total failures: 6
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3461631
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 42 files compared)
Checked 181 log files, 42 edm output root files, 43 DQM output files
TriggerResults: no differences found

smuzaffar · 2022-01-09T15:01:39Z

@makortel , I think this is ready to go in now ( along with cms-sw/cmsdist#7233 #34991)

smuzaffar · 2022-01-10T12:30:11Z

+core

cmsbuild · 2022-01-10T12:30:35Z

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

makortel · 2022-01-10T14:14:08Z

I think this is ready to go in now ( along with cms-sw/cmsdist#7233 #34991)

I agree, thanks!

qliphy · 2022-01-11T01:02:11Z

+1

For async calls always store XrdCl::FileSystem with the response-hand…

2806dac

…ler.

smuzaffar mentioned this pull request Sep 30, 2021

For async calls always store XrdCl::FileSystem with the response-handler. #35455

Merged

cmsbuild added this to the CMSSW_12_1_X milestone Sep 30, 2021

cmsbuild added backport-ok code-checks-pending core-pending orp-pending pending-signatures tests-pending labels Sep 30, 2021

smuzaffar mentioned this pull request Sep 30, 2021

Backport XrootD 5.3.1 PR to master from DEVEL #34991

Merged

cmsbuild added the requires-external label Sep 30, 2021

cmsbuild added code-checks-rejected and removed code-checks-pending labels Sep 30, 2021

code format

7798261

cmsbuild added code-checks-pending and removed code-checks-rejected labels Sep 30, 2021

smuzaffar mentioned this pull request Sep 30, 2021

XrootD - backport 531 from devel to master cms-sw/cmsdist#7233

Merged

cmsbuild added code-checks-approved and removed code-checks-pending labels Sep 30, 2021

cmsbuild added tests-started tests-rejected and removed tests-pending tests-started labels Sep 30, 2021

cmsbuild added the tests-started label Dec 9, 2021

cmsbuild added tests-rejected and removed tests-started labels Dec 9, 2021

cmsbuild added tests-started and removed tests-rejected labels Dec 17, 2021

cmsbuild added tests-rejected and removed tests-started labels Dec 17, 2021

cmsbuild added tests-started and removed tests-rejected labels Jan 8, 2022

cmsbuild added tests-approved and removed tests-started labels Jan 9, 2022

cmsbuild added core-approved fully-signed and removed core-pending pending-signatures labels Jan 10, 2022

cmsbuild added orp-approved and removed orp-pending labels Jan 11, 2022

cmsbuild merged commit b3ce8c2 into master Jan 11, 2022

cmsbuild mentioned this pull request Jan 12, 2022

for external PR tests, make sure to include latest cmssw merges cms-sw/cms-bot#1686

Merged

smuzaffar deleted the backport-master-35455 branch April 25, 2022 11:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

For async calls always store XrdCl::FileSystem with the response-handler. #35481

For async calls always store XrdCl::FileSystem with the response-handler. #35481

smuzaffar commented Sep 30, 2021

smuzaffar commented Sep 30, 2021

cmsbuild commented Sep 30, 2021

cmsbuild commented Sep 30, 2021

smuzaffar commented Sep 30, 2021

cmsbuild commented Sep 30, 2021 •

edited

Loading

cmsbuild commented Sep 30, 2021

smuzaffar commented Sep 30, 2021

cmsbuild commented Dec 9, 2021

smuzaffar commented Dec 17, 2021

cmsbuild commented Dec 17, 2021

smuzaffar commented Jan 8, 2022

cmsbuild commented Jan 9, 2022

smuzaffar commented Jan 9, 2022

smuzaffar commented Jan 10, 2022

cmsbuild commented Jan 10, 2022

makortel commented Jan 10, 2022

qliphy commented Jan 11, 2022

For async calls always store XrdCl::FileSystem with the response-handler. #35481

For async calls always store XrdCl::FileSystem with the response-handler. #35481

Conversation

smuzaffar commented Sep 30, 2021

smuzaffar commented Sep 30, 2021

cmsbuild commented Sep 30, 2021

cmsbuild commented Sep 30, 2021

smuzaffar commented Sep 30, 2021

cmsbuild commented Sep 30, 2021 • edited Loading

cmsbuild commented Sep 30, 2021

Clang Build

smuzaffar commented Sep 30, 2021

cmsbuild commented Dec 9, 2021

Unit Tests

Comparison Summary

smuzaffar commented Dec 17, 2021

cmsbuild commented Dec 17, 2021

Unit Tests

Comparison Summary

smuzaffar commented Jan 8, 2022

cmsbuild commented Jan 9, 2022

Comparison Summary

smuzaffar commented Jan 9, 2022

smuzaffar commented Jan 10, 2022

cmsbuild commented Jan 10, 2022

makortel commented Jan 10, 2022

qliphy commented Jan 11, 2022

cmsbuild commented Sep 30, 2021 •

edited

Loading