[10_6_X] Protect storage accounting UDP messages from NaN, and Use StatisticsSenderService for all framework files #36360

makortel · 2021-12-03T21:16:26Z

PR description:

This PR is a combined backport of #35362 and #35505, following requests in #29412 and #36349. It also contains small part of #26789 to make the cherry-picks easier, and #36403 (fully) as further cleanup.

PR validation:

Unit tests pass.

Backport from f49614b

Backport from 08cd101

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-03T21:16:49Z

A new Pull Request was created by @makortel (Matti Kortelainen) for CMSSW_10_6_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-03T21:24:03Z

@cmsbuild, please test

makortel · 2021-12-03T21:24:08Z

backport

cmsbuild · 2021-12-04T00:53:01Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-63717a/20979/summary.html
COMMIT: a5708ad
CMSSW: CMSSW_10_6_X_2021-11-30-1100/slc7_amd64_gcc700
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/36360/20979/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: 8 differences found in the comparisons
DQMHistoTests: Total files compared: 35
DQMHistoTests: Total histograms compared: 3215686
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3215351
DQMHistoTests: Total skipped: 334
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 34 files compared)
Checked 143 log files, 29 edm output root files, 35 DQM output files
TriggerResults: no differences found

cmsbuild · 2021-12-04T02:41:45Z

This pull request is fully signed and it will be integrated in one of the next CMSSW_10_6_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-04T09:21:59Z

IOPool/Input/src/RootInputFileSequence.cc includes also the updates originally in New developments using multiple data catalogs provided in site-local-config.xml #28911 (“New developments using multiple data catalogs provided in site-local-config.xml“): they were merged in CMSSW_11_1_0_patch1
Utilities/StorageFactory/interface/StatisticsSenderService.h, Utilities/StorageFactory/src/StatisticsSenderService.cc, Utilities/XrdAdaptor/src/XrdRequestManager.cc includes also the updates in Running code-format for core #26789 (applying code-format, therefore it must be safe)

makortel · 2021-12-06T17:18:55Z

hold

cmsbuild · 2021-12-09T21:35:27Z

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

makortel · 2021-12-09T21:39:22Z

Now including #36403 too.

makortel · 2021-12-09T21:39:26Z

unhold

makortel · 2021-12-09T21:39:32Z

@cmsbuild, please test

cmsbuild · 2021-12-10T00:55:51Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-63717a/21147/summary.html
COMMIT: c979f6e
CMSSW: CMSSW_10_6_X_2021-12-05-0000/slc7_amd64_gcc700
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/36360/21147/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: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 35
DQMHistoTests: Total histograms compared: 3215686
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3215351
DQMHistoTests: Total skipped: 334
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 34 files compared)
Checked 143 log files, 29 edm output root files, 35 DQM output files
TriggerResults: no differences found

makortel · 2021-12-10T02:07:31Z

+1

perrotta · 2021-12-10T06:36:39Z

ping bot

cmsbuild · 2021-12-10T06:37:06Z

This pull request is fully signed and it will be integrated in one of the next CMSSW_10_6_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-10T12:11:13Z

+1

makortel and others added 9 commits December 3, 2021 14:42

Clang-tidy

6960123

Backport from f49614b

Clang-Format

5f7f5e4

Backport from 08cd101

Protect storage accounting UDP messages from NaN

c03adef

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

98f94b6

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

93fa929

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

Changed SecondaryProducer to be a one::EDProducer

0a7e792

Added unit test for StatisticsSenderService

6108096

Pass InputType to StatisticsSenderService

a5708ad

Now broadcasts how the file is used.

cmsbuild added this to the CMSSW_10_6_X milestone Dec 3, 2021

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 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 and removed tests-started labels Dec 4, 2021

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

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

Silence file closuring printouts in StatisticsSenderService by default

c979f6e

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 10, 2021

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

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

cmsbuild merged commit ee580a7 into cms-sw:CMSSW_10_6_X Dec 10, 2021

makortel deleted the backportStatisticsSenderService_106x branch December 10, 2021 15:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[10_6_X] Protect storage accounting UDP messages from NaN, and Use StatisticsSenderService for all framework files #36360

[10_6_X] Protect storage accounting UDP messages from NaN, and Use StatisticsSenderService for all framework files #36360

makortel commented Dec 3, 2021 •

edited

cmsbuild commented Dec 3, 2021

makortel commented Dec 3, 2021

makortel commented Dec 3, 2021

cmsbuild commented Dec 4, 2021

cmsbuild commented Dec 4, 2021

perrotta commented Dec 4, 2021

makortel commented Dec 6, 2021

cmsbuild commented Dec 9, 2021

makortel commented Dec 9, 2021

makortel commented Dec 9, 2021

makortel commented Dec 9, 2021

cmsbuild commented Dec 10, 2021

makortel commented Dec 10, 2021

perrotta commented Dec 10, 2021

cmsbuild commented Dec 10, 2021

qliphy commented Dec 10, 2021

[10_6_X] Protect storage accounting UDP messages from NaN, and Use StatisticsSenderService for all framework files #36360

[10_6_X] Protect storage accounting UDP messages from NaN, and Use StatisticsSenderService for all framework files #36360

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 4, 2021

Comparison Summary

cmsbuild commented Dec 4, 2021

perrotta commented Dec 4, 2021

makortel commented Dec 6, 2021

cmsbuild commented Dec 9, 2021

makortel commented Dec 9, 2021

makortel commented Dec 9, 2021

makortel commented Dec 9, 2021

cmsbuild commented Dec 10, 2021

Comparison Summary

makortel commented Dec 10, 2021

perrotta commented Dec 10, 2021

cmsbuild commented Dec 10, 2021

qliphy commented Dec 10, 2021

makortel commented Dec 3, 2021 •

edited