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

implemented async_logger::sync() to solve issue #1696 #2819

Open
wants to merge 3 commits into
base: v1.x
Choose a base branch
from

Conversation

SaltedReed
Copy link

@SaltedReed SaltedReed commented Jul 15, 2023

This pr is to solve issue #1696 and async_logger::sync() allows users to wait for the completion of logging, so they can make sure everything they want to log is safe and can be passed for further processing.

You can use it like this:

    auto thrpool=std::make_shared<spdlog::details::thread_pool>(100, 2);
    auto aloger=std::make_shared<spdlog::async_logger>("aloger", std::make_shared<spdlog::sinks::stdout_color_sink_mt>(), thrpool);

    aloger->info(std::string(1000,'i')); // log something large

    bool fin=aloger->sync(100,1000); // call sync() to wait until logging is flushed

    std::cout << (fin ? "done\n" : "undone\n"); // this shows after the logging on console
    /**
     * @brief block until all messages in thread_pool_ are processed
     * @param intervalMs check whether should return or not on every 'intervalMs' milliseconds
     * @param timeoutMs timeout in milliseconds. -1 means to wait indefinitely
     * @return true: all messages in thread_pool_ are processed, false: otherwise
    */
    bool sync(int intervalMs=100, int timeoutMs=-1);

@SaltedReed SaltedReed changed the title implemented async_logger::sync() to solve issure #1696 implemented async_logger::sync() to solve issue #1696 Jul 15, 2023
@tt4g
Copy link
Contributor

tt4g commented Jul 15, 2023

IMO: Why is this feature implemented in async logger and not in thread pool?
When I saw spdlog::async_logger::sync(), I thought that one async logger would wait until the log messages it sent to the thread pool were processed.
However, since the thread pool is shared by multiple async loggers, it actually waits until the thread pool has completed all processing.

@SaltedReed
Copy link
Author

IMO: Why is this feature implemented in async logger and not in thread pool? When I saw spdlog::async_logger::sync(), I thought that one async logger would wait until the log messages it sent to the thread pool were processed. However, since the thread pool is shared by multiple async loggers, it actually waits until the thread pool has completed all processing.

You are right. I didn't take into consideration the sharing of thread_pool by multiple async_loggers. I'll fix that.

@walkerlala
Copy link
Contributor

I think we should reimplement the async_logger so that it has its own message queue that could be pulled continously by worker threads. Having the queue inside a thread pool make the whole thing more complicated and unnecessary.

By the way, I think the "async_logger::flush()" interface is broken. A "flush" operation should have the semantic of flushing the in-memory data into file (into the OS file-cache at least). A "sync" operation should persist the in-memory cache data into disk. These are long established conventions in the C / C++ / POSIX world and the database world: https://man7.org/linux/man-pages/man3/fflush.3.html , https://man7.org/linux/man-pages/man2/fsync.2.html , and spdlog's implementation is counter-intuitive. This is why there are so many issues and discussions all the way up. #2982 #1696 #2819

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.

3 participants