Hi, all Kvrocks contributors, we want to donate Kvrocks to Apache Software Foundation, the first step for us is changing the license to Apache License Version 2.0. As we know, Kvrocks is an open source software under the BSD-3-Clause license, we truly thank every contributor and user, you guys make Kvrocks great and have an active community, before we could submit PR to change license, we must to get your approval.

@git-hulk @karelrooted @ShooterIT @Alfejik @caipengbo @ChrisZMF @shangxiaoxiong @ColinChamber @guoxiangCN @calvinxiao @patpatbear @WyattJia @smartlee @popunit @clyang @byronhe @QQxiaoyuyu @nioshield @singgel @liangjingyang @gaoyuanzong @rongzeng54 @unext-ops @fishery86 @xujianhai666 @LykxSassinator

Please reply "Approved" if you agree.

1. masterA 有数据 slaveA slaveof过来，数据可以同步，flushall masterA的数据， slaveA 没有同步清空掉。都执行过dbsize scan 2)masterB 没数据 slaveB 同步过来up后 ，masterB上执行写数据,数据没有同步到slaveB 3)slaveof后 slave节点上经常会报以下错误, 但这个目录是清空后再启动slave的

E0216 13:35:53.319345 26935 storage.cc:703] [storage] Failed to delete dir: IO error: file rmdir: ./kvrocksdatamaster/backup/meta: Directory not empty
E0216 13:35:53.415446 26935 replication.cc:584] [replication] Failed to restore backup while IO error: No such file or directoryWhile opening a file for sequentially reading: ./kvrocksdatamaster/backup/meta/5: No such file or directory
Segmentation fault (core dumped)

Reverts KvrocksLabs/kvrocks#442 l find that new multi-get api can get keys which have been deleted before. the case could be reproduced with my PR KvrocksLabs/kvrocks/pull/444 . It seems a bug of RocksDB and I would submit a relevant PR to RocksDB in the near future.

Search before asking

• [X] I had searched in the issues and found no similar issues.

Motivation

Modern CMake provides many amazing features to organize the build structure of a project in a more convenient way, and we can make full use of them to improve the development experience! @tisonkun had a good suggestion in https://github.com/apache/incubator-kvrocks/issues/562#issuecomment-1123771508 of creating a tracking issue to help keep the discussion more focused, so this issue is created for that purpose.

Solution

The list below tracks some of the improvements that have been proposed so far, and I'd appreciate it if anyone came up with new ideas for improvements, PRs for current improvements, or participated in this discussion!

• [x] #561
• [x] #562
• [x] #574
• [x] #569
• [x] #563
• [x] #580
• [x] #589
• [x] #590

Are you willing to submit a PR?

• [X] I'm willing to submit a PR!

Search before asking

• [X] I had searched in the issues and found no similar issues.

Version

redisGenericCommand use raiseError to process errors, but raiseError will perform a longjmp in the lua_error api call, so raiseError never returns.

longjmp does not compatible with RAII in C++ (C++ exceptions may use longjmp in implementation but the compiler will insert some dtor call autometically and carefully, i.e. stack unwinding), so every object in the stack of redisGenericCommand will never be destroyed, i.e. no destructor will be call, hence all resource allocated in ctor like std::vector, std::unique_ptr etc. cannot be released.

@git-hulk

Minimal reproduce step

./runtest --dont-clean on a kvrocks binary with Address Sanitizer enabled. (try to build with #599) You can only run tests in unit/scripting to save time : )

What did you expect to see?

No memory leaks

What did you see instead?

Memory leaks are reported by Address Sanitizer

Anything Else?

No response

Are you willing to submit a PR?

• [X] I'm willing to submit a PR!

Describe the bug A clear and concise description of what the bug is.

• OS: MAC OS 10.15.7
• Version 2.0.5 @6a4ed16a

get following error message when try to start up kvrocks ( first time run )

E1229 15:41:02.217978 276704704 main.cc:314] Failed to open: IO error: No such file or directory: While opening a file for sequentially reading: /tmp/kvrocks/db/CURRENT: No such file or directory

