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

async behavior of setLoggerLevel #31

Closed
n506 opened this issue Nov 22, 2015 · 9 comments
Closed

async behavior of setLoggerLevel #31

n506 opened this issue Nov 22, 2015 · 9 comments

Comments

@n506
Copy link
Contributor

n506 commented Nov 22, 2015

hello
i consider this behavior as a bug

f.e.
LOG_INFO(id, "info1");
ILog4zManager::getPtr()->setLoggerLevel(id, LOG_LEVEL_TRACE);
LOG_TRACE(id, "trace");
LOG_INFO(id, "info2");

will result in
info1
info2

trace message are lost
if we add some sleep after setLoggerLevel - ok , message is displayed

behavior is active with LOG4Z_ALL_SYNCHRONOUS_OUTPUT = false
with LOG4Z_ALL_SYNCHRONOUS_OUTPUT = true - all goes as it should

@zsummer
Copy link
Owner

zsummer commented Nov 23, 2015

Design is that. It may be lost some log when set logger filter level. but. I can & will change the design resolve this problem.

@zsummer
Copy link
Owner

zsummer commented Nov 23, 2015

I already fix the first problem at the master.

@zsummer
Copy link
Owner

zsummer commented Nov 23, 2015

first. setLoggerLevel is one immediately sync call, but write log is async call. so it's will lose some log when switch filter level( specific level go up). like this code.

ILog4zManager::getRef().setLoggerLevel(LOG4Z_MAIN_LOGGER_ID, LOG_LEVEL_TRACE);
LOGT("set LOG_LEVEL_TRACE ----------------------------");
ILog4zManager::getRef().setLoggerLevel(LOG4Z_MAIN_LOGGER_ID, LOG_LEVEL_DEBUG);
LOGT("set LOG_LEVEL_TRACE ============================");

it's all log lose at the old log4z version.
current version only lose the second log. it's expect.
at current version. the setLoggerLevel call is immediately and sync when the level go down, and it's will be async when level go up.

@n506
Copy link
Contributor Author

n506 commented Nov 23, 2015

mm. somewhat complex..

may be its better to filter out messages in queue processing procedure and add to queue all messages? in this case with sync setLevel (and other behavior-modifing functions) no messages will be lost?

of course it'll impact performance. in some usecases noticeably. but in other cases - like mine - degraded performance is better than losing messages

moreover, splitting one queue to multiple, one per logger, will allow any dynamic in-sync tuning for loggers and may be even get rid of most hateful comment //! Needs to be called before ILog4zManager::Start, OR Do not call.

@n506
Copy link
Contributor Author

n506 commented Nov 23, 2015

after some thinking, about queue and level..

to have true full in-sync control on loggers behavior and messages control instructions should be also queued mixed with log messages

mm. i think its beyond this ticket/issue

@zsummer
Copy link
Owner

zsummer commented Nov 23, 2015

swich level, itself is filter/discard some log. so i think the "set filter level" operator can tolerate lose some log, now log4z will as more as possible to keep the more log when switch level, I think it's enough, you can assume the operation can't lose any log now.

@zsummer
Copy link
Owner

zsummer commented Nov 23, 2015

I thinked your suggest. the performence is 'queue i/o' > 'log create/destroy' > 'disk io'. so the multi -queue suggest is not important. and the log create/destroy is better important because it's new one fixed big memory each pushlog.
in my project have a lot of trace log, at most of the time it's all filter. if used all message input queue mod, it's not little degraded performence. so although this suggest is simple, i select other.

@zsummer
Copy link
Owner

zsummer commented Nov 23, 2015

at first of this project. I have no think about the sync write log, it's apend to project latter. so it's not perfect, only have the features.
I will think about and complete it later.
thanks your suggest.

@zsummer
Copy link
Owner

zsummer commented Nov 23, 2015

the sync problem may be can use diff log pre-name to solve. it's support hot-update, but have little ugly.

@zsummer zsummer closed this as completed Dec 17, 2015
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

No branches or pull requests

2 participants