Repository: dmwm/wmcore
Issue: JobAccountant crashing with "Column 'parent' cannot be null"

**Impact of the bug**
WMAgent, only submit6 so far

**Describe the bug**
JobAccountant fails to insert the parentage information into `dbsbuffer_file_parent` table because one of the binds results in a `null` result. Which means that file never got inserted into `dbsbuffer_file`.

**How to reproduce it**
This query
```
SELECT id FROM dbsbuffer_file WHERE lfn = "/store/mc/RunIISummer19UL17RECO/QCD_Pt-80to120_EMEnriched_TuneCP5_13TeV_pythia8/AODSIM/106X_mc2017_realistic_v6-v2/260000/7C4B69DF-FCEE-DE4D-A78B-53A41F1CFF3D.root";
```

doesn't result in any id.

**Expected behavior**
A pair of child/parent files to be inserted into DBSBuffer should always match files already inserted into `dbsbuffer_file`

**Additional context and error message**
Here's the error message and traceback:
```
2019-12-11 14:49:00,289:140252877453056:INFO:AccountantWorker:About to commit all DBSBuffer Heritage information
2019-12-11 14:49:00,289:140252877453056:INFO:AccountantWorker:9
2019-12-11 14:49:00,292:140252877453056:ERROR:AccountantWorker:Error while trying to handle the DBS LFN heritage
(_mysql_exceptions.OperationalError) (1048, "Column 'parent' cannot be null")
[SQL: INSERT INTO dbsbuffer_file_parent (child, parent) VALUES
               ((SELECT id FROM dbsbuffer_file WHERE lfn = %s),
               (SELECT id FROM dbsbuffer_file WHERE lfn = %s))]
[parameters: [('/store/mc/RunIISummer19UL17RECO/QCD_Pt-80to120_EMEnriched_TuneCP5_13TeV_pythia8/AODSIM/106X_mc2017_realistic_v6-v2/260000/7C4B69DF-FCEE-DE4D-A78B-53A41F1CFF3D.root', '/store/mc/RunIISummer19UL17GEN/QCD_Pt-80to120_EMEnriched_TuneCP5_13TeV_pythia8/GEN/106X_mc2017_realistic_v3-v1/260000/B714136F-9028-6240-B611-BEFCCDC6B961.root'), ('/store/mc/RunIISummer19UL17RECO/QCD_Pt-80to120_EMEnriched_TuneCP5_13TeV_pythia8/AODSIM/106X_mc2017_realistic_v6-v2/260000/7C4B69DF-FCEE-DE4D-A78B-53A41F1CFF3D.root', '/store/mc/RunIISummer19UL17GEN/QCD_Pt-80to120_EMEnriched_TuneCP5_13TeV_pythia8/GEN/106X_mc2017_realistic_v3-v1/260000/484271DC-4928-7943-829B-B56335EC6B62.root'), ('/store/mc/RunIISummer16MiniAODv3/TT_TuneCUETP8M2T4_13TeV-powheg-isrdown-pythia8/MINIAODSIM/94X_mcRun2_asymptotic_v3_ext3-v1/260000/50307203-E41B-EA11-A0DB-0025905B85F6.root', '/store/mc/RunIISummer16DR80/TT_TuneCUETP8M2T4_13TeV-powheg-isrdown-pythia8/AODSIM/80X_mcRun2_asymptotic_2016_TrancheIV_v6_ext3-v1/2610000/0C395487-2D1B-EA11-85F1-0CC47A4D7658.root'), ('/store/mc/RunIISummer16MiniAODv3/TT_TuneCUETP8M2T4_13TeV-powheg-isrdown-pythia8/MINIAODSIM/94X_mcRun2_asymptotic_v3_ext3-v1/260000/50307203-E41B-EA11-A0DB-0025905B85F6.root', '/store/mc/RunIISummer16DR80/TT_TuneCUETP8M2T4_13TeV-powheg-isrdown-pythia8/AODSIM/80X_mcRun2_asymptotic_2016_TrancheIV_v6_ext3-v1/2610000/34061A43-D91B-EA11-8C4E-0CC47A4D76D2.root'), ('/store/mc/RunIISummer16MiniAODv3/TT_TuneCUETP8M2T4_13TeV-powheg-isrdown-pythia8/MINIAODSIM/94X_mcRun2_asymptotic_v3_ext3-v1/260000/50307203-E41B-EA11-A0DB-0025905B85F6.root', '/store/mc/RunIISummer16DR80/TT_TuneCUETP8M2T4_13TeV-powheg-isrdown-pythia8/AODSIM/80X_mcRun2_asymptotic_2016_TrancheIV_v6_ext3-v1/2610000/46C16482-2D1B-EA11-A371-0025905B8576.root'), ('/store/mc/RunIISummer16MiniAODv3/TT_TuneCUETP8M2T4_13TeV-powheg-isrdown-pythia8/MINIAODSIM/94X_mcRun2_asymptotic_v3_ext3-v1/260000/50307203-E41B-EA11-A0DB-0025905B85F6.root', '/store/mc/RunIISummer16DR80/TT_TuneCUETP8M2T4_13TeV-powheg-isrdown-pythia8/AODSIM/80X_mcRun2_asymptotic_2016_TrancheIV_v6_ext3-v1/2610000/0CAB4583-2D1B-EA11-B519-0CC47A745294.root'), ('/store/mc/RunIISummer16MiniAODv3/TT_TuneCUETP8M2T4_13TeV-powheg-isrdown-pythia8/MINIAODSIM/94X_mcRun2_asymptotic_v3_ext3-v1/260000/50307203-E41B-EA11-A0DB-0025905B85F6.root', '/store/mc/RunIISummer16DR80/TT_TuneCUETP8M2T4_13TeV-powheg-isrdown-pythia8/AODSIM/80X_mcRun2_asymptotic_2016_TrancheIV_v6_ext3-v1/2610000/9085319D-2D1B-EA11-8BC4-0025905B85D6.root'), ('/store/mc/RunIISummer16MiniAODv3/TT_TuneCUETP8M2T4_13TeV-powheg-isrdown-pythia8/MINIAODSIM/94X_mcRun2_asymptotic_v3_ext3-v1/260000/50307203-E41B-EA11-A0DB-0025905B85F6.root', '/store/mc/RunIISummer16DR80/TT_TuneCUETP8M2T4_13TeV-powheg-isrdown-pythia8/AODSIM/80X_mcRun2_asymptotic_2016_TrancheIV_v6_ext3-v1/2610000/EE759784-2D1B-EA11-93E0-0025905B8568.root'), ('/store/mc/RunIISummer16MiniAODv3/TT_TuneCUETP8M2T4_13TeV-powheg-isrdown-pythia8/MINIAODSIM/94X_mcRun2_asymptotic_v3_ext3-v1/260000/50307203-E41B-EA11-A0DB-0025905B85F6.root', '/store/mc/RunIISummer16DR80/TT_TuneCUETP8M2T4_13TeV-powheg-isrdown-pythia8/AODSIM/80X_mcRun2_asymptotic_2016_TrancheIV_v6_ext3-v1/2610000/128CE182-2D1B-EA11-B399-0CC47A4D75EE.root')]]
...
        ModuleName : WMComponent.JobAccountant.AccountantWorker
        MethodName : handleDBSBufferParentage
        ClassInstance : None
        FileName : /data/srv/wmagent/v1.2.8/sw/slc7_amd64_gcc630/cms/wmagent/1.2.8/lib/python2.7/site-packages/WMComponent/JobAccountant/AccountantWorker.py
        ClassName : None
        LineNumber : 922
        ErrorNr : 0

Traceback: 
  File "/data/srv/wmagent/v1.2.8/sw/slc7_amd64_gcc630/cms/wmagent/1.2.8/lib/python2.7/site-packages/WMComponent/JobAccountant/AccountantWorker.py", line 914, in handleDBSBufferParentage
    transaction=self.existingTransaction())

  File "/data/srv/wmagent/v1.2.8/sw/slc7_amd64_gcc630/cms/wmagent/1.2.8/lib/python2.7/site-packages/WMComponent/DBS3Buffer/MySQL/DBSBufferFiles/BulkHeritageParent.py", line 23, in execute
    transaction = transaction)

  File "/data/srv/wmagent/v1.2.8/sw/slc7_amd64_gcc630/cms/wmagent/1.2.8/lib/python2.7/site-packages/WMCore/Database/DBCore.py", line 160, in processData
    connection=connection, returnCursor=returnCursor))

  File "/data/srv/wmagent/v1.2.8/sw/slc7_amd64_gcc630/cms/wmagent/1.2.8/lib/python2.7/site-packages/WMCore/Database/MySQLCore.py", line 140, in executemanybinds
    returnCursor)

  File "/data/srv/wmagent/v1.2.8/sw/slc7_amd64_gcc630/cms/wmagent/1.2.8/lib/python2.7/site-packages/WMCore/Database/DBCore.py", line 113, in executemanybinds
    result = connection.execute(s, b)

  File "/data/srv/wmagent/v1.2.8/sw/slc7_amd64_gcc630/external/py2-sqlalchemy/1.3.3/lib/python2.7/site-packages/sqlalchemy/engine/base.py", line 982, in execute
    return self._execute_text(object_, multiparams, params)

  File "/data/srv/wmagent/v1.2.8/sw/slc7_amd64_gcc630/external/py2-sqlalchemy/1.3.3/lib/python2.7/site-packages/sqlalchemy/engine/base.py", line 1155, in _execute_text
    parameters,

  File "/data/srv/wmagent/v1.2.8/sw/slc7_amd64_gcc630/external/py2-sqlalchemy/1.3.3/lib/python2.7/site-packages/sqlalchemy/engine/base.py", line 1248, in _execute_context
    e, statement, parameters, cursor, context

  File "/data/srv/wmagent/v1.2.8/sw/slc7_amd64_gcc630/external/py2-sqlalchemy/1.3.3/lib/python2.7/site-packages/sqlalchemy/engine/base.py", line 1466, in _handle_dbapi_exception
    util.raise_from_cause(sqlalchemy_exception, exc_info)

  File "/data/srv/wmagent/v1.2.8/sw/slc7_amd64_gcc630/external/py2-sqlalchemy/1.3.3/lib/python2.7/site-packages/sqlalchemy/util/compat.py", line 383, in raise_from_cause
    reraise(type(exception), exception, tb=exc_tb, cause=cause)

  File "/data/srv/wmagent/v1.2.8/sw/slc7_amd64_gcc630/external/py2-sqlalchemy/1.3.3/lib/python2.7/site-packages/sqlalchemy/engine/base.py", line 1224, in _execute_context
    cursor, statement, parameters, context

  File "/data/srv/wmagent/v1.2.8/sw/slc7_amd64_gcc630/external/py2-sqlalchemy/1.3.3/lib/python2.7/site-packages/sqlalchemy/dialects/mysql/mysqldb.py", line 132, in do_executemany
    rowcount = cursor.executemany(statement, parameters)

  File "/data/srv/wmagent/v1.2.8/sw/slc7_amd64_gcc630/external/py2-mysqldb/1.2.4b4-comp2/lib/python2.7/site-packages/MySQLdb/cursors.py", line 253, in executemany
    r = self._query('\\n'.join([query[:p], ',\\n'.join(q), query[e:]]))

  File "/data/srv/wmagent/v1.2.8/sw/slc7_amd64_gcc630/external/py2-mysqldb/1.2.4b4-comp2/lib/python2.7/site-packages/MySQLdb/cursors.py", line 346, in _query
    rowcount = self._do_query(q)

  File "/data/srv/wmagent/v1.2.8/sw/slc7_amd64_gcc630/external/py2-mysqldb/1.2.4b4-comp2/lib/python2.7/site-packages/MySQLdb/cursors.py", line 310, in _do_query
    db.query(q)
```


