Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce OnlinePopCon mechanism #44927

Merged
merged 1 commit into from
Nov 2, 2024

Conversation

francescobrivio
Copy link
Contributor

@francescobrivio francescobrivio commented May 8, 2024

PR description:

This PR introduces a new OnlinePopCon mechanism that will serve the O2Os aiming at writing lumi-based IOVs for HLT.
The implementation is following the same structure of the already existing PopCon mechanism:

and it is largely borrowed from the PopCon code itself, with the appropriate/minimal changes required for running the OnlineDBOutputService.

The first and only (for now) customer of this mechanism are the LHCInfoPerLS and LHCInfoPerFill O2O, when run in "duringFill mode".

Opening as draft since there are still a few open points to address:

  • Understand if m_dbService->forceInit(); (copied from PopCon) is needed also in OnlinePopCon
  • Add a unit-test
  • Confirm all the needed methods are present

PR Validation

Code compiles, but no other specific validation is done for now.

Backport:

Not a backport.
A backport for the 2024 data-taking release will be opened once we converge on this PR.


FYI @JanChyczynski

@cmsbuild
Copy link
Contributor

cmsbuild commented May 8, 2024

cms-bot internal usage

@cmsbuild
Copy link
Contributor

cmsbuild commented May 8, 2024

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44927/40203

  • This PR adds an extra 20KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented May 8, 2024

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

It involves the following packages:

  • CondCore/PopCon (alca, db)

@consuegs, @perrotta, @saumyaphor4252, @francescobrivio, @cmsbuild can you please review it and eventually sign? Thanks.
@mmusich, @yuanchao, @tocheng, @PonIlya, @rsreds, @JanChyczynski this is something you requested to watch as well.
@rappoccio, @sextonkennedy, @antoniovilela you are the release manager for this.

cms-bot commands are listed here

Copy link
Contributor

@JanChyczynski JanChyczynski left a comment

Choose a reason for hiding this comment

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

minors

CondCore/PopCon/interface/OnlinePopCon.h Outdated Show resolved Hide resolved
CondCore/PopCon/src/OnlinePopCon.cc Outdated Show resolved Hide resolved
CondCore/PopCon/src/OnlinePopCon.cc Show resolved Hide resolved
CondCore/PopCon/src/OnlinePopCon.cc Outdated Show resolved Hide resolved
@cmsbuild
Copy link
Contributor

cmsbuild commented May 8, 2024

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44927/40206

  • This PR adds an extra 20KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented May 8, 2024

Pull request #44927 was updated. @saumyaphor4252, @perrotta, @francescobrivio, @cmsbuild, @consuegs can you please check and sign again.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44927/40536

  • This PR adds an extra 20KB to repository

@cmsbuild
Copy link
Contributor

Pull request #44927 was updated. @francescobrivio, @saumyaphor4252, @cmsbuild, @consuegs, @perrotta can you please check and sign again.

@francescobrivio francescobrivio marked this pull request as ready for review June 12, 2024 14:26
@perrotta
Copy link
Contributor

please test
(das errors must be transient)

@cmsbuild
Copy link
Contributor

-1

Failed Tests: RelVals-INPUT
Size: This PR adds an extra 12KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c7f8c0/42452/summary.html
COMMIT: e279771
CMSSW: CMSSW_14_2_X_2024-10-29-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/44927/42452/install.sh to create a dev area with all the needed externals and cmssw changes.

  • DAS Queries: The DAS query tests failed, see the summary page for details.

RelVals-INPUT

  • 2024.0000012024.000001_RunJetMET02024D_10k/step1_dasquery.log
  • 2024.0010012024.001001_RunZeroBias2024D_10k/step1_dasquery.log
  • 2024.1000012024.100001_RunJetMET02024C_10k/step1_dasquery.log
Expand to see more relval errors ...
  • 2024.101001
  • 2024.000001
  • 2024.001001
  • 2024.100001
  • 2024.101001

Comparison Summary

Summary:

@JanChyczynski
Copy link
Contributor

I believe the relval failures are unrelated (failing in other recent PRs as well)

@JanChyczynski
Copy link
Contributor

Opened backport to 14_1_X: #46571

@perrotta
Copy link
Contributor

perrotta commented Nov 2, 2024

+1

  • RelVal failures with das errors are unrelated

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 2, 2024

This pull request is fully signed and it will be integrated in one of the next master IBs (but tests are reportedly failing). This pull request will now be reviewed by the release team before it's merged. @sextonkennedy, @antoniovilela, @rappoccio, @mandrenguyen (and backports should be raised in the release meeting by the corresponding L2)

@mandrenguyen
Copy link
Contributor

ignore tests-rejected with ib-failure

@mandrenguyen
Copy link
Contributor

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants