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

Developments for DQM Core must be tested also on Online DQM clients before deploying/merging #41713

Open
sroychow opened this issue May 17, 2023 · 8 comments

Comments

@sroychow
Copy link
Contributor

In the recent collision runs, several MEs were not displayed on Online GUI( see link). Upon iterating with Central DQM experts, it was understood that the problem is caused by #41400 -

  • So when an Online ME is listed as a per-LS Monitoring Element as well, plots in the Online live mode are snapshots of every LS and in the end they are not properly merged. DQM experts are looking into the cause for this. They have deployed a temporary fix since run 367657 where all online MEs are removed from per LS ME config.

With the above motivation, the proposal is the following:-

Whenever a change to DQMStore or any DQM Core is made, the Online clients should also be tested on top of the proposed changes before the relevant PRs are merged.

@cmsbuild
Copy link
Contributor

A new Issue was created by @sroychow Suvankar Roy Chowdhury.

@Dr15Jones, @perrotta, @dpiparo, @rappoccio, @makortel, @smuzaffar can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

@makortel
Copy link
Contributor

assign dqm

@cmsbuild
Copy link
Contributor

New categories assigned: dqm

@tjavaid,@micsucmed,@nothingface0,@rvenditti,@emanueleusai,@syuvivida,@pmandrik you have been requested to review this Pull request/Issue and eventually sign? Thanks

@makortel
Copy link
Contributor

(I'd like to understand a bit better the testing suggestion)

the Online clients should also be tested

The clients defined in DQM/Integration/python/clients are tested (as far as I know/recall) in the IBs (could also be tested in PRs), but none of those have shown any failure since the master PR #41196 was merged. Does that mean these tests are not sufficient (to reveal this kind of problems), they would need to be tested in the online environment, or did you mean something completely different?

@mmusich
Copy link
Contributor

mmusich commented May 17, 2023

Does that mean these tests are not sufficient (to reveal this kind of problems), they would need to be tested in the online environment, or did you mean something completely different?

I guess a DQM bin-by-bin comparison of the online client output might have caught the issue. The clients don't fail but (some of) the histograms are not there.

@syuvivida
Copy link
Contributor

+1
Agreed from the DQM side.

@cmsbuild
Copy link
Contributor

This issue is fully signed and ready to be closed.

@mmusich
Copy link
Contributor

mmusich commented May 18, 2023

+1
Agreed from the DQM side.

I natively would have thought that DQM would sign once a solution for the issue is proposed.

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

No branches or pull requests

5 participants