szaszm commented on code in PR #1490:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1490#discussion_r1093453991


##########
extensions/rocksdb-repos/database/OpenRocksDb.cpp:
##########
@@ -118,8 +114,18 @@ rocksdb::DB* OpenRocksDb::get() {
   return impl_.get();
 }
 
-}  // namespace internal
-}  // namespace minifi
-}  // namespace nifi
-}  // namespace apache
-}  // namespace org
+std::optional<uint64_t> OpenRocksDb::getApproximateSizes() const {
+  rocksdb::SizeApproximationOptions options;
+  options.include_memtabtles = true;
+  std::array<rocksdb::Range, 1> ranges;
+  ranges[0].start = "";
+  ranges[0].limit = "~";
+  uint64_t value = 0;
+  auto status = impl_->GetApproximateSizes(options, column_->handle.get(), 
ranges.data(), 1, &value);

Review Comment:
   ```suggestion
     const rocksdb::SizeApproximationOptions options{ .include_memtabtles = 
true };
     const rocksdb::Range range{ .start = "", .limit = "~" };
     uint64_t value = 0;
     auto status = impl_->GetApproximateSizes(options, column_->handle.get(), 
&range, 1, &value);
   ```
   
   Btw, is that typo in the options part of the rocksdb API?



##########
libminifi/include/core/Repository.h:
##########
@@ -54,28 +54,22 @@ constexpr auto MAX_REPOSITORY_STORAGE_SIZE = 10_MiB;
 constexpr auto MAX_REPOSITORY_ENTRY_LIFE_TIME = std::chrono::minutes(10);
 constexpr auto REPOSITORY_PURGE_PERIOD = std::chrono::milliseconds(2500);
 
-class Repository : public core::CoreComponent {
+class Repository : public core::ReportableRepository {

Review Comment:
   I feel like this relationship should be modeled the other way around: A 
repository is not necessarily "reportable", but a "reportable" repository is 
definitely a repository.
   
   It would be even better if "reportability" was implemented in terms of an 
independent interface used to export certain metrics.
   
   An interitance relationship results in a very tight coupling of the involved 
classes, that is hard to change later, so we need to get this right. What do 
you think?



-- 
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]

Reply via email to