Changed files:
src/python/WMCore/WMBS/Oracle/Jobs/LoadForErrorHandler.py, src/python/WMComponent/JobAccountant/AccountantWorker.py, src/python/WMCore/WMBS/MySQL/Jobs/LoadForErrorHandler.py, src/python/WMCore/WorkQueue/WMBSHelper.py

diff --git a/src/python/WMComponent/JobAccountant/AccountantWorker.py b/src/python/WMComponent/JobAccountant/AccountantWorker.py
index bea536400..739d122bc 100644
--- a/src/python/WMComponent/JobAccountant/AccountantWorker.py
+++ b/src/python/WMComponent/JobAccountant/AccountantWorker.py
@@ -379,11 +379,12 @@ class AccountantWorker(WMConnectionBase):
                                                        conn=self.getDBConn(),
                                                        transaction=self.existingTransaction())
         newParents = set()
+        logging.debug("Found %d potential parents for lfn: %s", len(parentsInfo), lfn)
         for parentInfo in parentsInfo:
             # This will catch straight to merge files that do not have redneck
             # parents.  We will mark the straight to merge file from the job
             # as a child of the merged parent.
-            if int(parentInfo["merged"]) == 1:
+            if int(parentInfo["merged"]) == 1 and not parentInfo["lfn"].startswith("/store/unmerged/"):
                 newParents.add(parentInfo["lfn"])
 
             elif parentInfo['gpmerged'] is None:
