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 "Merge pull request #38405 from JuliaLang/vc/distributed_ts" #41722

Merged
merged 1 commit into from
Jul 29, 2021

Conversation

vtjnash
Copy link
Member

@vtjnash vtjnash commented Jul 28, 2021

Also revert "fixup to pull request #38405 (#41641)"

Seems to be causing hanging in CI testing? @vchuravy

This reverts commit 5af1cf0.
This reverts commit 5a16805, reversing
changes made to 02807b2.

Revert "fixup to pull request #38405 (#41641)"

Seems to be causing hanging in CI testing?

This reverts commit 5af1cf0.
This reverts commit 5a16805, reversing
changes made to 02807b2.
@DilumAluthge DilumAluthge requested a review from vchuravy July 28, 2021 18:28
@JeffBezanson
Copy link
Member

Do you have a link to an example failure?

@jpsamaroo
Copy link
Member

As an alternative to reverting this kind of PR for the fourth? time, can we try to just disable the failing test, if it's the one introduced by #38405? I think reverting it is equivalent to saying that #38405 is introducing a bug, but realistically it's probably just fixing one bug, and accidentally exposing another which already exists.

@vchuravy
Copy link
Member

vchuravy commented Jul 29, 2021

@JeffBezanson I think https://build.julialang.org/#/builders/69/builds/1798/steps/5/logs/stdio is the kind of issue that Jameson is concerned around. We have been observing a general increase in flakiness in the tests, either hanging or otherwise misbehaving often around Distributed.

Then there is also https://build.julialang.org/#/builders/9/builds/1640 and another example https://build.julialang.org/#/builders/78/builds/1578

can we try to just disable the failing test,

If it were just a failing test...

@DilumAluthge
Copy link
Member

Did the rr trace yield anything useful?

@vtjnash
Copy link
Member Author

vtjnash commented Jul 29, 2021

Looking closer, the last log Valentin attached clearly shows this PR needs to be reverted and worked on more, as it is failing due the any_gc_flag lock in the new code on line 260:

  5 const any_gc_flag = Threads.Condition()
  4 function start_gc_msgs_task()
  3     errormonitor(
  2         Threads.@spawn begin
  1             while true
260                 lock(any_gc_flag) do
  1                     wait(any_gc_flag)
  2                     flush_gc_msgs() # handles throws internally
  3                 end
  4             end
  5         end
  6     )
  7 end

@vtjnash vtjnash merged commit 66f9b55 into master Jul 29, 2021
@vtjnash vtjnash deleted the jn/revert-38405 branch July 29, 2021 16:07
KristofferC pushed a commit that referenced this pull request Aug 2, 2021
…41722)

Also reverts "fixup to pull request #38405 (#41641)"

Seems to be causing hanging in CI testing.

This reverts commit 5af1cf0 and this
reverts commit 5a16805, reversing
changes made to 02807b2.

(cherry picked from commit 66f9b55)
vchuravy pushed a commit to JuliaLang/Distributed.jl that referenced this pull request Oct 6, 2023
…stributed_ts" (JuliaLang/julia#41722)

Also reverts "fixup to pull request JuliaLang/julia#38405 (JuliaLang/julia#41641)"

Seems to be causing hanging in CI testing.

This reverts commit 740a33a and this
reverts commit 5a1680533b461471f537f5f5d4ee3c2866b21e1f, reversing
changes made to 02807b279a5e6d5acaeb7095e4c0527e2a5c190e.

(cherry picked from commit bbc9429)
Keno pushed a commit that referenced this pull request Jun 5, 2024
…41722)

Also reverts "fixup to pull request #38405 (#41641)"

Seems to be causing hanging in CI testing.

This reverts commit 740a33a and this
reverts commit 5a16805, reversing
changes made to 02807b2.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants