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

Revert "Move traceback routine into a separate thread." #11270

Merged
merged 1 commit into from Sep 16, 2015

Conversation

smuzaffar
Copy link
Contributor

Reverts #11239

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @smuzaffar (Malik Shahzad Muzaffar) for CMSSW_7_6_X.

Revert "Move traceback routine into a separate thread."

It involves the following packages:

FWCore/Services

@cmsbuild, @smuzaffar, @Dr15Jones can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @wddgit, @wmtan this is something you requested to watch as well.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
If you are a L2 or a release manager you can ask for tests by saying 'please test' in the first line of a comment.
@Degano you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

smuzaffar added a commit that referenced this pull request Sep 16, 2015
…acktrace

Revert "Move traceback routine into a separate thread."
@smuzaffar smuzaffar merged commit faf88ce into CMSSW_7_6_X Sep 16, 2015
@Dr15Jones
Copy link
Contributor

@bbockelm

@Dr15Jones
Copy link
Contributor

@smuzaffar @davidlange6 @bbockelm Looking at the failures in CMSSW_7_6_X_2015-09-15-1100 I'm not convinced they were caused by this pull request. It looks to me like the failure was a binary incompatibliity because not every package dependent upon #11189 was recompiled. The reason I think this is the crashes I've looked at all happend in the destructor of edm::AssociationMap. Also, the problems appear to go away in the following IB which may have had more code recompiled.

@Dr15Jones
Copy link
Contributor

On the positive side for #11239 the IB RelVal logs for CMSSW_7_6_THREADED_X_2015-09-15-1100 for the crashed jobs do show the crashing thread this time. So in principal the change actually worked!

@bbockelm
Copy link
Contributor

(FWIW - I ran out of time last night, but hope to fixup the broken unit test and redo the PR by tomorrow-ish)

@Dr15Jones
Copy link
Contributor

Looking at the IB page
https://cmssdt.cern.ch/SDT/cgi-bin/showBuildLogs.py/slc6_amd64_gcc493/www/tue/7.6-tue-11/CMSSW_7_6_X_2015-09-15-1100

I can't pull up the full build log, however the library which has the code which is crashing is DataFormats/TrackReco and from what I can tell from this page, it appears that library was not recompiled. This would definitely cause a binary incompatibility.

@Dr15Jones
Copy link
Contributor

To further back my claim, the following IB, CMSSW_7_6_X_2015-09-15-2300, didn't have the crashing problems and it did recompile DataFormats/TrackReco

https://cmssdt.cern.ch/SDT/cgi-bin/buildlogs/slc6_amd64_gcc493/CMSSW_7_6_X_2015-09-15-2300/DataFormats/TrackReco

@smuzaffar
Copy link
Contributor Author

couple of strange things here

  • first it should have compiled DataFormats/TrackReco in CMSSW_7_6_X_2015-09-15-1100 as your PR Made AssociationMap thread-safe #11189 is part of this patch release. You Pr has explicitly modified DataFormats/TrackReco
  • unit tests are still failing in CMSSW_7_6_X_2015-09-15-2300 which is a full release.

@smuzaffar
Copy link
Contributor Author

@Dr15Jones , looks like our git-cms-sparse-checkout script ignore the empty lines. That is why your change in DataFormats/TrackReco/BuildFile.xml was not consider important and the package was not part of the patch release

@Dr15Jones
Copy link
Contributor

@smuzaffar I fully agree that the unit tests failures are from #11239 which is from the bad interaction between forking and threads. Brian says he is going to fix that case.

@smuzaffar smuzaffar deleted the revert-11239-separate_thread_stacktrace branch October 19, 2016 07:37
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

4 participants