Throw an exception if a module requiring synchronization on lumi boundaries is used when concurrent lumis are enabled #35326

makortel · 2021-09-17T19:17:44Z

PR description:

Following the completion of #25090. This PR simply turns an existing warning to an exception. It also addresses framework unit tests that would fail because of the exception. It is possible that some unit tests in other packages would fail too (in which case I'll address those separately).

Resolves cms-sw/framework-team#121

PR validation:

Framework unit tests run.

…to an exception

cmsbuild · 2021-09-17T19:26:37Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35326/25367

This PR adds an extra 68KB to repository

cmsbuild · 2021-09-17T19:27:04Z

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

It involves the following packages:

FWCore/Concurrency (core)
FWCore/Framework (core)
FWCore/Integration (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

makortel · 2021-09-17T19:37:48Z

test parameters:

enable_tests = threading

makortel · 2021-09-17T23:43:04Z

@cmsbuild, please test

cmsbuild · 2021-09-18T08:58:54Z

-1

Failed Tests: AddOn
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c4b1fc/18727/summary.html
COMMIT: 0910e65
CMSSW: CMSSW_12_1_X_2021-09-17-1100/slc7_amd64_gcc900
Additional Tests: THREADING
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/35326/18727/install.sh to create a dev area with all the needed externals and cmssw changes.

AddOn Tests

hlt_mc_Fake

----- Begin Fatal Exception 18-Sep-2021 03:00:22 CEST-----------------------
An exception of category 'ModulesSynchingOnLumis' occurred while
   [0] Calling beginJob
Exception Message:
The framework is configured to use at least two streams, but the following modules
require synchronizing on LuminosityBlock boundaries:
  Pythia8GeneratorFilter generator

The situation can be fixed by either
 * modifying the modules to support concurrent LuminosityBlocks (preferred), or
 * setting 'process.options.numberOfConcurrentLuminosityBlocks = 1' in the configuration file
----- End Fatal Exception -------------------------------------------------

hlt_mc_Fake

----- Begin Fatal Exception 18-Sep-2021 03:00:30 CEST-----------------------
An exception of category 'FileOpenError' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing input source of type PoolSource
   [2] Calling RootInputFileSequence::initTheFile()
   [3] Calling StorageFactory::open()
   [4] Calling File::sysopen()
Exception Message:
Failed to open the file 'RelVal_Raw_Fake_MC.root'
   Additional Info:
      [a] Input file file:RelVal_Raw_Fake_MC.root could not be opened.
      [b] open() failed with system error 'No such file or directory' (error code 2)
----- End Fatal Exception -------------------------------------------------

hlt_mc_Fake

----- Begin Fatal Exception 18-Sep-2021 03:01:12 CEST-----------------------
An exception of category 'FileOpenError' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing input source of type PoolSource
   [2] Calling RootInputFileSequence::initTheFile()
   [3] Calling StorageFactory::open()
   [4] Calling File::sysopen()
Exception Message:
Failed to open the file 'RelVal_Raw_Fake_MC.root'
   Additional Info:
      [a] Input file file:RelVal_Raw_Fake_MC.root could not be opened.
      [b] open() failed with system error 'No such file or directory' (error code 2)
----- End Fatal Exception -------------------------------------------------

Expand to see more addon errors ...

Comparison Summary

The workflows 140.53 have different files in step1_dasquery.log than the ones found in the baseline. You may want to check and retrigger the tests if necessary. You can check it in the "files" directory in the results of the comparisons

Summary:

No significant changes to the logs found
Reco comparison results: 1305 differences found in the comparisons
DQMHistoTests: Total files compared: 39
DQMHistoTests: Total histograms compared: 3000833
DQMHistoTests: Total failures: 3682
DQMHistoTests: Total nulls: 20
DQMHistoTests: Total successes: 2997109
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 45.699 KiB( 38 files compared)
DQMHistoSizes: changed ( 140.53 ): 44.531 KiB Hcal/DigiRunHarvesting
DQMHistoSizes: changed ( 140.53 ): 1.172 KiB RPC/DCSInfo
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

FWCore/Framework/interface/EventProcessor.h

makortel · 2021-10-04T17:46:21Z

@cmsbuild, please abort

makortel · 2021-10-04T17:47:39Z

Nope, not like this

Co-authored-by: Chris Jones <chrisdjones15@gmail.com>

cmsbuild · 2021-10-04T17:55:46Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35326/25742

This PR adds an extra 44KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File FWCore/Integration/test/TestParentage.cc modified in PR(s): Remove use of legacy module classes from framework tests #35526

makortel · 2021-10-04T17:56:47Z

@cmsbuild, please test

cmsbuild · 2021-10-04T17:57:08Z

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

cmsbuild · 2021-10-05T00:03:37Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c4b1fc/19385/summary.html
COMMIT: 9ab1bbf
CMSSW: CMSSW_12_1_X_2021-10-04-1300/slc7_amd64_gcc900
Additional Tests: THREADING
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/35326/19385/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: 40
DQMHistoTests: Total histograms compared: 3219394
DQMHistoTests: Total failures: 0
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3219372
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 39 files compared)
Checked 169 log files, 37 edm output root files, 40 DQM output files
TriggerResults: no differences found