@@ -393,7 +394,7 @@ class AccountantWorker(WMConnectionBase):
             # children.  We have to setup parentage and then check on whether or
             # not this file has any redneck children and update their parentage
             # information.
-            elif int(parentInfo["gpmerged"]) == 1:
+            elif int(parentInfo["gpmerged"]) == 1 and not parentInfo["gplfn"].startswith("/store/unmerged/"):
                 newParents.add(parentInfo["gplfn"])
 
             # If that didn't work, we've reached the great-grandparents
@@ -906,8 +907,7 @@ class AccountantWorker(WMConnectionBase):
 
         # Now all the parents should exist
         # Commit them to DBSBuffer
-        logging.info("About to commit all DBSBuffer Heritage information")
-        logging.info(len(bindList))
+        logging.info("About to commit DBSBuffer heritage information for: %d binds", len(bindList))
 
         if bindList:
             try:
diff --git a/src/python/WMCore/WMBS/MySQL/Jobs/LoadForErrorHandler.py b/src/python/WMCore/WMBS/MySQL/Jobs/LoadForErrorHandler.py
index 2129c0542..ee88125d9 100644
--- a/src/python/WMCore/WMBS/MySQL/Jobs/LoadForErrorHandler.py
+++ b/src/python/WMCore/WMBS/MySQL/Jobs/LoadForErrorHandler.py
@@ -38,7 +38,7 @@ class LoadForErrorHandler(DBFormatter):
     parentSQL = """SELECT parent.lfn AS lfn, wfp.child AS id
                      FROM wmbs_file_parent wfp
                      INNER JOIN wmbs_file_details parent ON parent.id = wfp.parent
-                     WHERE wfp.child = :fileid """
+                     WHERE wfp.child = :fileid AND parent.merged = 1"""
 
     runLumiSQL = """SELECT fileid, run, lumi, num_events FROM wmbs_file_runlumi_map
                      WHERE fileid = :fileid"""
@@ -52,6 +52,9 @@ class LoadForErrorHandler(DBFormatter):
         Fetch run/lumi/events information for each file and append Run objects
         to the files information.
         """
