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

Fix for the reconnect-each-run policy in CondDBESSource #12386

Merged

Conversation

ggovi
Copy link
Contributor

@ggovi ggovi commented Nov 11, 2015

The fix consists in reverting back the previous fix in PayloadProxy::reload

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @ggovi for CMSSW_7_5_X.

It involves the following packages:

CondCore/CondDB

@ggovi, @cmsbuild, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @apfeiffer1 this is something you requested to watch as well.

Following commands in first line of a comment are recognized

  • +1|approve[d]|sign[ed]: L1/L2's to approve it
  • -1|reject[ed]: L1/L2's to reject it
  • assign <category>[,<category>[,...]]: L1/L2's to request signatures from other categories
  • unassign <category>[,<category>[,...]]: L1/L2's to remove signatures from other categories
  • hold: L1/L2's to mark it as on hold
  • merge: L1 to merge this request
  • [@cmsbuild,] please test: L1/L2 and selected users to start jenkins tests
  • [@cmsbuild,] please test with cms-sw/cmsdist#<PR>: L1/L2 and selected users to start jenkins tests using externals from cmsdist

@Degano you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@ggovi
Copy link
Contributor Author

ggovi commented Nov 11, 2015

@cmsbuild please test

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/9658/console

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

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

The workflows 140.53 have different files in step1_dasquery.log than the ones found in the baseline. You may want to check and retrigger the tests if necessary. You can check it in the "files" directory in the results of the comparisons

@diguida
Copy link
Contributor

diguida commented Nov 12, 2015

@mmusich @franzoni to watch

@mmusich
Copy link
Contributor

mmusich commented Nov 12, 2015

@diguida @ggovi thanks. Are 7.6.X and 8.0.X safe?

@diguida
Copy link
Contributor

diguida commented Nov 12, 2015

@mmusich seems so...

@mmusich
Copy link
Contributor

mmusich commented Nov 12, 2015

am I misreading? https://github.com/cms-sw/cmssw/blob/CMSSW_8_0_X/CondCore/CondDB/src/PayloadProxy.cc#L34 (looks like it has the same bug as 75X). Or is it handled in a deeper way?

@diguida
Copy link
Contributor

diguida commented Nov 12, 2015

@mmusich seems so = seems it is buggy!

@diguida
Copy link
Contributor

diguida commented Nov 12, 2015

PR coming...

@ggovi
Copy link
Contributor Author

ggovi commented Nov 12, 2015

+1

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_5_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_7_6_X is complete. This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @Degano, @smuzaffar

davidlange6 added a commit that referenced this pull request Nov 12, 2015
Fix for the reconnect-each-run policy in CondDBESSource
@davidlange6 davidlange6 merged commit 457e3f0 into cms-sw:CMSSW_7_5_X Nov 12, 2015
@davidlange6
Copy link
Contributor

Could someone summarize what changed since 74x to set off this "bug"?

@ggovi
Copy link
Contributor Author

ggovi commented Nov 12, 2015

Hi David,
the package CondCore/CondDB (and the concerned function in particular) in 75X and 76X diverged from 74X for the introduction of the IOV sequence snapshot.
Here is the PR:
#9124
This is also contain the 'bad' fix which we are reverting here...

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