cms-sw · davidlange6 · Nov 3, 2015 · Sep 2, 2015
diff --git a/DQMServices/Components/src/DQMFileSaver.cc b/DQMServices/Components/src/DQMFileSaver.cc
@@ -735,7 +735,7 @@ DQMFileSaver::globalEndLuminosityBlock(const edm::LuminosityBlock & iLS, const e
     }
 
     // after saving per LS, delete the old LS global histograms.
-    dbe_->markForDeletion(enableMultiThread_ ? irun : 0, ilumi);
+    dbe_->deleteUnusedLumiHistograms(enableMultiThread_ ? irun : 0, ilumi);
   }
 }
 

diff --git a/DQMServices/Core/interface/DQMStore.h b/DQMServices/Core/interface/DQMStore.h
@@ -629,7 +629,7 @@ class DQMStore
 					      uint32_t streamId,
 					      uint32_t moduleId);
 
-  void markForDeletion(uint32_t run, uint32_t lumi);
+  void deleteUnusedLumiHistograms(uint32_t run, uint32_t lumi);
  private:
 
   // ---------------- Miscellaneous -----------------------------

diff --git a/DQMServices/Core/src/DQMStore.cc b/DQMServices/Core/src/DQMStore.cc
@@ -476,18 +476,6 @@ void DQMStore::mergeAndResetMEsLuminositySummaryCache(uint32_t run,
     // make the ME reusable for the next LS
     const_cast<MonitorElement*>(&*i)->Reset();
     ++i;
-
-    // check and remove the global lumi based histo belonging to the previous LSs
-    // if properly flagged as DQMNet::DQM_PROP_MARKTODELETE
-    global_me.setLumi(1);
-    std::set<MonitorElement>::const_iterator i_lumi = data_.lower_bound(global_me);
-    while (i_lumi->data_.lumi != lumi) {
-      auto temp = i_lumi++;
-      if (i_lumi->getName() == i->getName() && i_lumi->getPathname() == i->getPathname() &&  i_lumi->markedToDelete())
-	{
-	  data_.erase(temp);
-	}
-    }
   }
 }
 
@@ -2098,11 +2086,14 @@ DQMStore::forceReset(void)
 //////////////////////////////////////////////////////////////////////
 //////////////////////////////////////////////////////////////////////
 //////////////////////////////////////////////////////////////////////
-/** Mark a set of histograms for deletion based on run, lumi and path*/
+/** Delete *global* histograms which are no longer in used.
+ * Such histograms are created at the end of each lumi and should be
+ * deleted after last globalEndLuminosityBlock.
+ */
 void
-DQMStore::markForDeletion(uint32_t run,
-			  uint32_t lumi)
+DQMStore::deleteUnusedLumiHistograms(uint32_t run, uint32_t lumi)
 {
+  std::lock_guard<std::mutex> guard(book_mutex_);
 
   std::string null_str("");
   MonitorElement proto(&null_str, null_str, run, 0, 0);
@@ -2114,25 +2105,26 @@ DQMStore::markForDeletion(uint32_t run,
 
   while (i != e) {
     if (i->data_.streamId != 0 ||
-	i->data_.moduleId != 0)
+        i->data_.moduleId != 0)
       break;
     if ((i->data_.lumi != lumi) && enableMultiThread_)
       break;
     if (i->data_.run != run)
       break;
 
-    const_cast<MonitorElement*>(&*i)->markToDelete();  
-
-    if (verbose_ > 1)
-      std::cout << "DQMStore::markForDeletion: marked monitor element '"
-		<< *i->data_.dirname << "/" << i->data_.objname << "'"
-		<< "flags " << i->data_.flags << "\n";
-
+    auto temp = i;
     ++i;
+
+    if (verbose_ > 1) {
+      std::cout << "DQMStore::deleteUnusedLumiHistograms: deleted monitor element '"
+                << *i->data_.dirname << "/" << i->data_.objname << "'"
+                << "flags " << i->data_.flags << "\n";
+    } 
+
+    data_.erase(temp);
   }
 }
 
-
 //////////////////////////////////////////////////////////////////////
 //////////////////////////////////////////////////////////////////////
 //////////////////////////////////////////////////////////////////////
