Navigation Menu

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

Replace mark-and-delete at next merge algorithm in DQMStore (75x) #11086

Merged
merged 1 commit into from Nov 3, 2015

Conversation

dmitrijus
Copy link
Contributor

Additionally, the DQMStore will now acquire locks for writing files.

This PR also add a new file saver for HLT->PB (DQMFileSaverPB).
Which should replace DQMFileSaver in the near future.

Please do not accept this PR until I say okay.

…s now.

Additionally, the dqm store will now acquire locks for writing files.

Also add a new file saver for HLT->PB (DQMFileSaverPB).
Which should replace DQMFileSaver in the near future.
@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 2, 2015

A new Pull Request was created by @dmitrijus (Dmitrijus) for CMSSW_7_5_X.

Replace mark-and-delete at next merge algorithm in DQMStore (75x)

It involves the following packages:

DQMServices/Components
DQMServices/Core
DQMServices/FileIO

The following packages do not have a category, yet:

DQMServices/FileIO

@cmsbuild, @danduggan, @deguio can you please review it and eventually sign? Thanks.
@barvic this is something you requested to watch as well.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
If you are a L2 or a release manager you can ask for tests by saying 'please test' in the first line of a comment.
@Degano you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@deguio
Copy link
Contributor

deguio commented Sep 2, 2015

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 2, 2015

The tests are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 2, 2015

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 2, 2015

@dmitrijus
Copy link
Contributor Author

Ready to go in

@fwyzard
Copy link
Contributor

fwyzard commented Sep 23, 2015

I know that this is under DQM responsibility, but since the changes are intended explicitly for the HLT, let me ask...

Why do you think a dedicated DQMFileSaver module for .pb files is a better approach ?
Doesn't it increase code duplication, and make it easier for the online and offline code base to go out of sync ?

@dmitrijus
Copy link
Contributor Author

If you look at the DQMFileSaver, the code is not even related to the file saving,
but rather the glue code needed for each use case.
The actual saving is done by calling DQMStore->save{PB}.

As it is now, there is a huge mess of at least 7 parameters (and only a few of them are used for a given workflow). With this approach, we isolate each parameter and functionally to a specific workflow.

Doesn't it increase code duplication?

No, since this was essentially a glue code, it is very specific to a workflow.
For example, for .pb output we also write .json files and this is only true for .pb.
No other workflow writes a .json files.

Make it easier for the online and offline code base to go out of sync?

On contrary, this will make maintaining the code way easier:
we will be able to modify any workflow without affecting the other.

Even though saver for .pb is a separate module, it still has a common base (DQMFileSaverBase).
Our plan is to have 3 modules:

  • DQMFileSaverOnline -> for DQM saving at our online cluster, it is already done.
    It implements some "weird" features like writing origin files and making periodical backups;
  • DQMFileSaverPB -> for DQM output at HLT (ie .pb output);
  • DQMFileSaverOffline -> for the offline workflows (not yet done).

Old DQMFileSaver will be deprecated/removed once we migrate each workflow.

By the way, HLT does not have to switch to DQMFileSaverPB yet.
The deletion fix (main reason for this PR) applies to both old and new savers.

@fwyzard
Copy link
Contributor

fwyzard commented Sep 24, 2015 via email

@deguio
Copy link
Contributor

deguio commented Sep 24, 2015

+1
ups

davidlange6 added a commit that referenced this pull request Nov 3, 2015
Replace mark-and-delete at next merge algorithm in DQMStore (75x)
@davidlange6 davidlange6 merged commit 245df3a into cms-sw:CMSSW_7_5_X Nov 3, 2015
This was referenced Dec 13, 2019
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.

None yet

5 participants