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

Add binary logging to Ganesha #1103

Open
liorsu opened this issue Mar 21, 2024 · 7 comments
Open

Add binary logging to Ganesha #1103

liorsu opened this issue Mar 21, 2024 · 7 comments

Comments

@liorsu
Copy link
Contributor

liorsu commented Mar 21, 2024

By adding binary logging infrastructure to Ganesha, users will be able to gain better performance as format strings won't be processed in run-time and logs will be of smaller size.

We can also add to that an option to store logs in a cyclic buffer in-memory that will also reduce the disk iops that Ganesha is doing by default, while allowing users to configure an online / on-demand logs spooler to convert in-memory binary logs to printable logs, that can also be sent to an external service.

The ultimate goal of this work is to improve the IO performance that Ganesha can provide to client's while also allowing us to configure it to output logs with a higher granularity to improve its observability.

There are many out-of-the-box efficient binary logging infrastructures that exist in C++ that we will be able to benefit from but that will require an preliminary work to get Ganesha to fully compile in C++.

  1. NanoLog- https://github.com/PlatformLab/NanoLog (2.4k stars on GitHub, 300+ forks, ISC license [allows everything])
  2. spdlog- https://github.com/gabime/spdlog (21k stars, 4.2k forks, MIT license)
  3. quil- https://github.com/odygrd/quill (1k stars, 100+ forks, MIT license)
  4. binlog- https://github.com/morganstanley/binlog (developed by Morgan Stanley, Apache license)

That’s a partial list, the quil’s repository has a benchmark for those ones and more- https://github.com/odygrd/logger_benchmarks/tree/master/benchmarks/call_site_latency

List of features they provide-

If we decide to go that way, we shall launch an effort to compile NFS-Ganesha in modern C++ and then discuss on the binary logging infrastructure that we would want to integrate with.

LMK what you think.

@mattbenjamin
Copy link
Contributor

@liorsu I like the idea, to be honest, though I think we need it in ceph a lot more; I'm a bit concerned about compiling nfs-ganesha as c++, though.

@liorsu
Copy link
Contributor Author

liorsu commented Mar 21, 2024

I do see that as a challenge that will require some work, though what are the issues that makes you feel concerned about it?

@mattbenjamin
Copy link
Contributor

at c++98 (the last time I seriously considered doing that) there were small differences between c and c++ semantics in a number of areas, and it seems as if the gaps may have widened (e.g., atomics and sequence points?)--but I'm admittedly not an expert. More broadly, I think there'd be concern that compiling the whole of ganesha as c++ might lead to random drift towards c++ implementation. Right now ganesha has a pretty clean code profile and is mature. It would be problematic to lose that.

@dang
Copy link
Contributor

dang commented Mar 21, 2024

We already have gtests in Ganesha, so portions of it are compiled as C++. It was a fair amount of work to fix up those bits.

@liorsu
Copy link
Contributor Author

liorsu commented Mar 25, 2024

I guess that having C++ support will allow developers to make use of C++ features.
Personally, I see it as a positive byproduct, for example it will make it easier to integrate with additional CPP libraries and infrastructures (binary logging for improve performance, prometheus-cpp improve observability, gtest for additional UT coverage , etc..) and use cpp data structures and algorithms, if we choose to.

Otherwise, we have an internal legacy binary logging infrastructure in C that we can use, but as it is internal we won't be able to contribute it and the community wouldn't be able to benefit from that and that is why we explored that direction first.

@dang As I tried to do it and saw that it still requires some work, for example there are many places that use C++ reserved keywords in the code and there are some other changes that a C++ compiler will require

reserved keywords for example-
git grep "[ *,]this[,-]" -- ".[hc]"
git grep "[ *,]export[ ,]" -- "
.[hc]"

@xiaods
Copy link

xiaods commented May 22, 2024

prefer c native binary logging solution

@ffilz
Copy link
Member

ffilz commented May 22, 2024

I had a look at the suggested loggers above...

One of the biggest concerns is the major effort to convert our log messages over to some new format. Potentially an automated tool could be written to do it, but still a lot of work. And then of course merging across the change will require manual merging.

None of the options seemed to support our component concept, though at least Quill suggests custom tags that maybe would implement?

Most don't use printf formatting strings, so a change there...

In some sense, I'd be tempted to roll our own so the log invocation doesn't need changing, but if we are to put logs into a binary file, there's the question of how to link formatting strings to the binary format (if log to file is simply another thread processing the circular queue, we can store format string pointer in the queue, and processing thread can sprintf using the string pointer).

No matter what, I want to make sure we have some way to avoid loss of messages on a crash, but given we already have a handler for faults that logs stack back trace, we could also process the queue there also.

I wonder if there's any sensible way to use one of these libraries by front ending it, so we use our current macro invocations, though the macro may totally change, and then a front end function accesses the library. This would actually allow our code to stay C and just the interface needs to compile C++ (we already do this kind of thing with libcephfs).

Another consideration is integration with the FSAL back end logging...

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

No branches or pull requests

5 participants