-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
[12_0_X] Protect storage accounting UDP messages from NaN, and Use StatisticsSenderService for all framework files #36355
[12_0_X] Protect storage accounting UDP messages from NaN, and Use StatisticsSenderService for all framework files #36355
Conversation
NaN's were being reported from the values computed using sqrt. This most likely was from the different variables not being updated atomically together.
Previously, each try to open the file using a different PFN would report an open attempt for the same LFN. This meant we could have multiple opens but only one close for a given LFN.
When sending information to the StatisticsSenderService, the file LFN or URL must be supplied.
Send statistics for primary, secondary, and embedded files. The aggregate file statistics are only reset on primary file close boundaries to keep the behavior the same as previous. Changed all calls to closeFile_() to be the new closeFile()
Now broadcasts how the file is used.
A new Pull Request was created by @makortel (Matti Kortelainen) for CMSSW_12_0_X. It involves the following packages:
@cmsbuild, @smuzaffar, @Dr15Jones, @makortel can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@cmsbuild, please test |
backport |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e5d9ef/20974/summary.html Comparison SummarySummary:
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_12_0_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_12_2_X is complete. 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 @davidlange6 just to get an idea of the plans: would these backports in old but still active cycles require a new (patch) release? Or they should be merged "just if a new release has to be built independently"? (Or maybe they don't even need a new release: sorry, I don't know the details about how those popularity monitorings work...) |
hold |
Pull request has been put on hold by @makortel |
@perrotta I replied in #36349 (comment) (to avoid the discussion get buried in the backport PRs) |
Pull request #36355 was updated. @cmsbuild, @smuzaffar, @Dr15Jones, @makortel can you please check and sign again. |
unhold |
@cmsbuild, please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e5d9ef/21132/summary.html Comparison SummarySummary:
|
+1 |
This pull request is fully signed and it will be integrated in one of the next CMSSW_12_0_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_12_3_X is complete. 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) |
+1 |
PR description:
This PR is a combined backport of #35362 and #35505, following requests in #29412 and #36349. Includes also #36379 and #36403 as further fix and cleanup.
PR validation:
Unit tests pass.