Fix deadlock in case test performs lots of checks without yielding #417
Conversation
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
Before commit 6c5c8b7 ("Print luatest logs to stdout when unified log is disabled"), luatest implicitly configured the logger to operate in the non-blocking mode by using a pipe for the logs destination. Now, it uses a file albeit the file actually points to a pipe file descriptor. Tarantool thinks it's a regular file and writes all logs in the blocking mode. This leads to a deadlock if a test performs a lot of checks (which are logged) without yielding execution to the fiber processing logs because the size of a pipe is quite limited. Let's fix this issue by writing all logs through a pipe, as we did before. Let's also explicitly enable non-blocking logging to avoid breakages like this in future. The fix isn't ideal because it may lead to dropping logs. We should probably handle logs emitted by the luatest runner in a separate process, like we do for processes spawned in tests. Closes tarantool#416
Totktonada
approved these changes
Apr 6, 2025
There was a problem hiding this comment.
I'm OK with it taking it as a hotfix.
I think that we can bufferize luatest's output in runtime and actually call log.<level_func> from a separate fiber. That's similar to the test-run's problem with pipe buffer size.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Before commit 6c5c8b7 ("Print luatest logs to stdout when unified log is disabled"), luatest implicitly configured the logger to operate in the non-blocking mode by using a pipe for the logs destination. Now, it uses a file albeit the file actually points to a pipe file descriptor. Tarantool thinks it's a regular file and writes all logs in the blocking mode. This leads to a deadlock if a test performs a lot of checks (which are logged) without yielding execution to the fiber processing logs because the size of a pipe is quite limited. Let's fix this issue by writing all logs through a pipe, as we did before. Let's also explicitly enable non-blocking logging to avoid breakages like this in future.
The fix isn't ideal because it may lead to dropping logs. We should probably handle logs emitted by the luatest runner in a separate process, like we do for processes spawned in tests.
Closes #416