Changing LHCInfo O2O Implementation to access source data from OMS #36956

ggovi · 2022-02-14T13:07:52Z

PR description:

A relevant subset of the online databases related to the DAQ and BEAM parameters control have been significantly reworked in view of run3. In particular, parameters concerning Fills, Lumisections and also most of the data handled by DIP have been moved to a different storage layout. The direct access on the corresponding Oracle tables has been discouraged in favour of queries made on a new dedicated web service, OMS.

This PR aims to re-work the implementation of the LHCInfo O2O workflow, adapting the extraction of the data source to the new indicated service, the OMS.

For this purpose a few utility classes have been created:

A generic Convenience class for query data from a Web function, based on Curl
A utility class for the specific access on OMS, with several tasks: -form the expected connection string for the required queries; -decoding the JSON string returned by the queries; -converting the data types as needed for populating the existing data structure

The O2O workflow has been adapted and is using these utilities.

PR validation:

Local Integration tests

… params

…rom direct Oracle access to OMS Service

cmsbuild · 2022-02-14T13:13:53Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36956/28309

This PR adds an extra 40KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-checks:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36956/28309/code-checks.patch
e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36956/28309/code-checks.patch | patch -p1
You can also run scram build code-checks to apply code checks directly
code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36956/28309/code-format.patch
e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36956/28309/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

ggovi · 2022-02-14T13:42:31Z

can we do anything to re-trigger the code-checks? the concerned commit did not make it in time...

tvami · 2022-02-15T13:20:07Z

code-checks

tvami · 2022-02-15T13:21:28Z

CondCore/CondDB/src/WebUtils.cc

+
+namespace cond {
+
+  //


Was this supposed to be a comment after the // ?

cmsbuild · 2022-02-15T13:26:16Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36956/28337

This PR adds an extra 40KB to repository

cmsbuild · 2022-02-15T13:26:41Z

A new Pull Request was created by @ggovi for master.

It involves the following packages:

CondCore/CondDB (db)
CondFormats/RunInfo (db, alca)
CondTools/RunInfo (db)

@malbouis, @yuanchao, @cmsbuild, @ggovi, @francescobrivio, @tvami can you please review it and eventually sign? Thanks.
@mmusich, @tocheng, @seemasharmafnal 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

tvami · 2022-02-15T13:27:40Z

CondTools/RunInfo/src/LHCInfoPopConSourceHandler.cc

+        //coral::Attribute const& ctppsStatusAttribute = CTPPSDataCursor.currentRow()[std::string("CTPPS_STATUS")];
+        //if (!ctppsStatusAttribute.isNull()) {
+        //  ctppsStatus = ctppsStatusAttribute.data<std::string>();
+        //}


should this be removed?

@tvami In principle this has been left intentionally, because the concerned variables are no longer available in the source db... Are they critical? Should we ask a replacement? Not sure. But maybe we can remove that block for the moment.

Maybe keep them commented but add a sentence above to explain this?

tvami · 2022-02-15T13:28:51Z

@cmsbuild , please test

francescobrivio · 2022-02-15T13:37:07Z

CondCore/CondDB/interface/Types.h

@@ -72,7 +72,7 @@ namespace cond {
    std::string name;
    Iov_t lastInterval;
    size_t size = 0;
-    bool isEmpty() const { return lastInterval.since != time::MAX_VAL; }
+    bool isEmpty() const { return lastInterval.since == time::MAX_VAL; }


@ggovi this changes completely the logic of isEmpty() method, is this intended? (i.e. is it a bug-fix?)

@francescobrivio Good catch Francesco! Yes, this looks weird, it was clearly buggy before the fix, but fortunately never used ( according to my check ). Now, instead, it is used, so I need it correctly implemented!

Thanks for the explanation Giacomo! then I agree with the change!

cmsbuild · 2022-02-15T18:17:24Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1b812d/22431/summary.html
COMMIT: b6d2cf0
CMSSW: CMSSW_12_3_X_2022-02-14-2300/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/36956/22431/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: 46
DQMHistoTests: Total histograms compared: 3764435
DQMHistoTests: Total failures: 8
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3764405
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 45 files compared)
Checked 193 log files, 42 edm output root files, 46 DQM output files
TriggerResults: no differences found

cmsbuild · 2022-02-17T15:13:33Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36956/28382

This PR adds an extra 20KB to repository

cmsbuild · 2022-02-17T15:13:58Z

Pull request #36956 was updated. @malbouis, @yuanchao, @cmsbuild, @ggovi, @francescobrivio, @tvami can you please check and sign again.

tvami · 2022-02-17T15:48:56Z

@cmsbuild , please test

tvami · 2022-02-17T15:49:26Z

+1

only comments were changed wrt to Changing LHCInfo O2O Implementation to access source data from OMS #36956 (comment)

cmsbuild · 2022-02-17T15:49:35Z

This pull request is fully signed and it will be integrated in one of the next master IBs after it passes the integration tests. 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)