@@ -2487,6 +2479,8 @@ void DQMStore::savePB(const std::string &filename,
   using google::protobuf::io::GzipOutputStream;
   using google::protobuf::io::StringOutputStream;
 
+  std::lock_guard<std::mutex> guard(book_mutex_);
+
   std::set<std::string>::iterator di, de;
   MEMap::iterator mi, me = data_.end();
   dqmstorepb::ROOTFilePB dqmstore_message;
@@ -2616,6 +2610,8 @@ DQMStore::save(const std::string &filename,
                const std::string &fileupdate /* = RECREATE */,
 	       const bool resetMEsAfterWriting /* = false */)
 {
+  std::lock_guard<std::mutex> guard(book_mutex_);
+
   std::set<std::string>::iterator di, de;
   MEMap::iterator mi, me = data_.end();
   DQMNet::QReports::const_iterator qi, qe;

diff --git a/DQMServices/FileIO/plugins/DQMFileSaverBase.cc b/DQMServices/FileIO/plugins/DQMFileSaverBase.cc
@@ -23,10 +23,6 @@
 #include <TString.h>
 #include <TSystem.h>
 
-#include <openssl/md5.h>
-#include <boost/iostreams/device/mapped_file.hpp>
-#include <boost/filesystem.hpp>
-
 using namespace dqm;
 
 DQMFileSaverBase::DQMFileSaverBase(const edm::ParameterSet &ps) {
@@ -72,11 +68,11 @@ DQMFileSaverBase::DQMFileSaverBase(const edm::ParameterSet &ps) {
 
 DQMFileSaverBase::~DQMFileSaverBase() {}
 
-void DQMFileSaverBase::beginJob() {}
-
 std::shared_ptr<NoCache> DQMFileSaverBase::globalBeginRun(
     const edm::Run &r, const edm::EventSetup &) const {
 
+  this->initRun();  
+
   return nullptr;
 }
 
@@ -103,11 +99,10 @@ void DQMFileSaverBase::globalEndLuminosityBlock(const edm::LuminosityBlock &iLS,
   fp.lumi_ = ilumi;
   fp.run_ = irun;
 
-  edm::Service<DQMStore> store;
-
   this->saveLumi(fp);
 
-  store->markForDeletion(store->mtEnabled() ? irun : 0, ilumi);
+  edm::Service<DQMStore> store;
+  store->deleteUnusedLumiHistograms(store->mtEnabled() ? irun : 0, ilumi);
 }
 
 void DQMFileSaverBase::globalEndRun(const edm::Run &iRun,
@@ -123,8 +118,6 @@ void DQMFileSaverBase::globalEndRun(const edm::Run &iRun,
   this->saveRun(fp);
 }
 
-void DQMFileSaverBase::endJob(void) {}
-
 void DQMFileSaverBase::postForkReacquireResources(
     unsigned int childIndex, unsigned int numberOfChildren) {
   // this is copied from IOPool/Output/src/PoolOutputModule.cc, for consistency
@@ -145,7 +138,7 @@ void DQMFileSaverBase::postForkReacquireResources(
   initial_fp_.child_ = std::string(buffer);
 }
 
-const std::string DQMFileSaverBase::filename(FileParameters fp, bool useLumi) {
+const std::string DQMFileSaverBase::filename(const FileParameters& fp, bool useLumi) {
   char buf[256];
   if (useLumi) {
     snprintf(buf, 256, "%s_V%04d_%s_R%09ld_L%09ld%s", fp.producer_.c_str(),
@@ -164,99 +157,6 @@ const std::string DQMFileSaverBase::filename(FileParameters fp, bool useLumi) {
   return (path / file).string();
 }
 
-// file metadata saving stuff
-boost::property_tree::ptree
-DQMFileSaverBase::fillJson(int run, int lumi, const std::string& dataFilePathName, const std::string transferDestinationStr, evf::FastMonitoringService *fms)
-{
-  namespace bpt = boost::property_tree;
-  namespace bfs = boost::filesystem;
-
-  bpt::ptree pt;
-
-  int hostnameReturn;
-  char host[32];
-  hostnameReturn = gethostname(host ,sizeof(host));
-  if (hostnameReturn == -1)
-    throw cms::Exception("fillJson")
-          << "Internal error, cannot get host name";
-
-  int pid = getpid();
-  std::ostringstream oss_pid;
-  oss_pid << pid;
-
-  // Stat the data file: if not there, throw
-  struct stat dataFileStat;
-  if (stat(dataFilePathName.c_str(), &dataFileStat) != 0)
-    throw cms::Exception("fillJson")
-          << "Internal error, cannot get data file: "
-          << dataFilePathName;
-  // Extract only the data file name from the full path
-  std::string dataFileName = bfs::path(dataFilePathName).filename().string();
-  // The availability test of the FastMonitoringService was done in the ctor.
-  bpt::ptree data;
-  bpt::ptree processedEvents, acceptedEvents, errorEvents, bitmask, fileList, fileSize, inputFiles, fileAdler32, transferDestination;
-
-  processedEvents.put("", fms ? (fms->getEventsProcessedForLumi(lumi)) : -1); // Processed events
-  acceptedEvents.put("", fms ? (fms->getEventsProcessedForLumi(lumi)) : -1); // Accepted events, same as processed for our purposes
-
-  errorEvents.put("", 0); // Error events
-  bitmask.put("", 0); // Bitmask of abs of CMSSW return code
-  fileList.put("", dataFileName); // Data file the information refers to
-  fileSize.put("", dataFileStat.st_size); // Size in bytes of the data file
-  inputFiles.put("", ""); // We do not care about input files!
-  fileAdler32.put("", -1); // placeholder to match output json definition
-  transferDestination.put("", transferDestinationStr); // SM Transfer destination field
-
-  data.push_back(std::make_pair("", processedEvents));
-  data.push_back(std::make_pair("", acceptedEvents));
-  data.push_back(std::make_pair("", errorEvents));
-  data.push_back(std::make_pair("", bitmask));
-  data.push_back(std::make_pair("", fileList));
-  data.push_back(std::make_pair("", fileSize));
-  data.push_back(std::make_pair("", inputFiles));
-  data.push_back(std::make_pair("", fileAdler32));
-  data.push_back(std::make_pair("", transferDestination));
-
-  pt.add_child("data", data);
-
-  if (fms == nullptr) {
-    pt.put("definition", "/fakeDefinition.jsn");
-  } else {
-    // The availability test of the EvFDaqDirector Service was done in the ctor.
-    bfs::path outJsonDefName(edm::Service<evf::EvFDaqDirector>()->baseRunDir()); //we assume this file is written bu the EvF Output module
-    outJsonDefName /= (std::string("output_") + oss_pid.str() + std::string(".jsd"));
-    pt.put("definition", outJsonDefName.string());
-  }
-
-  char sourceInfo[64]; //host and pid information
-  sprintf(sourceInfo, "%s_%d", host, pid);
-  pt.put("source", sourceInfo);
-
-  return pt;
-}
-
-const std::string DQMFileSaverBase::fillOrigin(const std::string filename,
-                                  const std::string final_filename) {
-
-    // format.origin (one line):
-    //   md5:d566a34b27f48d507150a332b189398b 294835 final_filename.root
-
-    unsigned char md5[MD5_DIGEST_LENGTH];
-
-    boost::iostreams::mapped_file_source fp(filename);
-
-    MD5((unsigned char *)fp.data(), fp.size(), md5);
-
-    std::ostringstream hash;
-    for (int i = 0; i < MD5_DIGEST_LENGTH; ++i) {
-      hash << std::hex << std::setfill('0') << std::setw(2) << (int)(md5[i]);
-    }
-
-    std::ostringstream out;
-    out << "md5:" << hash.str() << " " << fp.size() << " " << final_filename;
-    return out.str();
-}
-
 void DQMFileSaverBase::saveJobReport(const std::string &filename) const
 {
   // Report the file to job report service.

diff --git a/DQMServices/FileIO/plugins/DQMFileSaverBase.h b/DQMServices/FileIO/plugins/DQMFileSaverBase.h
@@ -45,38 +45,34 @@ class DQMFileSaverBase
   };
 
  protected:
-  virtual void beginJob(void);
+  //virtual void beginJob(void) const override final;
+  //virtual void endJob(void) const override final;
+
   virtual std::shared_ptr<NoCache> globalBeginRun(
-      const edm::Run &, const edm::EventSetup &) const;
+      const edm::Run &, const edm::EventSetup &) const override final;
+
   virtual std::shared_ptr<NoCache> globalBeginLuminosityBlock(
-      const edm::LuminosityBlock &, const edm::EventSetup &) const;
+      const edm::LuminosityBlock &, const edm::EventSetup &) const override final;
+
   virtual void analyze(edm::StreamID, const edm::Event &e,
-                       const edm::EventSetup &) const;
+                       const edm::EventSetup &) const override final;
+
   virtual void globalEndLuminosityBlock(const edm::LuminosityBlock &,
-                                        const edm::EventSetup &) const;
-  virtual void globalEndRun(const edm::Run &, const edm::EventSetup &) const;
-  virtual void endJob(void);
+                                        const edm::EventSetup &) const override final ;
+  virtual void globalEndRun(const edm::Run &, const edm::EventSetup &) const override final;
+
   virtual void postForkReacquireResources(unsigned int childIndex,
                                           unsigned int numberOfChildren);
 
-  // these two should be overwritten
-  // in some cases, hsitograms are deleted after saving
+  // these method (and only these) should be overriden
   // so we need to call all file savers
-  virtual void saveLumi(FileParameters fp) const {};
-  virtual void saveRun(FileParameters fp) const {};
-
-  static const std::string filename(FileParameters fp, bool useLumi = false);
-
-  // also used by the JsonWritingTimedPoolOutputModule,
-  // fms will be nullptr in such case
-  static boost::property_tree::ptree fillJson(
-      int run, int lumi, const std::string &dataFilePathName, const std::string transferDestinationStr,
-      evf::FastMonitoringService *fms);
+  virtual void initRun(void) const {};
+  virtual void saveLumi(const FileParameters& fp) const {};
+  virtual void saveRun(const FileParameters& fp) const {};
 
-  static const std::string fillOrigin(const std::string filename,
-                                  const std::string final_filename);
+  static const std::string filename(const FileParameters& fp, bool useLumi = false);
 
-  // utilities
+    // utilities
   void logFileAction(const std::string& msg, const std::string& fileName) const;
   void saveJobReport(const std::string &filename) const;
 

diff --git a/DQMServices/FileIO/plugins/DQMFileSaverOnline.cc b/DQMServices/FileIO/plugins/DQMFileSaverOnline.cc
@@ -22,6 +22,10 @@
 #include <TString.h>
 #include <TSystem.h>
 
+#include <openssl/md5.h>
+#include <boost/iostreams/device/mapped_file.hpp>
+#include <boost/filesystem.hpp>
+
 using namespace dqm;
 
 DQMFileSaverOnline::DQMFileSaverOnline(const edm::ParameterSet &ps)
@@ -32,7 +36,7 @@ DQMFileSaverOnline::DQMFileSaverOnline(const edm::ParameterSet &ps)
 
 DQMFileSaverOnline::~DQMFileSaverOnline() {}
 
-void DQMFileSaverOnline::saveLumi(FileParameters fp) const {
+void DQMFileSaverOnline::saveLumi(const FileParameters& fp) const {
   if (backupLumiCount_ > 0) {
     if (fp.lumi_ % backupLumiCount_ == 0) {
 
@@ -42,7 +46,7 @@ void DQMFileSaverOnline::saveLumi(FileParameters fp) const {
   }
 }
 
-void DQMFileSaverOnline::saveRun(FileParameters fp) const {
+void DQMFileSaverOnline::saveRun(const FileParameters& fp) const {
   makeSnapshot(fp, true);
 }
 
@@ -91,7 +95,7 @@ void DQMFileSaverOnline::makeSnapshot(const FileParameters& fp, bool final) cons
   // write metadata
   // format.origin: md5:d566a34b27f48d507150a332b189398b 294835 /home/dqmprolocal/output/DQM_V0001_FED_R000194224.root
   std::ofstream meta_fd(tmp_meta_fp);
-  meta_fd << this->fillOrigin(tmp_root_fp, root_fp);
+  meta_fd << fillOrigin(tmp_root_fp, root_fp);
   meta_fd.close();
 
   checkError("Rename failed: ", root_fp, ::rename(tmp_root_fp.c_str(), root_fp.c_str()));
@@ -136,6 +140,30 @@ void DQMFileSaverOnline::checkError(const char *msg, const std::string file, int
   }
 }
 
+const std::string DQMFileSaverOnline::fillOrigin(const std::string filename,
+                                  const std::string final_filename) {
+
+    // format.origin (one line):
+    //   md5:d566a34b27f48d507150a332b189398b 294835 final_filename.root
+
+    unsigned char md5[MD5_DIGEST_LENGTH];
+
+    boost::iostreams::mapped_file_source fp(filename);
+
+    MD5((unsigned char *)fp.data(), fp.size(), md5);
+
+    std::ostringstream hash;
+    for (int i = 0; i < MD5_DIGEST_LENGTH; ++i) {
+      hash << std::hex << std::setfill('0') << std::setw(2) << (int)(md5[i]);
+    }
+
+    std::ostringstream out;
+    out << "md5:" << hash.str() << " " << fp.size() << " " << final_filename;
+    return out.str();
+}
+
+
+
 void DQMFileSaverOnline::fillDescriptions(
     edm::ConfigurationDescriptions& descriptions) {