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

Fixes for runBackground mutex and log management #3971

Merged
merged 15 commits into from
Nov 16, 2024

Conversation

lihaoyi
Copy link
Member

@lihaoyi lihaoyi commented Nov 15, 2024

Fixes #3955.

  • We make runBackground forward the stdout/stderr to $serverDir/{stdout,stderr} instead of os.Inherit. This is necessary because since we started using com-lihaoyi/os-lib@59b5fd9, os.Inherit is automatically pumped to the enclosing task logger, which for runBackground ends up being closed immediately so the logs are lost.

    • Now, the logs are instead picked up asynchronously by the FileToStreamTailer infrastructure, which picks them up and forwards them to any connected client regardless of who started the runBackground process
  • Moved usage of FileToStreamTailer from the mill client to the server.

    • This allows better integration with the Mill logging infrastructure, e.g. ensuring tailed logs properly interact with the multi-line prompt by clearing the prompt before being printed and re-printing the prompt after.
  • Simplified BackgroundWrapper

    • Renamed it MillBackgroundWrapper so it's more clear what it is when seen in jps
    • Use a file-lock for mutex, rather than polling on the process uuid/tombstone files
    • We still need to add a Thread.sleep after we take the lock, because the prior process seems to still hold on to sockets for some period of time. This defaults to 500ms (what is necessary experimentally) but is configurable by the new runBackgroundRestartDelayMillis: T[Int] task
    • Generally unified the creation/shutdown logic within MillBackgroundWrapper, rather than having it split between BackgroundWrapper and def backgroundSetup in the Mill server process

Tested manually by running rm -rf out && /Users/lihaoyi/Github/mill/target/mill-release -w runBackground inside example/javalib/web/1-hello-jetty. Forced updates via Enter in the terminal or via editing server source files. Verified that the runBackground server logs appear in the console and that they do not conflict with the multi-line status prompt

@lihaoyi lihaoyi marked this pull request as ready for review November 16, 2024 02:16
@lihaoyi lihaoyi changed the title Fixes for runBackground Fixes for runBackground mutex and log management Nov 16, 2024
@lihaoyi lihaoyi merged commit 7457601 into com-lihaoyi:main Nov 16, 2024
lihaoyi added a commit that referenced this pull request Nov 16, 2024
Covers the improvement we implemented in
#3971. Fails on any commit
before that PR
@lefou lefou added this to the 0.12.3 milestone Nov 17, 2024
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.

runBackground does no longer forward process output
2 participants