[12_0_X] Protect storage accounting UDP messages from NaN, and Use StatisticsSenderService for all framework files #36355

makortel · 2021-12-03T19:38:48Z

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.

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.

cmsbuild · 2021-12-03T19:39:10Z

A new Pull Request was created by @makortel (Matti Kortelainen) for CMSSW_12_0_X.

It involves the following packages:

IOPool/Input (core)
IOPool/SecondaryInput (core)
IOPool/TFileAdaptor (core)
Utilities/StorageFactory (core)
Utilities/XrdAdaptor (core)

@cmsbuild, @smuzaffar, @Dr15Jones, @makortel 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

makortel · 2021-12-03T19:44:36Z

@cmsbuild, please test

makortel · 2021-12-03T19:49:37Z

backport

cmsbuild · 2021-12-03T22:54:57Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e5d9ef/20974/summary.html
COMMIT: 88d9867
CMSSW: CMSSW_12_0_X_2021-12-03-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/36355/20974/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: 10 differences found in the comparisons
DQMHistoTests: Total files compared: 39
DQMHistoTests: Total histograms compared: 2998564
DQMHistoTests: Total failures: 12
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2998530
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 38 files compared)
Checked 165 log files, 37 edm output root files, 39 DQM output files
TriggerResults: no differences found

cmsbuild · 2021-12-04T02:37:29Z

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)

perrotta · 2021-12-06T17:16:11Z

@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...)

makortel · 2021-12-06T17:19:41Z

hold

cmsbuild · 2021-12-06T17:19:54Z

Pull request has been put on hold by @makortel
They need to issue an unhold command to remove the hold state or L1 can unhold it for all

makortel · 2021-12-06T19:41:17Z

@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...)

@perrotta I replied in #36349 (comment) (to avoid the discussion get buried in the backport PRs)

makortel · 2021-12-09T17:28:40Z

Now including #36379 and #36403 too.

cmsbuild · 2021-12-09T17:28:43Z

Pull request #36355 was updated. @cmsbuild, @smuzaffar, @Dr15Jones, @makortel can you please check and sign again.

makortel · 2021-12-09T17:29:35Z

unhold

makortel · 2021-12-09T17:29:42Z

@cmsbuild, please test

cmsbuild · 2021-12-09T20:41:40Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e5d9ef/21132/summary.html
COMMIT: 6bb791c
CMSSW: CMSSW_12_0_X_2021-12-09-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/36355/21132/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: 39
DQMHistoTests: Total histograms compared: 2998564
DQMHistoTests: Total failures: 5
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 2998536
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.004 KiB( 38 files compared)
DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
Checked 165 log files, 37 edm output root files, 39 DQM output files
TriggerResults: no differences found

makortel · 2021-12-09T20:47:54Z

+1

cmsbuild · 2021-12-09T20:48:15Z

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)

qliphy · 2021-12-10T04:10:49Z

+1

Dr15Jones added 7 commits December 3, 2021 13:10

Protect storage accounting UDP messages from NaN

7292426

NaN's were being reported from the values computed using sqrt. This most likely was from the different variables not being updated atomically together.

Report only once when trying to open file

13a6e59

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.

StatisticsSenderService now requires LFN or URL

2751d61

When sending information to the StatisticsSenderService, the file LFN or URL must be supplied.

Changed SecondaryProducer to be a one::EDProducer

2a13ce0

Added unit test for StatisticsSenderService

9b510dc

Pass InputType to StatisticsSenderService

88d9867

Now broadcasts how the file is used.

makortel mentioned this pull request Dec 3, 2021

Backport recent StatisticsSenderService changes to earlier releases cms-sw/framework-team#329

Closed

11 tasks

cmsbuild added this to the CMSSW_12_0_X milestone Dec 3, 2021

cmsbuild added core-pending orp-pending pending-signatures tests-pending labels Dec 3, 2021

cmsbuild added tests-started and removed tests-pending labels Dec 3, 2021

cmsbuild added tests-approved core-approved fully-signed and removed tests-started core-pending pending-signatures labels Dec 3, 2021

cmsbuild removed the core-approved label Dec 6, 2021

cmsbuild added core-pending hold pending-signatures labels Dec 6, 2021

makortel mentioned this pull request Dec 6, 2021

Bad sizes in Popularity #36349

Closed

makortel added 2 commits December 9, 2021 17:11

Set RootInputFileSequence::usedFallback_

4d429cb

Silence file closuring printouts in StatisticsSenderService by default

6bb791c

cmsbuild added tests-pending and removed tests-approved labels Dec 9, 2021

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

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

cmsbuild added core-approved fully-signed and removed core-pending pending-signatures labels Dec 9, 2021

cmsbuild added orp-approved and removed orp-pending labels Dec 10, 2021

cmsbuild merged commit 0a5b4b9 into cms-sw:CMSSW_12_0_X Dec 10, 2021

makortel deleted the backportStatisticsSenderService_120x branch December 10, 2021 15:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[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

makortel commented Dec 3, 2021 •

edited

cmsbuild commented Dec 3, 2021

makortel commented Dec 3, 2021

makortel commented Dec 3, 2021

cmsbuild commented Dec 3, 2021

cmsbuild commented Dec 4, 2021

perrotta commented Dec 6, 2021

makortel commented Dec 6, 2021

cmsbuild commented Dec 6, 2021

makortel commented Dec 6, 2021

makortel commented Dec 9, 2021

cmsbuild commented Dec 9, 2021

makortel commented Dec 9, 2021

makortel commented Dec 9, 2021

cmsbuild commented Dec 9, 2021

makortel commented Dec 9, 2021

cmsbuild commented Dec 9, 2021

qliphy commented Dec 10, 2021

[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

makortel commented Dec 3, 2021 • edited

PR description:

PR validation:

cmsbuild commented Dec 3, 2021

makortel commented Dec 3, 2021

makortel commented Dec 3, 2021

cmsbuild commented Dec 3, 2021

Comparison Summary

cmsbuild commented Dec 4, 2021

perrotta commented Dec 6, 2021

makortel commented Dec 6, 2021

cmsbuild commented Dec 6, 2021

makortel commented Dec 6, 2021

makortel commented Dec 9, 2021

cmsbuild commented Dec 9, 2021

makortel commented Dec 9, 2021

makortel commented Dec 9, 2021

cmsbuild commented Dec 9, 2021

Comparison Summary

makortel commented Dec 9, 2021

cmsbuild commented Dec 9, 2021

qliphy commented Dec 10, 2021

makortel commented Dec 3, 2021 •

edited