/tmp/kvrocks directory did exist but no sub-directory ./db under it.

FetchContent has been adopted by many projects as a dependency fetching method introduced in newer versions of cmake, for example the official documentation for gtest already recommends using FetchContent to import itself into other projects (replacing the previous recommended ExternalProject).

The advantage of FetchContent is that it is executed during the cmake configuration phase, which allows the configuration information of dependent projects to be introduced into the main project, unlike ExternalProject, which is executed during the build phase.

This is quite useful especially for dependency projects built with cmake, because the configured cmake target of a dependency project can be used directly in the main project via add_subdirectory (e.g. gtest::gtest), instead of having to manually configure the dependency's header directories, static or shared libraries, compilation options, install directories, etc. and keeping maintaining these redundant information in the main project.

This also makes git submodules unnecessary: by FetchContent, dependencies can be downloaded and configured at the cmake configuration stage, and URLs and commit hash/tag can be flexibly adjusted based on cmake options.

There are NOT a lot of dependencies in kvrocks, and I think using FetchContent to manage dependencies as opposed to ExternalProject will improve the development experience. Of course, introducing a dependency manager such as vcpkg or conan may be a good option, but it introduces additional complexity in development and build management, so there is a trade-off.

I'm happy to provide PRs if that sounds like a good idea.

@tisonkun

Describe the bug Kvrocks could not swich databases, any database key-value changes effect to all-other databases;

• OS: Ununtu 20.04 /docker
• Version 2.06/2.05

To Reproduce Steps to reproduce the behavior:

1. select 1 -> set t 1 -> get t -> "1"
2. select 2 -> get t -> "1"

Expected behavior database-1 key-value should not exist in database-2