cmsbuild · 2022-02-17T20:57:59Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1b812d/22486/summary.html
COMMIT: 539bd5f
CMSSW: CMSSW_12_3_X_2022-02-17-1100/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/36956/22486/install.sh to create a dev area with all the needed externals and cmssw changes.

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-1b812d/138.4_PromptCollisions+RunMinimumBias2021+ALCARECOPROMPTR3+HARVESTDPROMPTR3
/data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-1b812d/138.5_ExpressCollisions+RunMinimumBias2021+TIER0EXPRUN3+ALCARECOEXPR3+HARVESTDEXPR3
/data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-1b812d/139.001_RunMinimumBias2021+RunMinimumBias2021+HLTDR3_2021+RECODR3_MinBiasOffline+HARVESTD2021MB

Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 49
DQMHistoTests: Total histograms compared: 3965143
DQMHistoTests: Total failures: 2
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3965119
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
Checked 204 log files, 45 edm output root files, 49 DQM output files
TriggerResults: no differences found

perrotta · 2022-02-18T16:12:17Z

+1

ggovi added 4 commits February 14, 2022 13:45

Added Util class for data access on web services

7ea27bd

Expanded LHCInfo type to host new versions of Xing_angle and BetaStar…

fa4a518

… params

Prepared data access to OMS Service

b96ab48

Data access for Fill, Lumisection and various DIP information moved f…

ca4b3ba

…rom direct Oracle access to OMS Service

cmsbuild added this to the CMSSW_12_3_X milestone Feb 14, 2022

cmsbuild added alca-pending code-checks-pending db-pending orp-pending pending-signatures tests-pending labels Feb 14, 2022

code-checks/code-format

b6d2cf0

cmsbuild added code-checks-rejected and removed code-checks-pending labels Feb 14, 2022

cmsbuild added code-checks-pending and removed code-checks-rejected labels Feb 15, 2022

tvami reviewed Feb 15, 2022

View reviewed changes

CondCore/CondDB/src/WebUtils.cc Outdated

namespace cond {

//

Copy link

Contributor

tvami Feb 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this supposed to be a comment after the // ?

cmsbuild added code-checks-approved and removed code-checks-pending labels Feb 15, 2022

tvami reviewed Feb 15, 2022

View reviewed changes

cmsbuild added tests-started and removed tests-pending labels Feb 15, 2022

francescobrivio reviewed Feb 15, 2022

View reviewed changes

cmsbuild added alca-pending code-checks-pending tests-pending and removed tests-approved code-checks-approved labels Feb 17, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Feb 17, 2022

cmsbuild added tests-started and removed tests-pending labels Feb 17, 2022

cmsbuild added alca-approved db-approved fully-signed and removed alca-pending db-pending pending-signatures labels Feb 17, 2022

cmsbuild added tests-approved and removed tests-started labels Feb 17, 2022

cmsbuild added orp-approved and removed orp-pending labels Feb 18, 2022

cmsbuild merged commit f4b2b85 into cms-sw:master Feb 18, 2022

This was referenced Feb 18, 2022

[ROOT 6.24] Updated root to tip of branch v6-24-00-patches cms-sw/cmsdist#7603

Closed

Hgg photons variables in nanoAODs #36526

Merged

r626: TBranchElement: no drilling through new members. cms-sw/root#164

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Changing LHCInfo O2O Implementation to access source data from OMS #36956

Changing LHCInfo O2O Implementation to access source data from OMS #36956

ggovi commented Feb 14, 2022 •

edited

Loading

cmsbuild commented Feb 14, 2022

ggovi commented Feb 14, 2022

tvami commented Feb 15, 2022

tvami Feb 15, 2022

cmsbuild commented Feb 15, 2022

cmsbuild commented Feb 15, 2022

tvami Feb 15, 2022

ggovi Feb 16, 2022

tvami Feb 16, 2022

tvami commented Feb 15, 2022

francescobrivio Feb 15, 2022

ggovi Feb 16, 2022

francescobrivio Feb 16, 2022

cmsbuild commented Feb 15, 2022

cmsbuild commented Feb 17, 2022

cmsbuild commented Feb 17, 2022

tvami commented Feb 17, 2022

tvami commented Feb 17, 2022

cmsbuild commented Feb 17, 2022

cmsbuild commented Feb 17, 2022

perrotta commented Feb 18, 2022

Changing LHCInfo O2O Implementation to access source data from OMS #36956

Changing LHCInfo O2O Implementation to access source data from OMS #36956

Conversation

ggovi commented Feb 14, 2022 • edited Loading

PR description:

PR validation:

cmsbuild commented Feb 14, 2022

ggovi commented Feb 14, 2022

tvami commented Feb 15, 2022

tvami Feb 15, 2022

Choose a reason for hiding this comment

cmsbuild commented Feb 15, 2022

cmsbuild commented Feb 15, 2022

tvami Feb 15, 2022

Choose a reason for hiding this comment

ggovi Feb 16, 2022

Choose a reason for hiding this comment

tvami Feb 16, 2022

Choose a reason for hiding this comment

tvami commented Feb 15, 2022

francescobrivio Feb 15, 2022

Choose a reason for hiding this comment

ggovi Feb 16, 2022

Choose a reason for hiding this comment

francescobrivio Feb 16, 2022

Choose a reason for hiding this comment

cmsbuild commented Feb 15, 2022

Comparison Summary

cmsbuild commented Feb 17, 2022

cmsbuild commented Feb 17, 2022

tvami commented Feb 17, 2022

tvami commented Feb 17, 2022

cmsbuild commented Feb 17, 2022

cmsbuild commented Feb 17, 2022

Comparison Summary

perrotta commented Feb 18, 2022

ggovi commented Feb 14, 2022 •

edited

Loading