-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
[12_5_X] Release dependent modules only after the worker has finished for scheduled modules #39485
[12_5_X] Release dependent modules only after the worker has finished for scheduled modules #39485
Conversation
This change was triggered by a case where a PuttableProductResolver was filled by a Worker that produced many products, and one of the products (A) had a Ref to another product (B), and the product B was not consumed by any module (it was only accessed through the Ref). Since the putProduct() released the WaitingTaskList of the Resolver, that lead to the consumer of A to run, and that consumer dereferenced the Ref to see that B was not there. Besides scheduled modules another cases where PuttableProductResolver is used are Sources inheriting PuttableSourceBase, and TestProcessor. In these cases the products are put into the Resolvers (or left as non-produced) before launching the prefetching of the unscheduled system. Therefore in these use cases the consuming modules do not need to wait for the Resolver to be filled. After several fix attempts it seemed easiest to just use the Worker's WaitingTaskList directly in PuttableProductResolver. This approach fulfills the requirements of both Worker and Source(-like) use cases, and even simplifies the code.
A new Pull Request was created by @makortel (Matti Kortelainen) for CMSSW_12_5_X. It involves the following packages:
@cmsbuild, @smuzaffar, @Dr15Jones, @makortel can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
enable threading |
@cmsbuild, please test |
-1 Failed Tests: UnitTests Unit TestsI found errors in the following unit tests: ---> test EcalTPG_updateWeightIdMap_test had ERRORS ---> test TestIOPoolInputNoParentDictionary had ERRORS Comparison SummarySummary:
|
The The |
|
@cmsbuild, please test |
-1 Failed Tests: UnitTests Unit TestsI found errors in the following unit tests: ---> test EcalTPG_updateWeightGroup_test had ERRORS ---> test EcalTPG_updateWeightIdMap_test had ERRORS Comparison SummarySummary:
|
Apparently #39235 that fixed the |
+1 |
This pull request is fully signed and it will be integrated in one of the next CMSSW_12_5_X IBs (but tests are reportedly failing) and once validation in the development release cycle CMSSW_12_6_X is complete. This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2) |
backport of #39245 |
please test with #39503 |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-33bec1/27779/summary.html Comparison SummarySummary:
|
+1 |
PR description:
Backport of #39245. Fixes a rare scheduling bug that was observed at the HLT in #39064 (that was first mitigated in #39201).
PR validation:
Tests in #39245 and #39484.