127.0.0.1:6666> info
# Server
version:2.0.6
git_sha1:7c23f2fe
os:Linux 5.11.0-36-generic x86_64
gcc_version:7.5.0
arch_bits:64
process_id:1
tcp_port:6666
uptime_in_seconds:1277
uptime_in_days:0
# Clients
maxclients:10000
connected_clients:2
monitor_clients:0
# Memory
used_memory_rss:26529792
used_memory_human:25.30M
used_memory_lua:36864
used_memory_lua_human:36.00K
# Persistence
loading:0
bgsave_in_progress:0
last_bgsave_time:-1
last_bgsave_status:ok
last_bgsave_time_sec:-1
# Stats
total_connections_received:514
total_commands_processed:3090
instantaneous_ops_per_sec:1
total_net_input_bytes:76505
total_net_output_bytes:3490146
instantaneous_input_kbps:0
instantaneous_output_kbps:5
sync_full:0
sync_partial_ok:0
sync_partial_err:0
pubsub_channels:0
pubsub_patterns:0
# Replication
role:master
connected_slaves:0
master_repl_offset:1175757
# CPU
used_cpu_sys:0
used_cpu_user:1
# Commandstats
cmdstat_auth:calls=514,usec=1788,usec_per_call=3
cmdstat_client:calls=511,usec=747,usec_per_call=1
cmdstat_command:calls=3,usec=1770,usec_per_call=590
cmdstat_config:calls=510,usec=82148,usec_per_call=161
cmdstat_get:calls=4,usec=148,usec_per_call=37
cmdstat_info:calls=515,usec=373014,usec_per_call=724
cmdstat_keys:calls=5,usec=1709,usec_per_call=341
cmdstat_select:calls=6,usec=16,usec_per_call=2
cmdstat_set:calls=2,usec=268,usec_per_call=134
cmdstat_slowlog:calls=1020,usec=3864,usec_per_call=3
# Keyspace
# Last scan db time: Thu Jan  1 00:00:00 1970
db0:keys=0,expires=0,avg_ttl=0,expired=0
sequence:1175757
used_db_size:51464
max_db_size:0
used_percent: 0%
disk_capacity:490651459584
used_disk_size:111368089600
used_disk_percent: 22%
# RocksDB
estimate_keys[default]:0
block_cache_usage[default]:4256
block_cache_pinned_usage[default]:480
index_and_filter_cache_usage[default]:0
level0_file_limit_slowdown[default]:0
level0_file_limit_stop[default]:0
pending_compaction_bytes_slowdown[default]:0
pending_compaction_bytes_stop[default]:0
memtable_count_limit_slowdown[default]:0
memtable_count_limit_stop[default]:0
estimate_keys[metadata]:4
block_cache_usage[metadata]:4256
block_cache_pinned_usage[metadata]:480
index_and_filter_cache_usage[metadata]:224
level0_file_limit_slowdown[metadata]:0
level0_file_limit_stop[metadata]:0
pending_compaction_bytes_slowdown[metadata]:0
pending_compaction_bytes_stop[metadata]:0
memtable_count_limit_slowdown[metadata]:0
memtable_count_limit_stop[metadata]:0
estimate_keys[zset_score]:0
block_cache_usage[zset_score]:4256
block_cache_pinned_usage[zset_score]:480
index_and_filter_cache_usage[zset_score]:0
level0_file_limit_slowdown[zset_score]:0
level0_file_limit_stop[zset_score]:0
pending_compaction_bytes_slowdown[zset_score]:0
pending_compaction_bytes_stop[zset_score]:0
memtable_count_limit_slowdown[zset_score]:0
memtable_count_limit_stop[zset_score]:0
estimate_keys[pubsub]:0
block_cache_usage[pubsub]:432
block_cache_pinned_usage[pubsub]:0
index_and_filter_cache_usage[pubsub]:0
level0_file_limit_slowdown[pubsub]:0
level0_file_limit_stop[pubsub]:0
pending_compaction_bytes_slowdown[pubsub]:0
pending_compaction_bytes_stop[pubsub]:0
memtable_count_limit_slowdown[pubsub]:0
memtable_count_limit_stop[pubsub]:0
estimate_keys[propagate]:0
block_cache_usage[propagate]:0
block_cache_pinned_usage[propagate]:0
index_and_filter_cache_usage[propagate]:0
level0_file_limit_slowdown[propagate]:0
level0_file_limit_stop[propagate]:0
pending_compaction_bytes_slowdown[propagate]:0
pending_compaction_bytes_stop[propagate]:0
memtable_count_limit_slowdown[propagate]:0
memtable_count_limit_stop[propagate]:0
all_mem_tables:7343776
cur_mem_tables:7343776
snapshots:0
num_immutable_tables:0
num_running_flushes:0
memtable_flush_pending:0
compaction_pending:1
num_running_compactions:0
num_live_versions:5
num_superversion:7
num_background_errors:0
flush_count:0
compaction_count:2
put_per_sec:0
get_per_sec:0
seek_per_sec:0
next_per_sec:0
prev_per_sec:0
is_bgsaving:no
is_compacting:no

127.0.0.1:6666> set ttt 1
OK
127.0.0.1:6666> get ttt
"1"
127.0.0.1:6666> select 2
OK
127.0.0.1:6666[2]> get ttt
"1"

Describe the bug After SLAVEOF, 2 instances can contain different values for a given key.

To Reproduce Start 2 kvrocks instances and execute the following commands in the respective instances.

• SERVER2: SLAVEOF SERVER1
• SERVER2: SLAVEOF NO ONE
• SERVER2: SET test 222
• SERVER1: SET test 111
• SERVER2: SLAVEOF SERVER1
• SERVER2: GET test
  • Returns "222". However, it should return "111".

If a new SET command is used on SERVER1, SERVER2 immediately picks up the latest value. This issue doesn't occur if, on the "test" key, SERVER1 executed multiple SET commands while SERVER2 was not a slave.

Expected behavior After SLAVEOF command, SERVER2 should always contain identical data to SERVER1.

Describe

If your kvrocks config file has multiple rename-command, kvrocks would generate duplicate configuration items after executing config rewrite, whatever automatically or manually, and leads kvrocks to fail to restart.

It is introduced in #272

one example

