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

Gelf logging should use its own io_context #650

Closed
heifner opened this issue Jan 19, 2023 · 4 comments
Closed

Gelf logging should use its own io_context #650

heifner opened this issue Jan 19, 2023 · 4 comments
Assignees
Labels

Comments

@heifner
Copy link
Member

heifner commented Jan 19, 2023

Currently gelf_appender uses the main application io_context. This bypasses the application's priority queue and affectively means that gelf logging runs at the highest priority. This is clearly not what is intended. Gelf logging should be modified to use its own thread and its own io_context.

In my mind this should not wait for a new logging framework: eosnetworkfoundation/mandel#152

@spoonincode
Copy link
Member

spoonincode commented Jan 19, 2023

A couple other things to consider while touching this code:

gelf_appender is using some fc wrappers over asio stuff (and the wrappers are fairly old style, such as usage of the deprecated io_service instead of io_context). This is generally something we're trying to move away from: just use boost directly. Consider refactoring accordingly and removing the wrappers.

A separate thread is, imo, not strictly required. There will be very little back pressure on a UDP socket so a large socket buff (say 1MB or 2MB) would be really surprising to fill up (at which point gelf_appender could just drop the message on the floor instead of asyncing it). I'm certainly not saying it needs to be done this way, but imo it would be fine to simplify it further like this. Obviously other opinions welcome. See below.

@heifner
Copy link
Member Author

heifner commented Jan 20, 2023

Probably more gain from moving the format_string and zlib_compress to another thread. But something in gelf logging takes a considerable amount of time: #555

@spoonincode
Copy link
Member

huh, yes that's really good point and almost certainly what ought to be attempted if this code is being touched.

@heifner heifner moved this from Todo to In Progress in Team Backlog Feb 24, 2023
@heifner heifner added the OCI Work exclusive to OCI team label Feb 24, 2023
@huangminghuang huangminghuang moved this from In Progress to Awaiting Review in Team Backlog Feb 28, 2023
@huangminghuang huangminghuang moved this from Awaiting Review to Done in Team Backlog Mar 21, 2023
@spoonincode
Copy link
Member

resolved in #758

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

No branches or pull requests

5 participants