Can now disable CondorStatusUpdator #37673

Dr15Jones · 2022-04-25T19:53:08Z

PR description:

Added a parameter to allow the service to be disabled.

PR validation:

Code compiles.

finishes cms-sw/framework-team#389

cmsbuild · 2022-04-25T20:02:17Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37673/29489

This PR adds an extra 16KB to repository

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-37673/29489/code-format.patch
e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37673/29489/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

Added a parameter to allow the service to be disabled.

cmsbuild · 2022-04-25T20:29:07Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37673/29490

This PR adds an extra 16KB to repository

cmsbuild · 2022-04-25T20:29:29Z

A new Pull Request was created by @Dr15Jones (Chris Jones) for master.

It involves the following packages:

FWCore/Services (core)

@cmsbuild, @smuzaffar, @Dr15Jones, @makortel can you please review it and eventually sign? Thanks.
@makortel, @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

Dr15Jones · 2022-04-25T20:37:08Z

please test

makortel · 2022-04-26T00:14:00Z

FWCore/Services/plugins/CondorStatusUpdater.cc

@@ -419,6 +425,7 @@ void CondorStatusService::fillDescriptions(ConfigurationDescriptions &descriptio
      ->setComment("Interval, in seconds, to calculate event rate over (using EMA)");
  desc.addOptionalUntracked<std::string>("tag")->setComment(
      "Identifier tag for this process (a value of 'Foo' results in ClassAd attributes of the form 'ChirpCMSSWFoo*')");
+  desc.addOptionalUntracked<bool>("disable", false)->setComment("Disable this service");


How about inverting the logic along

Suggested change

desc.addOptionalUntracked<bool>("disable", false)->setComment("Disable this service");

desc.addOptionalUntracked<bool>("enable", true)->setComment("Enable this service");

? (just thinking if avoiding the double negation of "not disabling" would be more clear)

cmsbuild · 2022-04-26T02:12:18Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a3514c/24199/summary.html
COMMIT: 6174b8c
CMSSW: CMSSW_12_4_X_2022-04-25-1100/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/37673/24199/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-a3514c/24199/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a3514c/24199/git-merge-result

Comparison Summary

@slava77 comparisons for the following workflows were not done due to missing matrix map:

/data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-a3514c/39434.75_TTbar_14TeV+2026D88_HLT75e33+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14+DigiTrigger+RecoGlobal+HLT75e33+HARVESTGlobal

Summary:

No significant changes to the logs found
Reco comparison results: 19934 differences found in the comparisons
DQMHistoTests: Total files compared: 49
DQMHistoTests: Total histograms compared: 3695434
DQMHistoTests: Total failures: 31892
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 3663519
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.004 KiB( 48 files compared)
DQMHistoSizes: changed ( 138.5 ): 0.004 KiB JetMET/SUSYDQM
Checked 205 log files, 45 edm output root files, 49 DQM output files
TriggerResults: found differences in 4 / 48 workflows

cmsbuild · 2022-04-27T17:38:56Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37673/29548

This PR adds an extra 16KB to repository

cmsbuild · 2022-04-27T17:51:09Z

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

makortel

Sorry I commented in few drops, these should be all now.

makortel · 2022-04-27T17:54:05Z

FWCore/Services/plugins/CondorStatusUpdater.cc

@@ -80,6 +80,7 @@ namespace edm {
      edm::ParameterSetID m_processParameterSetID;

      std::uint_least64_t m_lastEventCount = 0;
+      bool m_disable = false;


This looks to be unnecessary

Suggested change

bool m_disable = false;

makortel · 2022-04-27T17:54:27Z

FWCore/Services/plugins/CondorStatusUpdater.cc

@@ -92,10 +93,15 @@ const unsigned int CondorStatusService::m_defaultUpdateInterval;
 constexpr float CondorStatusService::m_defaultEmaInterval;

 CondorStatusService::CondorStatusService(ParameterSet const &pset, edm::ActivityRegistry &ar)
-    : m_debug(false), m_lastUpdate(0), m_events(0), m_lumis(0), m_runs(0), m_files(0) {
+    : m_debug(pset.getUntrackedParameter("debug", false)),


Strictly speaking the default value should not be needed because of having default value in the fillDescriptions(), right?

optionals do not get injected into the PSet

https://twiki.cern.ch/twiki/bin/view/CMSPublic/SWGuideConfigurationValidationAndHelp#Optional_Parameters

Ah right, thanks.

makortel · 2022-04-27T17:54:48Z

FWCore/Services/plugins/CondorStatusUpdater.cc

  m_shouldUpdate.clear();
-  if (pset.exists("debug")) {
-    m_debug = true;
+  if (not pset.getUntrackedParameter("enable", true)) {


Same question here about the default value.

cmsbuild · 2022-04-27T18:09:19Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37673/29551

This PR adds an extra 16KB to repository

cmsbuild · 2022-04-27T18:09:39Z

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

makortel · 2022-04-27T20:42:46Z

@cmsbuild, please test

cmsbuild · 2022-04-27T23:56:06Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a3514c/24284/summary.html
COMMIT: 5b59b0b
CMSSW: CMSSW_12_4_X_2022-04-27-1100/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/37673/24284/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

There are some workflows for which there are errors in the baseline:
1001.2 step 2
The results for the comparisons for these workflows could be incomplete
This means most likely that the IB is having errors in the relvals.The error does NOT come from this pull request

Summary:

No significant changes to the logs found
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 49
DQMHistoTests: Total histograms compared: 3695434
DQMHistoTests: Total failures: 7
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 3695404
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -0.004 KiB( 48 files compared)
DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
Checked 205 log files, 45 edm output root files, 49 DQM output files
TriggerResults: no differences found

makortel · 2022-04-27T23:58:19Z

+1

cmsbuild · 2022-04-27T23:58:42Z

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)

qliphy · 2022-04-28T00:30:13Z

+1

cmsbuild added this to the CMSSW_12_4_X milestone Apr 25, 2022

cmsbuild added code-checks-pending core-pending orp-pending pending-signatures tests-pending labels Apr 25, 2022

cmsbuild added code-checks-rejected and removed code-checks-pending labels Apr 25, 2022

Can now disable CondorStatusUpdator

6174b8c

Added a parameter to allow the service to be disabled.

Dr15Jones force-pushed the canDisableCondorStatusUpdator branch from ac7cdf5 to 6174b8c Compare April 25, 2022 20:18

cmsbuild added code-checks-pending and removed code-checks-rejected labels Apr 25, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Apr 25, 2022

cmsbuild added tests-started and removed tests-pending labels Apr 25, 2022

makortel mentioned this pull request Apr 25, 2022

Improve CondorStatusService cms-sw/framework-team#389

Closed

1 task

makortel reviewed Apr 26, 2022

View reviewed changes

cmsbuild added tests-approved code-checks-pending tests-pending and removed tests-started tests-approved code-checks-approved labels Apr 26, 2022

cmsbuild added the code-checks-approved label Apr 27, 2022

makortel reviewed Apr 27, 2022

View reviewed changes

Switch parameter to enable

5b59b0b

Dr15Jones force-pushed the canDisableCondorStatusUpdator branch from 59a6e09 to 5b59b0b Compare April 27, 2022 18:01

cmsbuild added code-checks-pending and removed code-checks-approved labels Apr 27, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Apr 27, 2022

cmsbuild added tests-started and removed tests-pending labels Apr 27, 2022

cmsbuild added tests-approved and removed tests-started labels Apr 27, 2022

cmsbuild added core-approved fully-signed and removed core-pending pending-signatures labels Apr 27, 2022

cmsbuild added orp-approved and removed orp-pending labels Apr 28, 2022

cmsbuild merged commit 84a1e5a into cms-sw:master Apr 28, 2022

This was referenced Apr 28, 2022

Dynamic Reduction Network for photon energy regression in miniAOD using the SonicTriton service #37134

Merged

DeepSC algorithm for ECAL SuperClusters #37115

Merged

Dr15Jones deleted the canDisableCondorStatusUpdator branch May 9, 2022 18:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Can now disable CondorStatusUpdator #37673

Can now disable CondorStatusUpdator #37673

Dr15Jones commented Apr 25, 2022

cmsbuild commented Apr 25, 2022

cmsbuild commented Apr 25, 2022

cmsbuild commented Apr 25, 2022

Dr15Jones commented Apr 25, 2022

makortel Apr 26, 2022

cmsbuild commented Apr 26, 2022

cmsbuild commented Apr 27, 2022

cmsbuild commented Apr 27, 2022

makortel left a comment

makortel Apr 27, 2022

makortel Apr 27, 2022

Dr15Jones Apr 27, 2022

makortel Apr 27, 2022

makortel Apr 27, 2022

cmsbuild commented Apr 27, 2022

cmsbuild commented Apr 27, 2022

makortel commented Apr 27, 2022

cmsbuild commented Apr 27, 2022

makortel commented Apr 27, 2022

cmsbuild commented Apr 27, 2022

qliphy commented Apr 28, 2022

	desc.addOptionalUntracked<bool>("disable", false)->setComment("Disable this service");
	desc.addOptionalUntracked<bool>("enable", true)->setComment("Enable this service");

Can now disable CondorStatusUpdator #37673

Can now disable CondorStatusUpdator #37673

Conversation

Dr15Jones commented Apr 25, 2022

PR description:

PR validation:

cmsbuild commented Apr 25, 2022

cmsbuild commented Apr 25, 2022

cmsbuild commented Apr 25, 2022

Dr15Jones commented Apr 25, 2022

makortel Apr 26, 2022

Choose a reason for hiding this comment

cmsbuild commented Apr 26, 2022

Comparison Summary

cmsbuild commented Apr 27, 2022

cmsbuild commented Apr 27, 2022

makortel left a comment

Choose a reason for hiding this comment

makortel Apr 27, 2022

Choose a reason for hiding this comment

makortel Apr 27, 2022

Choose a reason for hiding this comment

Dr15Jones Apr 27, 2022

Choose a reason for hiding this comment

makortel Apr 27, 2022

Choose a reason for hiding this comment

makortel Apr 27, 2022

Choose a reason for hiding this comment

cmsbuild commented Apr 27, 2022

cmsbuild commented Apr 27, 2022

makortel commented Apr 27, 2022

cmsbuild commented Apr 27, 2022

Comparison Summary

makortel commented Apr 27, 2022

cmsbuild commented Apr 27, 2022

qliphy commented Apr 28, 2022