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

[dmitrijus]

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.

(discussion: #11086)

[cmsbuild]

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

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

It involves the following packages:

DQMServices/Components
DQMServices/Core
DQMServices/FileIO

The following packages do not have a category, yet:

DQMServices/FileIO

@cmsbuild, @danduggan, @vanbesien, @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]

please test

[cmsbuild]

The tests are being triggered in jenkins.

[fwyzard]

as a minimal fix for CMSSW 7.4.x, wouldn't it have been simpler to just make sure that the histograms marked for deletion are actually deleted, in the postGlobalEndLumi callback ?

[dmitrijus]

This PR was actually introduced a few weeks ago
with an idea to make this deletion simpler and to solve performance
issues reported by Srecko.

The memleak was caused by:
https://github.com/cms-sw/cmssw/blob/CMSSW_7_6_X/DQMServices/Core/src/DQMStore.cc#L486
'temp' should be checked, not 'i_lumi'.
This PR is removes that bit of the code and unintentionally fixes the memleak.

Dmitrijus

[cmsbuild]

+1
Tested at: 091bfda
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-11430/8275/summary.html

[deguio]

I discussed with andrea. let's wait to see the outcome of the test on the hilton then we can decide to merge it.
all the use cases should be supported (HLT, Online at p5). offline should be untouched.
let's keep open for now

[cmsbuild]

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-11430/8275/summary.html

[Sam-Harper]

So I ran the hilton tests on it and I still see what appears to be the same memory increase. I'm now going to double check I ran what I thought I ran. But just wanted to give you an update

[deguio]

thanks @Sam-Harper,
this is too bad. please let us know if this is confirmed.
thanks,
F.

[deguio]

+1
this one can go in at this point. it would be nice to have it included in the next 74x build.

[fwyzard]

ciao @deguio, after the comments from @Sam-Harper do we have any indications that this actually fixes the memory problem ?

[deguio]

ciao andrea,
this fixes one problem (inside DQM). the memory leak seen by sam is a root problem fixed by lorenzo already. see:
https://hypernews.cern.ch/HyperNews/CMS/get/edmFramework/3569/1.html

going to reply now asking to have the root we use patched with the fix.