Including instrumented_mutex.h leads to compilation errors

[udi-speedb]

When adding an include of instrumented_mutex.h in write_buffer_manager.h compilation fails:

#include "rocksdb/cache.h"
#include "monitoring/instrumented_mutex.h"

namespace ROCKSDB_NAMESPACE {
class CacheReservationManager;

When adding the same to the write_buffer_manager.cpp compilation passes. This will probably happen with other header files, I just stumbled on this while modifying write_buffer_manager.h.
This implies some issue somewhere in the include chain:

udi@udi-speedb:~/speedb-oss-OTHER$ make memtable/write_buffer_manager.o
$DEBUG_LEVEL is 1
Makefile:165: Warning: Compiling in debug mode. Don't use the resulting binary in production
* GEN     make_config.mk
  CC       memtable/write_buffer_manager.o
In file included from ./port/port_posix.h:16,
                 from ./port/port.h:18,
                 from ./monitoring/statistics.h:16,
                 from ./monitoring/instrumented_mutex.h:8,
                 from ./include/rocksdb/write_buffer_manager.h:22,
                 from memtable/write_buffer_manager.cc:10:
./include/rocksdb/options.h:886:19: error: ‘WriteBufferManager’ was not declared in this scope
  886 |   std::shared_ptr<WriteBufferManager> write_buffer_manager = nullptr;
      |                   ^~~~~~~~~~~~~~~~~~
./include/rocksdb/options.h:886:37: error: template argument 1 is invalid
  886 |   std::shared_ptr<WriteBufferManager> write_buffer_manager = nullptr;
      |                                     ^
./include/rocksdb/options.h:886:62: error: cannot convert ‘std::nullptr_t’ to ‘int’ in initialization
  886 |   std::shared_ptr<WriteBufferManager> write_buffer_manager = nullptr;
      |                                                              ^~~~~~~
make: *** [Makefile:2422: memtable/write_buffer_manager.o] Error 1

As a workaround/patch, I managed to resolve the compilation issue of write_buffer_manager file by forward declaring WriteBufferManager:

#include "rocksdb/cache.h"
namespace ROCKSDB_NAMESPACE {
class WriteBufferManager;

}
#include "monitoring/instrumented_mutex.h"

However, that causes failure of multiple other files including write_buffer_manager.h

[Yuval-Ariel]

happened while trying to include in write_controller.h as well.
found a work-around in a similar manner:

1. forward declare InstrumentedMutex in db/write_controller.h
2. include "monitoring/instrumented_mutex.h" in db/write_controller.cc
3. use std::unique_ptr

[udi-speedb]

This is not a fix. It shouldn't happen and should be fixed in RocksDB / SpeeDB

[Yuval-Ariel]

thats why we have this ticket......

[Yuval-Ariel]

in rocksdb v7.7.8, also adding in write_buffer_manager.h #include "monitoring/statistics.h" causes the compilation errors

[AmnonHanuhov]

@Yuval-Ariel @udi-speedb Does it make any difference what you are compiling with? I mean Makefile vs cmake?

[Yuval-Ariel]

i havent tried with cmake

[udi-speedb]

@AmnonHanuhov - I forgot to answer. I apologize.
Have you checked it with cmake?
Does it also fail with cmake?

[AmnonHanuhov]

@udi-speedb Yes

[AmnonHanuhov]

The circular dependency is as follows:
in @udi-speedb 's case when he tries to include monitoring/instrumented_mutex.h from include/rocksdb/write_buffer_manager.h:
include/rocksdb/write_buffer_manager.h includes monitoring/instrumented_mutex.h which includes port/port.h which includes port/port_posix.h which includes include/rocksdb/options.h which includes include/rocksdb/write_buffer_manager.h

in @Yuval-Ariel 's case when he tries to include monitoring/statistics.h from include/rocksdb/write_buffer_manager.h:
include/rocksdb/write_buffer_manager.h includes monitoring/statistics.h which includes port/port.h which includes port/port_posix.h which includes include/rocksdb/options.h which includes include/rocksdb/write_buffer_manager.h

[udi-speedb]

In that case, it seems that forward declaring WriteBufferManager in options.h instead of including write_buffer_manager.h, and including write_buffer_manager.h in options.cc should do the trick (assuming it does solve it)

[AmnonHanuhov]

In that case, it seems that forward declaring WriteBufferManager in options.h instead of including write_buffer_manager.h, and including write_buffer_manager.h in options.cc should do the trick (assuming it does solve it)

Including write_buffer_manager.h in options.cc doesn't work since db_options.cc includes options.h and tries to access members of WriteBufferManager which results in an error of "incomplete type". The solution I have found is to indeed forward declare WriteBufferManager in options.h as you have suggested, but include it in db_options.h instead of options.cc. Thoughts?

[udi-speedb]

A source file should include any header it needs. Therefore, if db_options.cc needs to use WriteBufferManager, then it should include its header, as it should any other header it relies on for its operation