fgerlits commented on code in PR #2081:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2081#discussion_r2912337850
##########
extensions/rocksdb-repos/database/RocksDbUtils.h:
##########
@@ -49,6 +49,18 @@ class Writable {
}
}
+ template <typename Method, typename... Args>
+ decltype(auto) call(Method method, Args&&... args) {
+ return std::invoke(method, target_, std::forward<Args>(args)...);
+ }
+
+ void optimizeForSmallDb(std::shared_ptr<rocksdb::Cache> cache,
std::shared_ptr<rocksdb::WriteBufferManager> wbm) {
+ if (!cache || !wbm) { return; }
+ target_.OptimizeForSmallDb(&cache);
+ target_.write_buffer_manager = wbm;
+ target_.max_open_files = 20;
Review Comment:
do we want to set `is_modified_` here?
##########
libminifi/test/libtest/unit/TestUtils.cpp:
##########
@@ -260,4 +260,43 @@ std::error_code sendMessagesViaSSL(const
std::vector<std::string_view>& contents
return {};
}
+std::vector<LogMessageView> extractLogMessageViews(const std::string& log_str)
{
+ std::vector<LogMessageView> messages;
+ const std::regex header_pattern(R"((\[[\d\-\s\:\.]+\]) (\s*\[[^\]]+\])
\[(.*)\])");
Review Comment:
minor, but this will pick up garbage for the log level if the payload
contains a `]` character; also I wouldn't include the `[]` characters in the
timestamp and the class name:
```suggestion
const std::regex
header_pattern(R"(\[([\d\-\s\:\.]+)\]\s+\[(.*?)\]\s+\[(.*?)\])");
```
##########
extensions/rocksdb-repos/DatabaseContentRepository.cpp:
##########
@@ -58,21 +58,51 @@ bool DatabaseContentRepository::initialize(const
std::shared_ptr<minifi::Configu
setCompactionPeriod(configuration);
- auto set_db_opts = [encrypted_env]
(minifi::internal::Writable<rocksdb::DBOptions>& db_opts) {
+ const auto cache_size =
configuration->get(Configure::nifi_dbcontent_optimize_for_small_db_cache_size).or_else([]
{ return std::make_optional<std::string>("8 MB"); })
+ | utils::andThen([](const auto& cache_size_str) -> std::optional<uint64_t>
{
+ return parsing::parseDataSize(cache_size_str) | utils::toOptional();
+ });
Review Comment:
This disables the cache whenever the configuration setting cannot be parsed,
if I read it correctly. So not only `""`, but also `"two million bytes"`, `"8
NB"` etc will lead to a silently disabled cache.
I think it would be better to only disable the cache if the configuration is
set to `""` and fail to start up if it is set to an invalid value.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
