Fix background job logging#6992
Merged
Merged
Conversation
A new context is created and closed for each state of the MainLoop. But the context of the backgroundJob must stay alive. So we use a context that is owned by the BackgroundJobService. It creates a new logger for each background job and cleans it when the job stops.
If we use the ProxyTerminal in the background jobs, the logs would be spread across different terminals, switching from active client to active client. We want the logs to stick to the client that started the job.
We want the background job to stay alive even if its terminal has been closed and we cannot write to it anymore
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #6842
Description
In order of commits
Use BackgroundJobService context instead of MainLoop context
Use the
LoggerContextof theBackgroundJobServiceto create and clean the loggers of the jobs. This is to avoid using theLoggerContextof theMainLoopwhich is closed after each command: the logger must stay alive after the execution of the command until the job is done.Don't use ProxyTerminal in background job
Do not use the
ProxyTerminalin theConsoleAppenderof a job, so that the logs do not switch from active client to active client. Instead we get the current active terminal at the time the logger is created, and we stick to it until the job is done. So if the job is started by client network 1, the logs will be redirected to client network 1 until the end of the job.Catch
ClosedChannelExceptionin background job loggerIf the terminal is closed before the end of the job we catch the
ClosedChannelExceptionthrown by theConsoleOut, otherwise the job would crash.Results
The logs of a background job are only redirected to the clients that started the job.
If a terminal is killed the job keeps running in the background and the logs are not displayed anywhere.
Limitation
Getting the log of background job whose terminal is closed
Currently the logs of a job whose terminal is closed are lost. It would be nice to get them back when running
bgWaiton that job.Call to
printlnin a background jobA call to
printlnin a background job will always print to the terminal that is active, the ones that is running a task. In particular, if we run the main class of a project without forking, its standard output will move from one active terminal to the other active terminal.Unfortunately I don't think there is a solution for this.
RelayAppenderCurrently the logger of a background job contains a relay appender and all BSP clients receive all logs of all background jobs.
This kind-of pollutes the logs of BSP clients. For instance in Metals:
It would be better if a BSP client only receives the logs of the background jobs that it started.