rename-command debug                               290033_dont_run_debug_rudely
rename-command bgsave                              290033_dont_run_bgsave_rudely
rename-command shutdown                            290033_dont_run_shutdown_rudely
rename-command keys                                290033_dont_run_keys_rudely
rename-command debug                               290033_dont_run_debug_rudely

there are two debug rename-commands, so there will be the following error logs.

* Failed to load config, err: at line: #L19, err: No such command in rename-command

How to fix

• you can find the duplicate configuration items, and remove duplicate items, only keep one remame-command for one command.
• of course, you can cherry-pick this commit https://github.com/KvrocksLabs/kvrocks/commit/6c8534a17f056f06faa09bfeada3f92df686975a (#429 )
• use 2.0.6 version (coming soon)

Close #621.

Benchmark between make and ninja build for kvrocks can be found in #625. TL;DR: The benchmark shows that ninja is slightly slower than make build in clean build, but has huge advantages in incremental build, which is very friendly to kvrocks developers who need to do frequent incremental builds.

So in this PR, kvrocks still use make by default, but the problem that blocks ninja backend to successfully build in cmake is fixed and now build.sh has a --ninja option to enable ninja.

Fix this warning:

incubator-kvrocks/src/scripting.cc: In function 'int Lua::raiseError(lua_State*)':
incubator-kvrocks/src/scripting.cc:783:1: Warning: 'noreturn' function does return
  783 | }
      | ^

Both clangd, ccls, clang-tidy and other C++ language tools (like cppcheck) need compile_commands.json (a database for compile commands) to retrieve compile information like include dirs, standard, compile options, and more. CMake provide an option CMAKE_EXPORT_COMPILE_COMMANDS to generate the file. So setting it to ON as a default behavior can make these tools more easier to use.

Small change about README.

When I read the README and operate according to it, I'm a little confuse about connecting. Because the doc about connecting is in the Docker part.

It's clearer that split the doc about connect and docker.

I set the order of doc :

• run kvrocks.
• run kvrocks with Docker.
• connect.

Search before asking

• [X] I had searched in the issues and found no similar issues.

Motivation

As mentioned in https://github.com/apache/incubator-kvrocks/pull/646#issuecomment-1162660044, some integration test cases failed frequently which may frustrate and confuse contributors, so I think we can find out and disable them from old runs.

Solution

• [ ] Find out all failure frequently cases from old runs
• [ ] Disable them first, then fix them one by one

Are you willing to submit a PR?

• [ ] I'm willing to submit a PR!

Search before asking

• [X] I had searched in the issues and found no similar issues.

Version

unstable, linux

Minimal reproduce step

./build.sh build --unittest -DENABLE_TSAN=ON
./build/unittest

What did you expect to see?

No TSan report.

What did you see instead?

Selected TSan log in unittest:

