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.

Sign up for GitHub

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

Jump to bottom

[14_0_X] Remove tensorflow::setLogging() as thread-unsafe #46107

Merged
merged 1 commit into from
Sep 28, 2024
Merged

[14_0_X] Remove tensorflow::setLogging() as thread-unsafe #46107

merged 1 commit into from
Sep 28, 2024

Conversation

makortel
Copy link
Contributor

PR description:

Backport of #46065

PR validation:

None beyond #46065

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

Backport of #46065 (following request in #46065 (comment))

@makortel
Remove tensorflow::setLogging() as thread-unsafe
b6598d0 
The setLogging() calls setenv(), which is not required to be thread
safe, and specifically in glibc leads to a race condition with any
concurrent getenv() calls.
@cmsbuild cmsbuild added this to the CMSSW_14_0_X milestone Sep 23, 2024
@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 23, 2024 •
edited
Loading

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

It involves the following packages:

  • DQM/DTMonitorClient (dqm)
  • L1Trigger/L1THGCal (upgrade, l1)
  • L1Trigger/Phase2L1ParticleFlow (upgrade, l1)
  • PhysicsTools/TensorFlow (ml)
  • RecoEcal/EgammaCoreTools (reconstruction)
  • RecoMuon/TrackerSeedGenerator (reconstruction)
  • RecoTauTag/HLTProducers (hlt)

@Martin-Grunewald, @Moanwar, @aloeliger, @antoniovagnerini, @cmsbuild, @epalencia, @jfernan2, @mandrenguyen, @mmusich, @nothingface0, @rvenditti, @srimanob, @subirsarkar, @syuvivida, @tjavaid, @valsdav, @y19y19 can you please review it and eventually sign? Thanks.
@CeliaFernandez, @Fedespring, @HuguesBrun, @Martin-Grunewald, @Prasant1993, @ReyerBand, @Sam-Harper, @a-kapoor, @abbiendi, @afiqaize, @amarini, @andrea21z, @argiro, @azotz, @battibass, @bellan, @cericeci, @jainshilpi, @jbsauvan, @jhgoh, @lgray, @mbluj, @missirol, @mmusich, @ram1123, @rchatter, @riga, @rociovilar, @sameasy, @silviodonato, @sobhatta, @thomreis, @trocino, @valsdav, @varuns23, @wang0jin this is something you requested to watch as well.
@antoniovilela, @mandrenguyen, @rappoccio, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 23, 2024 •
edited
Loading

cms-bot internal usage

@makortel makortel mentioned this pull request Sep 23, 2024
Merged
@makortel
Copy link
Contributor Author

@cmsbuild, please test with cms-sw/cmsdist#9432

This was referenced Sep 23, 2024
Merged
Closed
@cmsbuild
Copy link
Contributor

+1

Size: This PR adds an extra 40KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4f4a0d/41717/summary.html
COMMIT: b6598d0
CMSSW: CMSSW_14_0_X_2024-09-23-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/46107/41717/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

@jfernan2
Copy link
Contributor

+1

@mmusich
Copy link
Contributor

mmusich commented Sep 24, 2024

+hlt

@valsdav
Copy link
Contributor

valsdav commented Sep 24, 2024

+ml

@Moanwar
Copy link
Contributor

Moanwar commented Sep 24, 2024

+Upgrade

@aloeliger
Copy link
Contributor

+l1

@antoniovagnerini
Copy link

+1

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_14_0_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_14_2_X is complete. This pull request will now be reviewed by the release team before it's merged. @mandrenguyen, @antoniovilela, @sextonkennedy, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)
Notice This PR was tested with additional Pull Request(s), please also merge them if necessary: cms-sw/cmsdist#9432

@mandrenguyen
Copy link
Contributor

backport of #46065

@mandrenguyen
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 5863311 into cms-sw:CMSSW_14_0_X Sep 28, 2024
9 checks passed
@makortel makortel deleted the tensorflowRemoveLogging_140x branch September 30, 2024 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
backport-ok dqm-approved fully-signed hlt-approved l1-approved ml-approved orp-approved reconstruction-approved requires-external tests-approved upgrade-approved
Projects
None yet
Milestone
CMSSW_14_0_X
Development

Successfully merging this pull request may close these issues.
9 participants
@makortel @cmsbuild @jfernan2 @mmusich @valsdav @Moanwar @aloeliger @antoniovagnerini @mandrenguyen