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

Creating a CsvWriter with custom FrontendOptions gives compile error, and CsvWriter with default FrontendOptions but custom Logger gives runtime assertion violation #609

Closed
jellehierck opened this issue Oct 13, 2024 · 5 comments · Fixed by #610
Labels
bug Something isn't working

Comments

@jellehierck
Copy link

Describe the bug
There seems to be no way to create a CsvWriter when using custom FrontendOptions. There are two scenarios which I tried, and both resulted in unexpected errors. I will give the brief errors here and the full code to reproduce in the next section (probably due to #453).

  • When I create a CsvWriter with custom FrontendOptions (e.g. quill::CsvWriter<MyCsvSchema, CustomFrontendOptions> csv_writer{"orders.csv"};), I get a compilation error that the CsvWriter cannot create a logger:

    [build] /home/jelle/thesis/zmq/quill-temp/include/quill/CsvWriter.h:54:45: error: cannot convert ‘quill::v7::FrontendImpl<quill::v7::FrontendOptions>::logger_t*’ {aka ‘quill::v7::LoggerImpl<quill::v7::FrontendOptions>*’} to ‘quill::v7::LoggerImpl<CustomFrontendOptions>*’ in assignment
    [build]    54 |     _logger = Frontend::create_or_get_logger(
    [build]       |               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
    [build]       |                                             |
    [build]       |                                             quill::v7::FrontendImpl<quill::v7::FrontendOptions>::logger_t* {aka quill::v7::LoggerImpl<quill::v7::FrontendOptions>*}
    ... (left out the rest for brevity)
    
  • When I create a CsvWriter with default quill::FrontendOptions and try to log to a logger which uses custom FrontendOptions, I get a runtime assertion error when compiled with Debug.

    Code snippet:

    CustomLogger* logger = CustomFrontend::create_or_get_logger(
      "root", CustomFrontend::create_or_get_sink<quill::ConsoleSink>("sink_id_1"));
    quill::CsvWriter<MyCsvSchema, quill::FrontendOptions> csv_writer{"orders.csv"};
    LOG_INFO(logger, "CSV writing example");

    The program compiles. When I run it, the following error is raised:

    /home/jelle/thesis/zmq/quill-temp/build/bug_csvwriter
    bug_csvwriter: /home/jelle/thesis/zmq/quill-temp/include/quill/core/ThreadContextManager.h:90: std::conditional_t<(((queue_type == quill::v7::QueueType::UnboundedBlocking) || (queue_type == quill::v7::QueueType::UnboundedUnlimited)) || (queue_type == quill::v7::QueueType::UnboundedDropping)), quill::v7::detail::UnboundedSPSCQueue, quill::v7::detail::BoundedSPSCQueueImpl<long unsigned int> >& quill::v7::detail::ThreadContext::get_spsc_queue() [with quill::v7::QueueType queue_type = quill::v7::QueueType::BoundedDropping; std::conditional_t<(((queue_type == quill::v7::QueueType::UnboundedBlocking) || (queue_type == quill::v7::QueueType::UnboundedUnlimited)) || (queue_type == quill::v7::QueueType::UnboundedDropping)), quill::v7::detail::UnboundedSPSCQueue, quill::v7::detail::BoundedSPSCQueueImpl<long unsigned int> > = quill::v7::detail::BoundedSPSCQueueImpl<long unsigned int>]: Assertion `(_queue_type == queue_type) && "ThreadContext queue_type mismatch"' failed.
    Aborted (core dumped)
    

To Reproduce

I used the following code to get the compilation error when creating a CsvWriter with custom FrontendOptions:

#include "quill/Backend.h"
#include "quill/CsvWriter.h"
#include "quill/Frontend.h"
#include "quill/LogMacros.h"
#include "quill/Logger.h"
#include "quill/backend/BackendOptions.h"
#include "quill/core/Common.h"
#include "quill/sinks/ConsoleSink.h"

struct CustomFrontendOptions
{
  static constexpr quill::QueueType queue_type = quill::QueueType::BoundedDropping;
  static constexpr uint32_t initial_queue_capacity = 128 * 1024;    // 128 KiB
  static constexpr uint32_t blocking_queue_retry_interval_ns = 800; // Ignored for Dropping queue
  static constexpr bool huge_pages_enabled = false;
};

using CustomFrontend = quill::FrontendImpl<CustomFrontendOptions>;

using CustomLogger = quill::LoggerImpl<CustomFrontendOptions>;

struct MyCsvSchema
{
  static constexpr char const* header = "order_id,symbol,quantity,price,side";
  static constexpr char const* format = "{},{},{},{:.2f},{}";
};

int main()
{

  quill::BackendOptions backend_options;
  quill::Backend::start(backend_options);

  // Creating logger amd sink with custom frontend options
  CustomLogger* logger = CustomFrontend::create_or_get_logger(
    "root", CustomFrontend::create_or_get_sink<quill::ConsoleSink>("sink_id_1"));

  // Creating CSV writer with custom frontend options
  quill::CsvWriter<MyCsvSchema, CustomFrontendOptions> csv_writer{"orders.csv"};

  LOG_INFO(logger, "CSV writing example");

  csv_writer.append_row(13212123, "AAPL", 100, 210.32321, "BUY");
  csv_writer.append_row(132121123, "META", 300, 478.32321, "SELL");
  csv_writer.append_row(13212123, "AAPL", 120, 210.42321, "BUY");
}

When compiling this program, I get an error. The full error is:

[build] In file included from /home/jelle/thesis/zmq/quill-temp/bug_csvwriter.cpp:4:
[build] /home/jelle/thesis/zmq/quill-temp/include/quill/CsvWriter.h: In instantiation of ‘quill::v7::CsvWriter<TCsvSchema, TFrontendOptions>::CsvWriter(const string&, char, quill::v7::FilenameAppendOption) [with TCsvSchema = MyCsvSchema; TFrontendOptions = CustomFrontendOptions; std::string = std::__cxx11::basic_string<char>]’:
[build] /home/jelle/thesis/zmq/quill-temp/bug_csvwriter.cpp:41:79:   required from here
[build] /home/jelle/thesis/zmq/quill-temp/include/quill/CsvWriter.h:54:45: error: cannot convert ‘quill::v7::FrontendImpl<quill::v7::FrontendOptions>::logger_t*’ {aka ‘quill::v7::LoggerImpl<quill::v7::FrontendOptions>*’} to ‘quill::v7::LoggerImpl<CustomFrontendOptions>*’ in assignment
[build]    54 |     _logger = Frontend::create_or_get_logger(
[build]       |               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
[build]       |                                             |
[build]       |                                             quill::v7::FrontendImpl<quill::v7::FrontendOptions>::logger_t* {aka quill::v7::LoggerImpl<quill::v7::FrontendOptions>*}
[build]    55 |       _logger_name_prefix + filename,
[build]       |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~        
[build]    56 |       Frontend::create_or_get_sink<FileSink>(filename,
[build]       |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[build]    57 |                                              [open_mode, filename_append]()
[build]       |                                              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[build]    58 |                                              {
[build]       |                                              ~
[build]    59 |                                                FileSinkConfig cfg;
[build]       |                                                ~~~~~~~~~~~~~~~~~~~
[build]    60 |                                                cfg.set_open_mode(open_mode);
[build]       |                                                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[build]    61 |                                                cfg.set_filename_append_option(filename_append);
[build]       |                                                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[build]    62 |                                                return cfg;
[build]       |                                                ~~~~~~~~~~~
[build]    63 |                                              }()),
[build]       |                                              ~~~~~
[build]    64 |       PatternFormatterOptions{"%(message)", "", Timezone::GmtTime});
[build]       |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

When we change the line to create the CsvWriter with default quill::FrontendOptions instead, but still logging to the custom logger using the custom FrontendOptions (this is obviously wrong, as we should not mix default quill::FrontendOptions with custom FrontendOptions as mentioned in the Frontend Options documentation, but since this raises a runtime error instead of a compile-time error, I think it is important to mention anyways):

  ...

  // Creating logger amd sink with custom frontend options
  CustomLogger* logger = CustomFrontend::create_or_get_logger(
    "root", CustomFrontend::create_or_get_sink<quill::ConsoleSink>("sink_id_1"));

  // Creating CSV writer with custom frontend options
  // quill::CsvWriter<MyCsvSchema, CustomFrontendOptions> csv_writer{"orders.csv"};

  // CSV writer with default frontend options, but still using the custom logger
  quill::CsvWriter<MyCsvSchema, quill::FrontendOptions> csv_writer{"orders.csv"};

  LOG_INFO(logger, "CSV writing example");
  ...

The program compiles in Debug. When I run it, the following error is raised:

/home/jelle/thesis/zmq/quill-temp/build/bug_csvwriter
bug_csvwriter: /home/jelle/thesis/zmq/quill-temp/include/quill/core/ThreadContextManager.h:90: std::conditional_t<(((queue_type == quill::v7::QueueType::UnboundedBlocking) || (queue_type == quill::v7::QueueType::UnboundedUnlimited)) || (queue_type == quill::v7::QueueType::UnboundedDropping)), quill::v7::detail::UnboundedSPSCQueue, quill::v7::detail::BoundedSPSCQueueImpl<long unsigned int> >& quill::v7::detail::ThreadContext::get_spsc_queue() [with quill::v7::QueueType queue_type = quill::v7::QueueType::BoundedDropping; std::conditional_t<(((queue_type == quill::v7::QueueType::UnboundedBlocking) || (queue_type == quill::v7::QueueType::UnboundedUnlimited)) || (queue_type == quill::v7::QueueType::UnboundedDropping)), quill::v7::detail::UnboundedSPSCQueue, quill::v7::detail::BoundedSPSCQueueImpl<long unsigned int> > = quill::v7::detail::BoundedSPSCQueueImpl<long unsigned int>]: Assertion `(_queue_type == queue_type) && "ThreadContext queue_type mismatch"' failed.
Aborted (core dumped)

Interestingly, when compiling and running in Release, the program executes fine and the CSV file is written correctly.

Expected Behaviour

  • When creating a CsvWriter with custom FrontendOptions, I would expect no compilation error.
  • When creating a CsvWriter with default quill::FrontendOptions and still logging to a custom logger with custom FrontendOptions, I would expect a compilation error, warning, or runtime error in both Debug and Release mode.

Environment Details

  • Library Version: 7.3.0
  • Linkage: Static
  • Operating System: Ubuntu 20.04
  • Compiler: g++ (Ubuntu 9.4.0-1ubuntu1~20.04.2) 9.4.0

Additional context
Add any other context about the problem here.

@odygrd odygrd linked a pull request Oct 13, 2024 that will close this issue
@odygrd
Copy link
Owner

odygrd commented Oct 13, 2024

Thank you for reporting this!

Once you've decided to use CustomFrontendOptions, it should be consistently applied when creating new Loggers or CsvWriters. This means you should be able to use quill::CsvWriter<CsvSchema, CustomFrontendOptions> csv_writer without encountering any compile errors. A fix will be merged into master shortly.

Would you be able to work with the master branch once the fix is in, or do you need a new release?

@odygrd odygrd added the bug Something isn't working label Oct 13, 2024
@jellehierck
Copy link
Author

Thanks for this amazingly fast fix.

I would like to use a new release but I can work with the master branch until 7.4.0 is out! Out of curiosity, when are you planning to release that?

@odygrd
Copy link
Owner

odygrd commented Oct 13, 2024

Thanks for your patience! Releases typically happen spontaneously, depending on the accumulation of bug fixes and new features. I'm aiming to delay the next release slightly to bundle multiple fixes and reduce the workload for package maintainers. That said, you can expect a new release no later than the beginning of November, though it could happen sooner.

@odygrd odygrd reopened this Oct 13, 2024
@odygrd
Copy link
Owner

odygrd commented Oct 13, 2024

The fix has been merged into master.

Please give it a try when you have a chance, and feel free to reach out if you encounter any further issues.

@jellehierck
Copy link
Author

I have tried the fixed CsvWriter and it is working well! So I believe this issue is fixed. Thanks for your help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants