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

make HLTFilters global modules - part 2 #1303

Merged
merged 30 commits into from Nov 11, 2013

Conversation

fwyzard
Copy link
Contributor

@fwyzard fwyzard commented Nov 3, 2013

convert HLTFilters into global::EDFilters (mostly) or stream::EDFilters

  • modify HLTFilter to inherit from global::EDFilter, without chaning the interface seen from children modules, except for hltFilter(...) becoming const
  • introduce HLTStreamFilter inheriting from stream::EDFilter, with the same interface as HLTFilter except that hltFilter(...) is NOT const
  • adapt most HLTFilter modules to the new interface
  • convert HLTLevel1GTSeed into an HLTStreamFilter, as it needs a per-stream internal state

this includes #1235, #1240, #1295

fwyzard and others added 29 commits October 31, 2013 07:48
 - migrate to getByToken and consumes<>
 - make configuration variables 'const'
need for in-progress work to convert all hltFilters to global modules
When ModuleDescription access was added to the other module types, the stream types were accidentally left out. This makes them uniform with all the other types.
…r HLT filters that need a per-stream internal state
@fwyzard
Copy link
Contributor Author

fwyzard commented Nov 3, 2013

+1 : it passes the "small" matrix:

$ runTheMatrix.py -j8 -l limited -i all
[...]
$ cat runall-report-step123-.log
[...]
7 7 6 6 4 tests passed, 0 0 0 0 0 failed

@emeschi
Copy link
Contributor

emeschi commented Nov 3, 2013

+1

can I send a "+Inf" so that I don't get asked any longer ;)


Emilio Meschi
Senior Physicist
CERN PH/CMD
LHC Programme Coordinator

On Nov 3, 2013, at 6:58 PM, cmsbuild notifications@github.com wrote:

A new Pull Request was created by @fwyzard (Andrea Bocci) for CMSSW_7_0_X.

make HLTFilters global modules - part 2

It involves the following packages:

EventFilter/Cosmics
CaloOnlineTools/EcalTools
HLTrigger/HLTfilters
DQM/DTMonitorModule
RecoTracker/DeDx
HLTrigger/special
RecoTauTag/HLTProducers
FWCore/Framework
HLTrigger/Muon
HLTrigger/Egamma
HLTrigger/HLTcore
HLTrigger/JetMET
DPGAnalysis/Skims
HLTrigger/btau

@perrotta, @Dr15Jones, @vlimant, @demattia, @danduggan, @fwyzard, @emeschi, @rovere, @ktf, @Martin-Grunewald, @franzoni, @thspeer, @rcastello, @deguio, @slava77, @mommsen, @eliasron, @fabiocos, @nclopezo can you please review it and eventually sign? Thanks.
@ghellwig, @wmtan, @GiacomoSguazzoni, @rovere, @gpetruc, @cerati 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.
@ktf you are the release manager for this.


Reply to this email directly or view it on GitHub.

@ktf
Copy link
Contributor

ktf commented Nov 4, 2013

@emeschi, it would actually make sense for the case in which you do not care about the updates of a given pull request…

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 4, 2013

-1
When I ran the RelVals I found an error in the following worklfows:
1306.0 step1

runTheMatrix-results/1306.0_SingleMuPt1_UP15+SingleMuPt1_UP15+DIGIUP15+RECOUP15+HARVESTUP15/step1_SingleMuPt1_UP15+SingleMuPt1_UP15+DIGIUP15+RECOUP15+HARVESTUP15.log
----- Begin Fatal Exception 04-Nov-2013 09:50:29 CET-----------------------
An exception of category 'StdException' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing ESSource: class=PoolDBESSource label='GlobalTag'
Exception Message:
A std::exception was thrown.
Can not get data (Additional Information: [frontier.c:1034]: No more proxies. Last error was: Request 60 on chan 2 failed at Mon Nov  4 09:50:29 2013: -8 [payload.c:105]: Server signalled payload error 1: FrontierPrep java.sql.SQLException: Listener refused the connection with the following error: ORA-12514, TNS:listener does not currently know of service requested in connect descriptor at oracle.jdbc.driver.T4CConnection.logon(T4CConnection.java:512)) ( CORAL : "coral::FrontierAccess::Statement::execute" from "CORAL/RelationalPlugins/frontier" )
----- End Fatal Exception -------------------------------------------------

4.53 step2

