logger: Set the queue and the log file up before the writer thread starts - #6
jasonsopko wants to merge 1 commit into
Conversation
luke-jr
left a comment
luke-jr
left a comment
There was a problem hiding this comment.
Why not just move the top of datum_logger_thread down before the thread gets started?
3b8ad35 to
18c2a13
Compare
|
That's simpler, and it removes the window instead of waiting it out. The queue and log file setup now runs in init before pthread_create and the thread keeps only its loop. One thing that followed: a failure in that setup used to call panic_from_thread(), which from the main thread would only spin, so init returns -1 like the other init functions and main exits on it. Checked against an unreachable node: the file now starts with the first startup lines, which are missing on master. |
jasonsopko
changed the title
| datum_logger_init(); | ||
| if (datum_logger_init()) { | ||
| DLOG_FATAL("Error initializing the logger!"); | ||
| usleep(100000); |
There was a problem hiding this comment.
No, the logger isn't up on that path, so the message prints directly. Removed.
| const char *level_text[] = { " ALL", "DEBUG", " INFO", " WARN", "ERROR", "FATAL" }; | ||
|
|
||
| volatile bool datum_logger_initialized = false; | ||
| static FILE *log_handle = NULL; |
There was a problem hiding this comment.
How about passing it as the thread's ptr instead?
There was a problem hiding this comment.
Done, init passes the handle as the thread's argument and the static is gone. Init also checks pthread_create now, since it sets the ready flag itself.
…arts datum_logger_init() returned as soon as the writer thread was created, while the thread was still allocating its queue and opening the log file. Anything logged in that window went to the console or nowhere, so the first lines of a run were missing from the file. Do that setup in init itself, before pthread_create, and leave the thread with only its loop; init passes it the open log handle as its argument. A failure to allocate or to open the file now returns -1 like the other init functions, and main exits on it, instead of panic_from_thread(), which from the main thread would only spin. Init now marks the logger ready only once the thread has started, so a failed pthread_create still logs to the console instead of queueing for a thread that does not exist. A run against an unreachable node now has its first startup lines in the file; on master the file starts several lines in.
18c2a13 to
97c7eef
Compare
2 participants
What
datum_logger_initdoes the logger's setup itself, the queue allocation and opening the log file, before it starts the writer thread. The thread keeps only its loop.Why
It started the thread and returned at once. Anything logged in the next millisecond, which includes
datum_protocol_initanddatum_api_init, went to the console if console logging was on and nowhere if it was off. Withlog_to_console: falseand a log file, theDATUM pool host is blank. NON-POOLED MINING!warning never reached the file.The first version of this PR waited for the thread instead. Doing the setup in init removes the window rather than waiting it out, and it makes a setup failure an init failure: it now returns -1 like the other init functions and main exits on it, where the thread's
panic_from_thread()would only have spun.How I tested
Same config both ways, file on, console off, a gateway pointed at an unreachable node, five seconds. On master the file starts several lines in and the startup key and warning lines are missing; with this change the file starts with them. An unwritable log path now logs two FATAL lines and exits. Built with
-Wall -Werror,--testpasses.Risk and rollback
None to the running loop; the log handle moved from a thread local to file scope, which the rotate path already treated as shared. Revert the commit.