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

thermald: add function for blocking cloudlog instead of sleep #23489

Closed
wants to merge 20 commits into from

Conversation

whowe444
Copy link

perform fsync after cloudlog.info to make sure logs are written to disk before shutdown

self.info(msg)
# else get_file_handler().emit(msg)
# pass
fd = os.open(fsync_dir, os.O_RDONLY)
Copy link
Contributor

Choose a reason for hiding this comment

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

You're probably looking for something like

    for h in self.handlers:
      h.flush()

However, that also doesn't guarantee the message has been picked up by logmessaged already and has hit disk.

Copy link
Author

Choose a reason for hiding this comment

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

So now I'm adding the file handler which bypasses logmessaged since it uses the filestream, calling handle which calls handle on all the handlers, which emits in all the handlers, but emit for the file_handler is just writing to a filestream and calling flush... then I make sure to fsync the whole directory

@adeebshihadeh adeebshihadeh linked an issue Jan 11, 2022 that may be closed by this pull request
@adeebshihadeh
Copy link
Contributor

Making a blocking cloudlog is quite a bit more complicated than this. I would recommend something simpler for a first contribution.

@whowe444
Copy link
Author

Making a blocking cloudlog is quite a bit more complicated than this. I would recommend something simpler for a first contribution.

Does it have to use logmessaged? In one of the comments it says that you can use the get_file_handler in the case that logmessaged isn't running. This uses SwaglogRotatingFileHandler which can call emit directly, which then calls flush on the file stream. So might just have to make sure to fsync

@whowe444 whowe444 closed this Jan 12, 2022
@whowe444 whowe444 reopened this Jan 12, 2022
@pd0wm
Copy link
Contributor

pd0wm commented Jan 14, 2022

Think the cleanest solution is to set the DoShutdown param. Then manager will handle the rest of the shutdown. That shuts down logmessaged cleanly before shutting down.

@pd0wm
Copy link
Contributor

pd0wm commented Jan 14, 2022

#23528

@pd0wm pd0wm closed this Jan 14, 2022
@whowe444 whowe444 deleted the willh-blockinginfo branch January 16, 2022 03:08
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.

Joystick 🕹️
3 participants