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

Performance improvements for file logging #9655

Merged
merged 1 commit into from Nov 3, 2023
Merged

Performance improvements for file logging #9655

merged 1 commit into from Nov 3, 2023

Conversation

mathewc
Copy link
Member

@mathewc mathewc commented Nov 1, 2023

Making some performance improvements to file logging based on a recent CRI investigation. In that investigation, we were seeing thread exhaustion in the host process with many threads blocked on file IO. When the issue occurred, the app had FileLoggingMode enabled (see doc for fileLoggingMode) because they had the portal open on the app. This causes the log stream to also go to files. At the time the app was under high load, so there was a lot of file logging happening. Compounding things was the fact that the customer was also logging errors, triggering our "force flush on error" logic.

Pull request checklist

  • My changes do not require documentation changes
    • Otherwise: Documentation issue linked to PR
  • My changes should not be added to the release notes for the next release
    • Otherwise: I've added my notes to release_notes.md
  • My changes do not need to be backported to a previous version
    • Otherwise: Backport tracked by issue/PR #issue_or_pr
  • My changes do not require diagnostic events changes
    • Otherwise: I have added/updated all related diagnostic events and their documentation (Documentation issue linked to PR)
  • I have added all required tests (Unit tests, E2E tests)

@mathewc mathewc requested a review from a team as a code owner November 1, 2023 18:10
@fabiocav
Copy link
Member

fabiocav commented Nov 1, 2023

Approved the changes, but some of the tests seem to be failing here, likely due to the timing difference introducing races.

@mathewc mathewc merged commit 2c1150a into dev Nov 3, 2023
9 checks passed
@mathewc mathewc deleted the file-logging-fix branch November 3, 2023 17:13
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.

None yet

3 participants