Fix exception handling in Worker::prePrefetchSelectionAsync()

[makortel]

PR description:

This PR fixes a bug introduced in #44767 where a return when the worker is not selected was moved inside a lambda passed to convertException(). This PR fixes the function to return at the same spot as before if the worker is not selected. (this part fixes the ASAN failures in reported in #44888)

Other problem was that if implDoPrePrefetchSelection() throws an exception, the exception status needs to stored in the Worker::status_, and the success task must be destroyed. (this part changes the assertion failures reported in #44887 to exceptions)

Thanks to @Dr15Jones for helping to figure out the cause.

Fixes #44888
Fixes #44887
Resolves cms-sw/framework-team#912

PR validation:

Workflow 4.17 succeeded in CMSSW_14_1_ASAN_X_2024-05-01-2300. Workflow 1001.3 properly reports the exceptions.

[cmsbuild]

cms-bot internal usage

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44891/40153

• This PR adds an extra 20KB to repository

• There are other open Pull requests which might conflict with changes you have proposed:

  • File FWCore/Framework/src/Worker.cc modified in PR(s): Revert "Throw a clear exception if OutputModule SelectEvents refers to a non-existent Path or Process" #44889

[cmsbuild]

A new Pull Request was created by @makortel for master.

It involves the following packages:

• FWCore/Framework (core)

@smuzaffar, @Dr15Jones, @cmsbuild, @makortel can you please review it and eventually sign? Thanks.
@missirol, @wddgit this is something you requested to watch as well.
@rappoccio, @antoniovilela, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

[makortel]

test parameters:

• enable_tests = threading
• release = CMSSW_14_1_ASAN_X
• workflows = 4.17, 1001.3, 1041.0

[makortel]

@cmsbuild, please test

[smuzaffar]

@makortel , if you want 4.17, 1001.3, 1041.0 workflows in threaded mode then please update test parameters to also have workflow_threading = 4.17, 1001.3, 1041.0

[smuzaffar]

also it is better to use please test for CMSSW_14_1_ASAN_X instead of using release = CMSSW_14_1_ASAN_X as test parameters. Using explicit please test will allow you to start default and ASAN tests in parallel :-)

[makortel]

@cmsbuild, please abort

[makortel]

test parameters:

• enable_tests = threading
• workflows_threading = 1001.3, 1041.0

[makortel]

@cmsbuild, please test

[makortel]

@cmsbuild, please test for CMSSW_14_1_ASAN_X

[makortel]

Thanks @smuzaffar. For these parallel ASAN tests is there no way to specify additional workflows? (in this case it probably won't matter, the ASAN failures were quite widespread)

[smuzaffar]

Thanks @smuzaffar. For these parallel ASAN tests is there no way to specify additional workflows? (in this case it probably won't matter, the ASAN failures were quite widespread)

@makortel , no not for threaded mode. If you say please test workflows 1001.3, 1041.0 for CMSSW_14_1_ASAN_X then bot will run these extra workflows for only ASAN but not in threaded mode

[Dr15Jones]

looks good

[makortel]

• workflows_threading = 1001.3,1040.1,1041.0,1042.0

Note that with this PR these workflows are still expected to fail. But now they should report the exception properly (that will be followed up separately) instead of dying in assertion failure.

[cmsbuild]

-1

Failed Tests: RelVals-THREADING
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-df182c/39221/summary.html
COMMIT: c90b071
CMSSW: CMSSW_14_1_ASAN_X_2024-05-01-2300/el8_amd64_gcc12
Additional Tests: THREADING
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/44891/39221/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-THREADING

• 1041.01041.0_RunExpressPhy2017F/step3_RunExpressPhy2017F.log
• 1040.11040.1_RunExpressPhy2017F/step3_RunExpressPhy2017F.log
• 1042.01042.0_RunExpressPhy2017F/step3_RunExpressPhy2017F.log

Expand to see more relval errors ...

• 1001.3

[Dr15Jones]

Note: for a future PR we should modify the exception message to include the module label.

[Dr15Jones]

All the failing workflows are expected to fail as this PR is meant to find these problems (which were previously hidden)

[makortel]

The 4 workflows report now exceptions properly

[makortel]

+core

[cmsbuild]

This pull request is fully signed and it will be integrated in one of the next master IBs (but tests are reportedly failing). This pull request will now be reviewed by the release team before it's merged. @rappoccio, @sextonkennedy, @antoniovilela (and backports should be raised in the release meeting by the corresponding L2)

[makortel]

test parameters:

[makortel]

@cmsbuild, please test

Maybe one more round to have "clean tests"

[cmsbuild]

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-df182c/39226/summary.html
COMMIT: c90b071
CMSSW: CMSSW_14_1_X_2024-05-02-2300/el8_amd64_gcc12
Additional Tests: THREADING
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/44891/39226/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

• You potentially removed 187 lines from the logs
• Reco comparison results: 7 differences found in the comparisons
• DQMHistoTests: Total files compared: 48
• DQMHistoTests: Total histograms compared: 3331548
• DQMHistoTests: Total failures: 6
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3331522
• DQMHistoTests: Total skipped: 20
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
• Checked 202 log files, 165 edm output root files, 48 DQM output files
• TriggerResults: no differences found

[antoniovilela]

+1