WARNING: ThreadSanitizer: data race (pid=14679)
  Read of size 1 at 0x7b7c0000673b by thread T1:
    #0 memcmp <null> (libtsan.so.0+0x67cb3)
    #1 rocksdb::Slice::compare(rocksdb::Slice const&) const /home/runner/work/incubator-kvrocks/incubator-kvrocks/build/_deps/rocksdb-src/include/rocksdb/slice.h:250 (unittest+0x5d75ef)
    #2 Compare /home/runner/work/incubator-kvrocks/incubator-kvrocks/build/_deps/rocksdb-src/util/comparator.cc:34 (unittest+0x5d75ef)
    #3 Engine::SubKeyFilter::Filter(int, rocksdb::Slice const&, rocksdb::Slice const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*, bool*) const /home/runner/work/incubator-kvrocks/incubator-kvrocks/src/compact_filter.cc:111 (unittest+0x18605c)
    #4 rocksdb::CompactionFilter::FilterV2(int, rocksdb::Slice const&, rocksdb::CompactionFilter::ValueType, rocksdb::Slice const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*) const /home/runner/work/incubator-kvrocks/incubator-kvrocks/build/_deps/rocksdb-src/include/rocksdb/compaction_filter.h:171 (unittest+0x67ba24)
    #5 rocksdb::CompactionIterator::InvokeFilterIfNeeded(bool*, rocksdb::Slice*) /home/runner/work/incubator-kvrocks/incubator-kvrocks/build/_deps/rocksdb-src/db/compaction/compaction_iterator.cc:276 (unittest+0x67ba24)
  Previous write of size 1 at 0x7b7c0000673b by main thread (mutexes: write M19766):
    #0 memcpy <null> (libtsan.so.0+0x61afe)
    #1 memcpy /usr/include/x86_64-linux-gnu/bits/string_fortified.h:34 (unittest+0x427deb)
    #2 rocksdb::MemTable::Add(unsigned long, rocksdb::ValueType, rocksdb::Slice const&, rocksdb::Slice const&, rocksdb::ProtectionInfoKVOS<unsigned long> const*, bool, rocksdb::MemTablePostProcessInfo*, void**) /home/runner/work/incubator-kvrocks/incubator-kvrocks/build/_deps/rocksdb-src/db/memtable.cc:556 (unittest+0x427deb)
    #3 Redis::Bitmap::SetBit(rocksdb::Slice const&, unsigned int, bool, bool*) /home/runner/work/incubator-kvrocks/incubator-kvrocks/src/redis_bitmap.cc:217 (unittest+0x1b727e)
    #4 RedisBitmapTest_BitPosSetBit_Test::TestBody() /home/runner/work/incubator-kvrocks/incubator-kvrocks/tests/t_bitmap_test.cc:86 (unittest+0xedaa1)
    #5 void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /home/runner/work/incubator-kvrocks/incubator-kvrocks/build/_deps/gtest-src/googletest/src/gtest.cc:2607 (unittest+0x7ee349)
    #6 void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /home/runner/work/incubator-kvrocks/incubator-kvrocks/build/_deps/gtest-src/googletest/src/gtest.cc:2643 (unittest+0x7ee349)
    #7 __libc_start_main <null> (libc.so.6+0x21c86)
  Location is heap block of size 3264 at 0x7b7c00006200 allocated by main thread:
    #0 operator new(unsigned long) <null> (libtsan.so.0+0x8e62e)
    #1 rocksdb::ColumnFamilyData::ConstructNewMemtable(rocksdb::MutableCFOptions const&, unsigned long) /home/runner/work/incubator-kvrocks/incubator-kvrocks/build/_deps/rocksdb-src/db/column_family.cc:1065 (unittest+0x66cc9d)
    #2 Engine::Storage::Open() /home/runner/work/incubator-kvrocks/incubator-kvrocks/src/storage.cc:323 (unittest+0x306775)
    #3 TestBase::TestBase() /home/runner/work/incubator-kvrocks/incubator-kvrocks/tests/test_base.h:34 (unittest+0xef3c8)
    #4 RedisBitmapTest::RedisBitmapTest() /home/runner/work/incubator-kvrocks/incubator-kvrocks/tests/t_bitmap_test.cc:28 (unittest+0xef3c8)
    #5 RedisBitmapTest_BitPosSetBit_Test::RedisBitmapTest_BitPosSetBit_Test() /home/runner/work/incubator-kvrocks/incubator-kvrocks/tests/t_bitmap_test.cc:82 (unittest+0xef3c8)
    #6 testing::internal::TestFactoryImpl<RedisBitmapTest_BitPosSetBit_Test>::CreateTest() /home/runner/work/incubator-kvrocks/incubator-kvrocks/build/_deps/gtest-src/googletest/include/gtest/internal/gtest-internal.h:472 (unittest+0xef3c8)
    #7 testing::Test* testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::TestFactoryBase, testing::Test*>(testing::internal::TestFactoryBase*, testing::Test* (testing::internal::TestFactoryBase::*)(), char const*) /home/runner/work/incubator-kvrocks/incubator-kvrocks/build/_deps/gtest-src/googletest/src/gtest.cc:2607 (unittest+0x7ee4f9)
    #8 testing::Test* testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::TestFactoryBase, testing::Test*>(testing::internal::TestFactoryBase*, testing::Test* (testing::internal::TestFactoryBase::*)(), char const*) /home/runner/work/incubator-kvrocks/incubator-kvrocks/build/_deps/gtest-src/googletest/src/gtest.cc:2643 (unittest+0x7ee4f9)
    #9 __libc_start_main <null> (libc.so.6+0x21c86)
  Mutex M19766 (0x7b0c000cbc70) created at:
    #0 pthread_mutex_lock <null> (libtsan.so.0+0x532b4)
    #1 __gthread_mutex_lock /usr/include/x86_64-linux-gnu/c++/7/bits/gthr-default.h:748 (unittest+0x1ada7e)
    #2 std::mutex::lock() /usr/include/c++/7/bits/std_mutex.h:103 (unittest+0x1ada7e)
    #3 LockManager::Lock(rocksdb::Slice const&) /home/runner/work/incubator-kvrocks/incubator-kvrocks/src/lock_manager.cc:49 (unittest+0x1ada7e)
    #4 LockGuard::LockGuard(LockManager*, rocksdb::Slice) /home/runner/work/incubator-kvrocks/incubator-kvrocks/src/lock_manager.h:52 (unittest+0x1b698f)
    #5 Redis::Bitmap::SetBit(rocksdb::Slice const&, unsigned int, bool, bool*) /home/runner/work/incubator-kvrocks/incubator-kvrocks/src/redis_bitmap.cc:[169](https://github.com/apache/incubator-kvrocks/runs/6922999327?check_suite_focus=true#step:7:170) (unittest+0x1b698f)
    #6 RedisBitmapTest_BitPosSetBit_Test::TestBody() /home/runner/work/incubator-kvrocks/incubator-kvrocks/tests/t_bitmap_test.cc:86 (unittest+0xedaa1)
    #7 void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /home/runner/work/incubator-kvrocks/incubator-kvrocks/build/_deps/gtest-src/googletest/src/gtest.cc:2607 (unittest+0x7ee349)
    #8 void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /home/runner/work/incubator-kvrocks/incubator-kvrocks/build/_deps/gtest-src/googletest/src/gtest.cc:2643 (unittest+0x7ee349)
    #9 __libc_start_main <null> (libc.so.6+0x21c86)
  Thread T1 'rocksdb:low' (tid=14682, running) created by main thread at:
    #0 pthread_create <null> (libtsan.so.0+0x5fe84)
    #1 std::thread::_M_start_thread(std::unique_ptr<std::thread::_State, std::default_delete<std::thread::_State> >, void (*)()) <null> (unittest+0x83c9d4)
    #2 Engine::Storage::Open(bool) /home/runner/work/incubator-kvrocks/incubator-kvrocks/src/storage.cc:238 (unittest+0x304953)
    #3 Engine::Storage::Open() /home/runner/work/incubator-kvrocks/incubator-kvrocks/src/storage.cc:323 (unittest+0x306775)
    #4 Compact_Filter_Test::TestBody() /home/runner/work/incubator-kvrocks/incubator-kvrocks/tests/compact_test.cc:36 (unittest+0xc57be)
    #5 void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /home/runner/work/incubator-kvrocks/incubator-kvrocks/build/_deps/gtest-src/googletest/src/gtest.cc:2607 (unittest+0x7ee349)
    #6 void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /home/runner/work/incubator-kvrocks/incubator-kvrocks/build/_deps/gtest-src/googletest/src/gtest.cc:2643 (unittest+0x7ee349)
    #7 __libc_start_main <null> (libc.so.6+0x21c86)
SUMMARY: ThreadSanitizer: data race (/usr/lib/x86_64-linux-gnu/libtsan.so.0+0x67cb3) in memcmp

For more details check https://github.com/apache/incubator-kvrocks/runs/6922999327 or reproduce in your side.

Anything Else?

As in https://github.com/apache/incubator-kvrocks/pull/630#discussion_r900909805, we create an issue for this bug to reference easily.

Are you willing to submit a PR?

• [ ] I'm willing to submit a PR!