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

Configurable time before shutdown #130

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

VladSemenyuk
Copy link
Contributor

@VladSemenyuk VladSemenyuk commented Jul 21, 2021

Implements #1941

This PR is not ready for review.

Risk
This PR makes no API changes.

Summary
Updates docs for #3740

CLA


To prevent missing logs after SDL shutdown, after receiving IGNITION_OFF signal SDL dumps all logs into the file system before shutdown. To control logs flushing process SDL logger has additional ini file options:

* Write all logs to file system before shutdown(using `LoggerImpl::Flush` function)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"write all logs" is misleading info, it does not guarantee all logs are written to the disk with a small maximum wait time.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the string FlushLogMessagesBeforeShutdown in the sdl proposal is better

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't quite get what you mean. Per the proposal FlushLogMessagesBeforeShutdown is Boolean, not string. Could you please clarify this?
I can suggest to change "Write all logs to file system before shutdown" to "Write logs from queue to file system before shutdown"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the following from the proposal
option to flush log messages before shutdown.
option that specifies maximum time before shutdown.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@VladSemenyuk the keyword "all" that causes the confusion, my understanding is everything in the logs will be written to disk, that is all. but it might not be the case

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yang1070 Done

Copy link

@yang1070 yang1070 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the change looks good to me.

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.

2 participants