-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
use bloom filter, add event listener to collect stats of compaction, improve update rocksdb options #1959
use bloom filter, add event listener to collect stats of compaction, improve update rocksdb options #1959
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
/* Copyright (c) 2020 vesoft inc. All rights reserved. | ||
* | ||
* This source code is licensed under Apache 2.0 License, | ||
* attached with Common Clause Condition 1.0, found in the LICENSES directory. | ||
*/ | ||
|
||
#include "base/Base.h" | ||
#include "rocksdb/db.h" | ||
#include "rocksdb/listener.h" | ||
|
||
namespace nebula { | ||
namespace kvstore { | ||
|
||
class EventListener : public rocksdb::EventListener { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good Job |
||
public: | ||
void OnCompactionCompleted(rocksdb::DB*, const rocksdb::CompactionJobInfo& info) override { | ||
LOG(INFO) << "Rocksdb compact column family: " << info.cf_name | ||
<< " because of " << static_cast<int32_t>(info.compaction_reason) | ||
<< ", status: " << info.status.ToString() | ||
<< ", compacted " << info.input_files.size() | ||
<< " files into " << info.output_files.size() | ||
<< ", base level is " << info.base_input_level | ||
<< ", output level is " << info.output_level; | ||
} | ||
}; | ||
|
||
} // namespace kvstore | ||
} // namespace nebula |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -96,25 +96,19 @@ void MetaServerBasedPartManager::onSpaceOptionUpdated( | |
"disable_auto_compactions", | ||
"max_write_buffer_number", | ||
// TODO: write_buffer_size will cause rocksdb crash | ||
// "write_buffer_size", | ||
"compression", | ||
"write_buffer_size", | ||
"level0_file_num_compaction_trigger", | ||
"level0_slowdown_writes_trigger", | ||
"level0_stop_writes_trigger", | ||
"target_file_size_base", | ||
"target_file_size_multiplier", | ||
"max_bytes_for_level_base", | ||
"max_bytes_for_level_multiplier", | ||
"ttl", | ||
"block_size", | ||
"block_restart_interval" | ||
}; | ||
static std::unordered_set<std::string> supportedDbOpt = { | ||
"max_total_wal_size", | ||
"delete_obsolete_files_period_micros", | ||
"max_background_jobs", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. support max_subcompactions There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it can't be changed dynamically, for those options, it would take effect when reboot. |
||
"base_background_compactions", | ||
"max_background_compactions", | ||
"stats_dump_period_sec", | ||
"compaction_readahead_size", | ||
"writable_file_max_buffer_size", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,11 +6,13 @@ | |
|
||
#include "base/Base.h" | ||
#include "kvstore/RocksEngineConfig.h" | ||
#include "kvstore/EventListner.h" | ||
#include "rocksdb/db.h" | ||
#include "rocksdb/cache.h" | ||
#include "rocksdb/convenience.h" | ||
#include "rocksdb/utilities/options_util.h" | ||
#include "rocksdb/slice_transform.h" | ||
#include "rocksdb/filter_policy.h" | ||
#include "base/Configuration.h" | ||
|
||
// [WAL] | ||
|
@@ -44,6 +46,7 @@ DEFINE_int32(rocksdb_batch_size, | |
DEFINE_int64(rocksdb_block_cache, 1024, | ||
"The default block cache size used in BlockBasedTable. The unit is MB"); | ||
|
||
DEFINE_bool(enable_partitioned_index_filter, false, "True for partitioned index filters"); | ||
|
||
namespace nebula { | ||
namespace kvstore { | ||
|
@@ -62,6 +65,7 @@ rocksdb::Status initRocksdbOptions(rocksdb::Options &baseOpts) { | |
if (!s.ok()) { | ||
return s; | ||
} | ||
dbOpts.listeners.emplace_back(new EventListener()); | ||
|
||
std::unordered_map<std::string, std::string> cfOptsMap; | ||
if (!loadOptionsMap(cfOptsMap, FLAGS_rocksdb_column_family_options)) { | ||
|
@@ -87,6 +91,15 @@ rocksdb::Status initRocksdbOptions(rocksdb::Options &baseOpts) { | |
static std::shared_ptr<rocksdb::Cache> blockCache | ||
= rocksdb::NewLRUCache(FLAGS_rocksdb_block_cache * 1024 * 1024); | ||
bbtOpts.block_cache = blockCache; | ||
bbtOpts.filter_policy.reset(rocksdb::NewBloomFilterPolicy(10, false)); | ||
if (FLAGS_enable_partitioned_index_filter) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how about add prefix bloom filter? for the prefix 12bytes There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good suggestion. Later I will check if it improves perf. |
||
bbtOpts.index_type = rocksdb::BlockBasedTableOptions::IndexType::kTwoLevelIndexSearch; | ||
bbtOpts.partition_filters = true; | ||
bbtOpts.cache_index_and_filter_blocks = true; | ||
bbtOpts.cache_index_and_filter_blocks_with_high_priority = true; | ||
bbtOpts.pin_l0_filter_and_index_blocks_in_cache = | ||
baseOpts.compaction_style == rocksdb::CompactionStyle::kCompactionStyleLevel; | ||
} | ||
baseOpts.table_factory.reset(NewBlockBasedTableFactory(bbtOpts)); | ||
baseOpts.create_if_missing = true; | ||
return s; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,7 +28,7 @@ DEFINE_uint64(raft_snapshot_timeout, 60 * 5, "Max seconds between two snapshot r | |
|
||
DEFINE_uint32(max_batch_size, 256, "The max number of logs in a batch"); | ||
|
||
DEFINE_int32(wal_ttl, 86400, "Default wal ttl"); | ||
DEFINE_int32(wal_ttl, 14400, "Default wal ttl"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. current is 4 hour ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Current is 1 day, @whitewum suggest change it shorter. |
||
DEFINE_int64(wal_file_size, 16 * 1024 * 1024, "Default wal file size"); | ||
DEFINE_int32(wal_buffer_size, 8 * 1024 * 1024, "Default wal buffer size"); | ||
DEFINE_int32(wal_buffer_num, 2, "Default wal buffer number"); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,7 +14,7 @@ std::unordered_set<std::string> SetConfigProcessor::mutableFields_ = { | |
// rocksdb_column_family_options | ||
"disable_auto_compactions", | ||
// TODO: write_buffer_size will cause rocksdb crash | ||
// "write_buffer_size", | ||
"write_buffer_size", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "write_buffer_size" works well now ? if yes , do we need to uncomment in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Sorry, the comment has been removed. I'm wrong. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Forgot to remove the TODO : ) |
||
"max_write_buffer_number", | ||
"level0_file_num_compaction_trigger", | ||
"level0_slowdown_writes_trigger", | ||
|
@@ -23,14 +23,11 @@ std::unordered_set<std::string> SetConfigProcessor::mutableFields_ = { | |
"target_file_size_multiplier", | ||
"max_bytes_for_level_base", | ||
"max_bytes_for_level_multiplier", | ||
"ttl", | ||
|
||
// rocksdb_db_options | ||
"max_total_wal_size", | ||
"delete_obsolete_files_period_micros", | ||
"max_background_jobs", | ||
"base_background_compactions", | ||
"max_background_compactions", | ||
"stats_dump_period_sec", | ||
"compaction_readahead_size", | ||
"writable_file_max_buffer_size", | ||
|
@@ -141,9 +138,6 @@ cpp2::ErrorCode SetConfigProcessor::setNestedConfig(const cpp2::ConfigModule& mo | |
} | ||
|
||
Configuration conf; | ||
auto confRet = conf.parseFromString(item.get_value()); | ||
CHECK(confRet.ok()); | ||
|
||
std::vector<std::string> updateFields; | ||
folly::split(",", updateList, updateFields, true); | ||
bool updated = false; | ||
|
@@ -156,11 +150,12 @@ cpp2::ErrorCode SetConfigProcessor::setNestedConfig(const cpp2::ConfigModule& mo | |
auto key = field.substr(0, pos); | ||
auto value = field.substr(pos + 1); | ||
// TODO: Maybe need to handle illegal value here | ||
if (mutableFields_.count(key) && conf.updateStringField(key.c_str(), value).ok()) { | ||
if (!conf.upsertStringField(key.c_str(), value).ok()) { | ||
LOG(ERROR) << "Update configs failed: " << key; | ||
return cpp2::ErrorCode::E_UNSUPPORTED; | ||
} | ||
if (mutableFields_.count(key)) { | ||
updated = true; | ||
} else { | ||
LOG(ERROR) << "Unsupported configs " << key; | ||
return cpp2::ErrorCode::E_NOT_FOUND; | ||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A question, Why naming
upsert
?What I understand that upsert is : should be override the old value if key exists; should be insert a new value if the key doesn't exist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly, so we can insert some option which doesn't exists, and update some option with a new value. The function actually is a upsert, a bad name.