runTheMatrix-results/4.53_RunPhoton2012B+RunPhoton2012B+HLTD+RECODreHLT+HARVESTDreHLT/step2_RunPhoton2012B+RunPhoton2012B+HLTD+RECODreHLT+HARVESTDreHLT.log
----- Begin Fatal Exception 04-Nov-2013 09:50:30 CET-----------------------
An exception of category 'StdException' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing ESSource: class=PoolDBESSource label='GlobalTag'
Exception Message:
A std::exception was thrown.
Can not get data (Additional Information: [frontier.c:1034]: No more proxies. Last error was: Request 60 on chan 2 failed at Mon Nov  4 09:50:30 2013: -8 [payload.c:105]: Server signalled payload error 1: FrontierPrep java.sql.SQLException: Listener refused the connection with the following error: ORA-12514, TNS:listener does not currently know of service requested in connect descriptor at oracle.jdbc.driver.T4CConnection.logon(T4CConnection.java:512)) ( CORAL : "coral::FrontierAccess::Statement::execute" from "CORAL/RelationalPlugins/frontier" )
----- End Fatal Exception -------------------------------------------------

4.22 step2

runTheMatrix-results/4.22_RunCosmics2011A+RunCosmics2011A+RECOCOSD+ALCACOSD+SKIMCOSD+HARVESTDC/step2_RunCosmics2011A+RunCosmics2011A+RECOCOSD+ALCACOSD+SKIMCOSD+HARVESTDC.log
----- Begin Fatal Exception 04-Nov-2013 09:50:47 CET-----------------------
An exception of category 'StdException' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing ESSource: class=PoolDBESSource label='GlobalTag'
Exception Message:
A std::exception was thrown.
Can not get data (Additional Information: [frontier.c:1034]: No more proxies. Last error was: Request 60 on chan 2 failed at Mon Nov  4 09:50:47 2013: -8 [payload.c:105]: Server signalled payload error 1: FrontierPrep java.sql.SQLException: Listener refused the connection with the following error: ORA-12514, TNS:listener does not currently know of service requested in connect descriptor at oracle.jdbc.driver.T4CConnection.logon(T4CConnection.java:512)) ( CORAL : "coral::FrontierAccess::Statement::execute" from "CORAL/RelationalPlugins/frontier" )
----- End Fatal Exception -------------------------------------------------

1001.0 step2

runTheMatrix-results/1001.0_RunMinBias2011A+RunMinBias2011A+TIER0EXP+ALCAEXP+ALCAHARVD/step2_RunMinBias2011A+RunMinBias2011A+TIER0EXP+ALCAEXP+ALCAHARVD.log
----- Begin Fatal Exception 04-Nov-2013 09:50:56 CET-----------------------
An exception of category 'StdException' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing ESSource: class=PoolDBESSource label='GlobalTag'
Exception Message:
A std::exception was thrown.
Can not get data (Additional Information: [frontier.c:1034]: No more proxies. Last error was: Request 60 on chan 2 failed at Mon Nov  4 09:50:56 2013: -8 [payload.c:105]: Server signalled payload error 1: FrontierPrep java.sql.SQLException: Listener refused the connection with the following error: ORA-12514, TNS:listener does not currently know of service requested in connect descriptor at oracle.jdbc.driver.T4CConnection.logon(T4CConnection.java:512)) ( CORAL : "coral::FrontierAccess::Statement::execute" from "CORAL/RelationalPlugins/frontier" )
----- End Fatal Exception -------------------------------------------------

1000.0 step2

runTheMatrix-results/1000.0_RunMinBias2011A+RunMinBias2011A+TIER0+SKIMD+HARVESTDfst2+ALCASPLIT/step2_RunMinBias2011A+RunMinBias2011A+TIER0+SKIMD+HARVESTDfst2+ALCASPLIT.log
----- Begin Fatal Exception 04-Nov-2013 09:50:59 CET-----------------------
An exception of category 'StdException' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing ESSource: class=PoolDBESSource label='GlobalTag'
Exception Message:
A std::exception was thrown.
Can not get data (Additional Information: [frontier.c:1034]: No more proxies. Last error was: Request 60 on chan 2 failed at Mon Nov  4 09:50:59 2013: -8 [payload.c:105]: Server signalled payload error 1: FrontierPrep java.sql.SQLException: Listener refused the connection with the following error: ORA-12514, TNS:listener does not currently know of service requested in connect descriptor at oracle.jdbc.driver.T4CConnection.logon(T4CConnection.java:512)) ( CORAL : "coral::FrontierAccess::Statement::execute" from "CORAL/RelationalPlugins/frontier" )
----- End Fatal Exception -------------------------------------------------

you can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/1142/summary.html

@fwyzard
Copy link
Contributor Author

fwyzard commented Nov 4, 2013

Dear @cmsbuild ,
can you please stop rejecting all my pull requests for completely unrelated issues ?

If the build system is unable to read its input files or conditions, it is not my fault.

.A

@nclopezo
Copy link
Contributor

nclopezo commented Nov 4, 2013

Hi @fwyzard,

