Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Consider PU location for top level MC task #9659

Merged
merged 2 commits into from
Apr 26, 2020
Merged

Conversation

amaltaro
Copy link
Contributor

@amaltaro amaltaro commented Apr 23, 2020

Fixes #9658

Status

ready

Description

When adding MCFakeFile files into wmbs_file_details table, consider the common locations of the WQE instead of setting every single PNN as a candidate for that file.

In other words, when fetching a WQE without any input dataset, but with pileup dataset, set the MCFakeFile location to the same location as of the pileup data, such that we ensure that jobs will only be submitted to sites hosting some of the pileup data (be it a top level task or the subsequent ones).

NOTE: we should probably make the same changes to top level tasks *with input data (or ACDC data). I'm not making those changes right now because I believe we would have negative side effects. E.g., misassignment generating an empty file location; e.g., ACDC collections with much less locations than it should... and anything else that still didn't come to my mind.

Is it backward compatible (if not, which system it affects?)

yes

Related PRs

none

External dependencies / deployment changes

none

@cmsdmwmbot
Copy link

Jenkins results:

  • Unit tests: failed
    • 8 new failures
    • 3 tests no longer failing
    • 1 changes in unstable tests
  • Pylint check: failed
    • 1 warnings and errors that must be fixed
    • 10 warnings
    • 70 comments to review
  • Pycodestyle check: succeeded
    • 5 comments to review
  • Python3 compatibility checks: succeeded

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/9943/artifact/artifacts/PullRequestReport.html

@cmsdmwmbot
Copy link

Jenkins results:

  • Unit tests: succeeded
    • 3 tests no longer failing
  • Pylint check: failed
    • 21 warnings and errors that must be fixed
    • 10 warnings
    • 118 comments to review
  • Pycodestyle check: succeeded
    • 44 comments to review
  • Python3 compatibility checks: succeeded

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/9946/artifact/artifacts/PullRequestReport.html

@amaltaro
Copy link
Contributor Author

squashing

_wmbsPreparation needs to receive a list of PNNs, not PSNs
@cmsdmwmbot
Copy link

Jenkins results:

  • Unit tests: succeeded
  • Pylint check: failed
    • 21 warnings and errors that must be fixed
    • 10 warnings
    • 118 comments to review
  • Pycodestyle check: succeeded
    • 44 comments to review
  • Python3 compatibility checks: succeeded

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/9951/artifact/artifacts/PullRequestReport.html

Copy link
Contributor

@todor-ivanov todor-ivanov left a comment

Choose a reason for hiding this comment

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

I had Only one comment inline. The rest looks good.

locations.add(siteInfo[0]['pnn'])
except Exception as ex:
self.logger.error('Error getting storage element for "%s": %s' % (site, str(ex)))
siteInfo = self.getLocationInfo.execute(conn=self.getDBConn(),
Copy link
Contributor

Choose a reason for hiding this comment

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

@amaltaro I have only one question. Why did you take the siteInfo query out of the 'try: - except:' block?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should be caught upstream, without breaking the whole cycle, but only this one element.
I also removed it, because as you can see in the old line 352, it wasn't covered by any try/except block as well.

@amaltaro
Copy link
Contributor Author

Thank you for your comments, Todor.

I still think input data and pileup data location should not be correlated in WMBS, only for building the DESIRED_Sites lists. Otherwise we have an impact in the ACDC documents, where data location would actually be their intersection with the pileup location.
Anyways, that would be a big change. I just wanted to register it here.

@amaltaro amaltaro merged commit ad99c05 into dmwm:master Apr 26, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Top level task must consider pileup location of subsequent tasks
3 participants