westerndigitalcorporation / zenfs

ZenFS is a storage backend for RocksDB that enables support for ZNS SSDs and SMR HDDs.
GNU General Public License v2.0
238 stars 87 forks source link

fs: Improve debugability by enabling libzbd error logging in zenfs. #171

Closed imukherjee-wdc closed 2 years ago

imukherjee-wdc commented 2 years ago

Addresses issue #84 and improves debugability by enabling libzbd error logging in zenfs. Currently there's no debug build, hence the error logging level is set to ZBD_LOG_ERROR. Can be changed to ZBD_LOG_DEBUG if needed.

imukherjee-wdc commented 2 years ago

Still not able to find a way to initialise the libzbd per thread variable for all threads. Need to come to a conclusion on how to proceed on this.

Tried the following code which initialises the variable for only 1 thread that calls the Open method.

class LibZbdSetLogLevel { public: LibZbdSetLogLevel() { zbd_set_log_level(ZBD_LOG_DEBUG);

if (WITH_TESTS == OFF)

    zbd_set_log_level(ZBD_LOG_ERROR);

endif

}

};

IOStatus ZonedBlockDevice::Open(bool readonly, bool exclusive) { static thread_local LibZbdSetLogLevel zenfs_libzbd_log_level_init; ... ... }

metaspace commented 2 years ago

Tried the following code which initialises the variable for only 1 thread that calls the Open method.

This will only initialize the variable for threads that call the Open() function. Did you try making it a global variable? I think that should work.

imukherjee-wdc commented 2 years ago

Tried the following code which initialises the variable for only 1 thread that calls the Open method.

This will only initialize the variable for threads that call the Open() function. Did you try making it a global variable? I think that should work.

If you make this a global, it's not initialised even for a single thread.

metaspace commented 2 years ago

If you make this a global, it's not initialised even for a single thread.

According to the c++17 standard section 6.7.2 basic.stc.thread, initialization is allowed to be lazy. It just has to occur before a thread uses the variable. So we have to make sure that all threads that use libzbd access the global. But at that point, we might as well just call zbd_set_log_level().

Not sure what a good approach is here. What do people do in general logging frameworks?

yhr commented 2 years ago

closing this as it turned out that the per-thread loglevel setting is unpractical