Sorry to spam you with these messages, that message is generated automatically and @cmsbuild just reports what it sees in the logs. Sometimes when the IB is broken or another unrelated error appears this happens. it is also not @cmsbuild fault. I can make Jenkins to generate the message but not post it to avoid this.

@fwyzard
Copy link
Contributor Author

fwyzard commented Nov 4, 2013

Hi David,
the message per se is certainly not a problem - and sometimes the error in accessing the conditions or input files can be due to the changes in the pull request itself.
The problem (or at least this is my impression) is that once Jenkins reports the build as failed, it will not try again (say, the following day), so the pull request never gets the '+1' from the build system.

.A

@nclopezo
Copy link
Contributor

nclopezo commented Nov 4, 2013

Hi Andrea,

I can discuss that with Giulio, we could make that when a pull request fails, and then it is updated, the tests are triggered again automatically. But if the error is unrelated to the pull request, the tests could by triggered again in this case by a human.

@ktf
Copy link
Contributor

ktf commented Nov 4, 2013

Andrea, tests are still run by hand (and I'd like to keep it that way to avoid malicious code being executed unscrutinised, at least for the moment).

David, I think yours is probably a good idea in the long term, but again I'd rather avoid having failing tests being run all over. In this particular case, if the system autonomously decided to run again, it would fail again simply because the problem is infrastructural.

I suggest we do the following:

  • We set up a job which continuously runs runTheMatrix.py -s
  • We only run tests (or report about their failure) if the above mentioned job is successful.

this could be ran on a virtual machine to avoid overloading the build cluster for nothing.

@emeschi
Copy link
Contributor

emeschi commented Nov 4, 2013

Right, that's exactly what I meant.
As I mentioned before, the one package for which I get notifications does not really belong to daq.
But it's ok. I don't mind sending you guys a +1 every now and then ;)
e.


Emilio Meschi
Senior Physicist
CERN PH/CMD
LHC Programme Coordinator

On Nov 4, 2013, at 9:05 AM, Giulio Eulisse notifications@github.com wrote:

@emeschi, it would actually make sense for the case in which you do not care about the updates of a given pull request…


Reply to this email directly or view it on GitHub.

@ktf
Copy link
Contributor

ktf commented Nov 4, 2013

Added to the todo list...

@nclopezo
Copy link
Contributor

nclopezo commented Nov 4, 2013

@Martin-Grunewald
Copy link
Contributor

+1

@fwyzard
Copy link
Contributor Author

fwyzard commented Nov 4, 2013

sorry, I accidentally introduced a bug in HLTDTActivityFilter, fixed in the last push :-(

@fwyzard
Copy link
Contributor Author

fwyzard commented Nov 4, 2013

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 4, 2013

Pull request #1303 was updated. @Dr15Jones, @vlimant, @demattia, @danduggan, @emeschi, @rovere, @ktf, @franzoni, @thspeer, @rcastello, @deguio, @slava77, @mommsen, @eliasron, @fabiocos, @nclopezo can you please check and sign again.

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 5, 2013

@deguio
Copy link
Contributor

deguio commented Nov 6, 2013

+1

2 similar comments
@rcastello
Copy link

+1

@ktf
Copy link
Contributor

ktf commented Nov 6, 2013

+1

@@ -51,6 +51,10 @@
static void prevalidate(ConfigurationDescriptions& descriptions);
static const std::string& baseType();

// Warning: the returned moduleDescription will be invalid during construction
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't look like a very effective coding to repeat the same stuff for EDProducer, EDAnalyzer, and EDFilter.
@Dr15Jones
Chris, is there a reason there's no base class with all these methods?

@slava77
Copy link
Contributor

slava77 commented Nov 7, 2013

@slava77 working on it

@slava77
Copy link
Contributor

slava77 commented Nov 8, 2013

+1

tested in CMSSW_7_0_X_2013-11-05-1400 (sign270) in a combination of

no differences as expected

@Martin-Grunewald
Copy link
Contributor

Hi,

Can this be integrated asap (ie, missing signatures added)? I am asking because I need to make further
HLT-only changes and do not want to force again all affected to sign an updated PR if all I need to change
on top of 1303 is within HLT.

ktf added a commit that referenced this pull request Nov 11, 2013
…part2

Multithreading fixes -- Make HLTFilters global modules - part 2
@ktf ktf merged commit a0a47f1 into cms-sw:CMSSW_7_0_X Nov 11, 2013
@ktf
Copy link
Contributor

ktf commented Nov 11, 2013

I've bypassed the operations signature. @fabiocos @vlimant @franzoni complain if you did not want that.

@fwyzard fwyzard deleted the make_HLTFilters_global_modules_part2 branch November 12, 2013 15:14
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

10 participants