+        if not fileBinds:
+            return
+
         lumiResult = self.dbi.processData(self.runLumiSQL, fileBinds, conn=conn,
                                           transaction=transaction)
         lumiList = self.formatDict(lumiResult)
@@ -80,8 +83,7 @@ class LoadForErrorHandler(DBFormatter):
                 f['newRuns'].append(newRun)
         return
 
-    def execute(self, jobID, fileSelection=None,
-                conn=None, transaction=False):
+    def execute(self, jobID, fileSelection=None, conn=None, transaction=False):
         """
         _execute_
 
@@ -101,32 +103,32 @@ class LoadForErrorHandler(DBFormatter):
         result = self.dbi.processData(self.sql, binds, conn=conn,
                                       transaction=transaction)
         jobList = self.formatDict(result)
-        for entry in jobList:
-            entry.setdefault('input_files', [])
 
         filesResult = self.dbi.processData(self.fileSQL, binds, conn=conn,
                                            transaction=transaction)
         fileList = self.formatDict(filesResult)
 
-        noDuplicateFiles = {}
-        fileBinds = []
+        # special case for skipped files
         if fileSelection:
             fileList = [x for x in fileList if x['lfn'] in fileSelection[x['jobid']]]
+
+        noDuplicateFiles = {}
+        fileBinds = []
         for x in fileList:
             # Assemble unique list of binds
             if {'fileid': x['id']} not in fileBinds:
                 fileBinds.append({'fileid': x['id']})
                 noDuplicateFiles[x['id']] = x
 
+        # only upload to not duplicate files to prevent excessive memory
+        self.getRunLumis(fileBinds, noDuplicateFiles.values(), conn, transaction)
+
         parentList = []
-        if len(fileBinds) > 0:
+        if fileBinds:
             parentResult = self.dbi.processData(self.parentSQL, fileBinds, conn=conn,
                                                 transaction=transaction)
             parentList = self.formatDict(parentResult)
 
-            # only upload to not duplicate files to prevent excessive memory
-            self.getRunLumis(fileBinds, noDuplicateFiles.values(), conn, transaction)
-
         filesForJobs = {}
         for f in fileList:
             jobid = f['jobid']
@@ -145,12 +147,14 @@ class LoadForErrorHandler(DBFormatter):
                     if entry['id'] == f['id']:
                         wmbsFile['parents'].add(entry['lfn'])
                 wmbsFile.pop('pnn', None)  # not needed for anything, just remove it
+                wmbsFile.pop('newlocations', None)  # not needed for anything, just remove it
                 filesForJobs[jobid][f['id']] = wmbsFile
             elif f['pnn']:
                 # If the file is there and it has a location, just add it
                 filesForJobs[jobid][f['id']]['locations'].add(f['pnn'])
 
         for j in jobList:
+            j.setdefault('input_files', [])
             if j['id'] in filesForJobs.keys():
                 j['input_files'] = filesForJobs[j['id']].values()
 
diff --git a/src/python/WMCore/WMBS/Oracle/Jobs/LoadForErrorHandler.py b/src/python/WMCore/WMBS/Oracle/Jobs/LoadForErrorHandler.py
index d14f957bf..e1db3a0a8 100644
--- a/src/python/WMCore/WMBS/Oracle/Jobs/LoadForErrorHandler.py
+++ b/src/python/WMCore/WMBS/Oracle/Jobs/LoadForErrorHandler.py
@@ -19,3 +19,8 @@ class LoadForErrorHandler(MySQLLoadForErrorHandler):
                    LEFT OUTER JOIN wmbs_file_location wfl ON wfd.id = wfl.fileid
                    LEFT OUTER JOIN wmbs_pnns wpnn ON wpnn.id = wfl.pnn
                  WHERE wja.job = :jobid"""