makortel · 2021-10-05T00:50:57Z

+1

@perrotta @qliphy It would be good to exercise this PR in the IBs for at least one round (mostly to really check that everything still works) before cutting the pre4. It is also ok for me to merge this PR after pre4.

cmsbuild · 2021-10-05T00:51:15Z

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)

perrotta · 2021-10-05T08:39:41Z

+1

Try to exercize it in the 1100 IB, and see if any exception is thrown

makortel added 3 commits September 17, 2021 19:52

Turn warning on modules requiring synchronization on Lumi boundaries …

e914d6c

…to an exception

Set numberOfConcurrentLuminosityBlocks explicitly to 1 in various tests

add59dc

Make edmtest::TestParentage global

0910e65

makortel mentioned this pull request Sep 17, 2021

Change LogSystem to exception when trying to run a job with >= 2 concurrent lumis with an incompatible module cms-sw/framework-team#121

Closed

cmsbuild added this to the CMSSW_12_1_X milestone Sep 17, 2021

cmsbuild added code-checks-pending core-pending orp-pending pending-signatures tests-pending labels Sep 17, 2021

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

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

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

makortel mentioned this pull request Sep 20, 2021

Fix addOnTests to work with exception being thrown for lumi-synchronizing modules cms-sw/framework-team#285

Closed

2 tasks

cmsbuild added code-checks-pending hlt-pending tests-pending and removed tests-rejected code-checks-approved labels Sep 21, 2021

makortel force-pushed the throwLumiSynchronize branch from 0a2e752 to 0910e65 Compare September 21, 2021 00:01

cmsbuild removed hlt-pending tests-pending labels Sep 21, 2021

Dr15Jones reviewed Oct 4, 2021

View reviewed changes

FWCore/Framework/interface/EventProcessor.h Outdated Show resolved Hide resolved

cmsbuild added tests-pending code-checks-pending and removed tests-started code-checks-approved labels Oct 4, 2021

Fix typo

9ab1bbf

Co-authored-by: Chris Jones <chrisdjones15@gmail.com>

makortel force-pushed the throwLumiSynchronize branch from 3e4c56b to 9ab1bbf Compare October 4, 2021 17:49

cmsbuild added code-checks-approved tests-started and removed tests-pending code-checks-pending labels Oct 4, 2021

cmsbuild added tests-approved and removed tests-started labels Oct 5, 2021

cmsbuild added core-approved fully-signed and removed core-pending pending-signatures labels Oct 5, 2021

cmsbuild added orp-approved and removed orp-pending labels Oct 5, 2021

cmsbuild merged commit dd549ca into cms-sw:master Oct 5, 2021

makortel deleted the throwLumiSynchronize branch October 5, 2021 12:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Throw an exception if a module requiring synchronization on lumi boundaries is used when concurrent lumis are enabled #35326

Throw an exception if a module requiring synchronization on lumi boundaries is used when concurrent lumis are enabled #35326

makortel commented Sep 17, 2021

cmsbuild commented Sep 17, 2021

cmsbuild commented Sep 17, 2021

makortel commented Sep 17, 2021

makortel commented Sep 17, 2021

cmsbuild commented Sep 18, 2021

makortel commented Oct 4, 2021

makortel commented Oct 4, 2021 •

edited

cmsbuild commented Oct 4, 2021

makortel commented Oct 4, 2021

cmsbuild commented Oct 4, 2021

cmsbuild commented Oct 5, 2021

makortel commented Oct 5, 2021

cmsbuild commented Oct 5, 2021

perrotta commented Oct 5, 2021

Throw an exception if a module requiring synchronization on lumi boundaries is used when concurrent lumis are enabled #35326

Throw an exception if a module requiring synchronization on lumi boundaries is used when concurrent lumis are enabled #35326

Conversation

makortel commented Sep 17, 2021

PR description:

PR validation:

cmsbuild commented Sep 17, 2021

cmsbuild commented Sep 17, 2021

makortel commented Sep 17, 2021

makortel commented Sep 17, 2021

cmsbuild commented Sep 18, 2021

AddOn Tests

Comparison Summary

makortel commented Oct 4, 2021

makortel commented Oct 4, 2021 • edited

cmsbuild commented Oct 4, 2021

makortel commented Oct 4, 2021

cmsbuild commented Oct 4, 2021

cmsbuild commented Oct 5, 2021

Comparison Summary

makortel commented Oct 5, 2021

cmsbuild commented Oct 5, 2021

perrotta commented Oct 5, 2021

makortel commented Oct 4, 2021 •

edited