+
+    parentSQL = """SELECT parent.lfn AS lfn, wfp.child AS id
+                     FROM wmbs_file_parent wfp
+                     INNER JOIN wmbs_file_details parent ON parent.id = wfp.parent
+                     WHERE wfp.child = :fileid AND parent.merged = '1'"""
diff --git a/src/python/WMCore/WorkQueue/WMBSHelper.py b/src/python/WMCore/WorkQueue/WMBSHelper.py
index b2cb2ec34..92d135974 100644
--- a/src/python/WMCore/WorkQueue/WMBSHelper.py
+++ b/src/python/WMCore/WorkQueue/WMBSHelper.py
@@ -669,22 +669,20 @@ class WMBSHelper(WMConnectionBase):
         adds the ACDC files into WMBS database
         """
         wmbsParents = []
-        # TODO:  this check can be removed when ErrorHandler filters parents file for unmerged data
-        if acdcFile["parents"]:
-            firstParent = next(iter(acdcFile["parents"]))
-            # If files is merged and has unmerged parents skip the wmbs population
-            if acdcFile.get("merged", 0) and ("/store/unmerged/" in firstParent or "MCFakeFile" in firstParent):
-                # don't set the parents
-                pass
-            else:
-                # set the parentage for all the unmerged parents
-                for parent in acdcFile["parents"]:
-                    logging.debug("WMBS ACDC Parent File: %s", parent)
-                    parent = self._addACDCFileToWMBSFile(DatastructFile(lfn=parent,
-                                                                        locations=acdcFile["locations"],
-                                                                        merged=True),
-                                                         inFileset=False)
-                    wmbsParents.append(parent)
+        # If file is merged, then it will be the parent of whatever output that
+        # process this job (it twists my mind!). Meaning, block below can be skipped
+        if int(acdcFile.get("merged", 0)) == 0 and acdcFile["parents"]:
+            # set the parentage for all the unmerged parents
+            for parent in acdcFile["parents"]:
+                if parent.startswith("/store/unmerged/") or parent.startswith("MCFakeFile"):
+                    logging.warning("WMBS ACDC skipped parent invalid file: %s", parent)
+                    continue
+                logging.debug("WMBS ACDC Parent File: %s", parent)
+                parent = self._addACDCFileToWMBSFile(DatastructFile(lfn=parent,
+                                                                    locations=acdcFile["locations"],
+                                                                    merged=True),
+                                                     inFileset=False)
+                wmbsParents.append(parent)
 
         # pass empty check sum since it won't be updated to dbs anyway
         checksums = {}