MemLens Database Specific Changes by harish876 · Pull Request #170 · apache/incubator-resilientdb

harish876 · 2025-02-12T11:42:38Z

Summary:

Integrated a Stats Class with LevelDB to export internal statistics, including LRU cache hit-and-miss metrics for the Memory Engine.
Added an LRU Cache controlled via a switch to track and report cache hit-and-miss ratios as part of the storage engine metrics on the front end.
Implemented the generation of compile_commands.json to enable autocompletion and IntelliSense during development.
Introduced debug flags during build steps for better profiling and performance tracking.
Fixed a bug related to passing the LevelDB configuration to the KV store.
Created a new endpoint /transaction_data to export specific statistics to the frontend.

Todo:

Bootstrap script like ./INSTALL.sh to install and setup all monitoring tools

…fluxDB.

MemLens - V1 project commits

apratimshukla6

@cjcchen can you review and approve this?

.bazelrc

cjcchen · 2025-02-13T11:01:56Z

chain/storage/BUILD

    name = "leveldb",
    srcs = ["leveldb.cpp"],
    hdrs = ["leveldb.h"],
+    linkopts = ["-pg","-g","-ggdb"],    # Enable profiling during linking


Only open this debug opts if needed.
Most of time, we dont to debug the code.

cjcchen · 2025-02-13T11:03:30Z

chain/storage/leveldb.cpp

    }
  }
+  if ((*config).enable_block_cache()) {
+    LRUCache<std::string, std::string>* block_cache =


When to destroy this pointer?
It looks like there is a memory leak.

You can use unique_pointer instead.

chain/storage/leveldb.cpp

cjcchen · 2025-02-13T11:09:46Z

common/lru/lru_cache.cpp

+    return ValueType();
+  }
+
+  auto it = std::find(dq_.begin(), dq_.end(), key);


performance issue?

cjcchen · 2025-02-13T11:10:10Z

common/lru/lru_cache.cpp

+  }
+
+  auto it = std::find(dq_.begin(), dq_.end(), key);
+  dq_.erase(it);


Suggested change

dq_.erase(it);

if ( it != dq_.end()){

dq_.erase(it);

}

cjcchen · 2025-02-13T11:12:37Z

common/lru/lru_cache.h

+  int cache_hits_;    // Cache hits count
+  int cache_misses_;  // Cache misses count
+
+  deque<KeyType> dq_;  // To maintain most and least recently used items


why not use link-list?

https://en.cppreference.com/w/cpp/container/list

cjcchen · 2025-02-13T11:15:59Z

common/lru/lru_cache.cpp

@@ -0,0 +1,101 @@
+#include "lru_cache.h"


Please add some UTs.

cjcchen · 2025-02-13T11:19:39Z

Hi @harish876,

Thanks for the contribution and it is a nice accomplishment.
I left some comments. Please take a look.

Thanks.

harish876 · 2025-02-13T18:16:10Z

Hi @cjcchen
Thanks for the comments. I will go through them and resolve them.

…tation var names

cjcchen · 2025-02-21T22:53:42Z

platform/statistic/stats.cpp

  }
 }

+long getRSS() {


Suggested change

long getRSS() {

long GetRSS() {

cjcchen · 2025-02-21T22:55:12Z

platform/statistic/stats.cpp

+    return 0;
+  }
+
+  unsigned long size, resident, share, text, lib, data, dt;


let's use int64_t and uint64_t other than long and unsigned long.

platform/statistic/stats.cpp

cjcchen · 2025-02-21T23:03:40Z

platform/statistic/stats.h

  std::chrono::system_clock::time_point execution_time;
+
+  // Storage Engine Stats
+  double ext_cache_hit_ratio;


some naming issues. using "_" as the subfix.

cjcchen · 2025-02-21T23:43:01Z

Hi @cjcchen , I have made the necessary changes pointed out by you. Can you please review them?

Very nice job.
Left a few comments.

harish876 · 2025-02-27T23:28:54Z

Thanks @cjcchen , will resolve the additional comments

…ng in ResLevelDB

…te metrics return type

harish876 · 2025-03-07T08:42:07Z

Hi @cjcchen tried to resolve the comments. please do review once and let me know if further changes need to be made. thanks

cjcchen · 2025-03-07T15:42:28Z

Hi @cjcchen tried to resolve the comments. please do review once and let me know if further changes need to be made. thanks

Very good job.

One question here, what is the motivation to remove the O3 option?
Do we need to always open this feature?

If this is optional to just for the profile tracing, we can add an introduction on how to enable the feature.

harish876 · 2025-03-07T19:28:05Z

yes @cjcchen . it was a mistake. corrected it now

cjcchen · 2025-03-08T01:09:54Z

service/kv/BUILD

-        "//chain/storage/setting:enable_leveldb_setting": ["-DENABLE_LEVELDB"],
-        "//conditions:default": [],
+        "//chain/storage/setting:enable_leveldb_setting": ["-DENABLE_LEVELDB","-g","-ggdb"],
+        "//conditions:default": ["-pg", "-g", "-ggdb"],


How about this -pg -ggdb?

cjcchen · 2025-03-08T01:10:32Z

service/tools/kv/api_tools/BUILD

+    copts = ["-pg"],
+    linkopts = ["-pg"]


do we need this?

cjcchen · 2025-03-08T01:11:15Z

yes @cjcchen . it was a mistake. corrected it now

Great.
How about the "pg" in the BUILD files?

harish876 · 2025-03-14T07:44:03Z

Hi @cjcchen. I have removed the debug flags. Our profiler only works when the build has debug information at the top level. I am unsure if profiling would work without the -pg flag. We do get kernel level metrics without the debug flag but userspace metrics are not available without that.

Please do let us know what could be done here?

cjcchen · 2025-03-14T08:45:32Z

Hi @cjcchen. I have removed the debug flags. Our profiler only works when the build has debug information at the top level. I am unsure if profiling would work without the -pg flag. We do get kernel level metrics without the debug flag but userspace metrics are not available without that.

Please do let us know what could be done here?

Very cool!
Merged.

cjcchen · 2025-03-14T10:28:43Z

Hi @cjcchen. I have removed the debug flags. Our profiler only works when the build has debug information at the top level. I am unsure if profiling would work without the -pg flag. We do get kernel level metrics without the debug flag but userspace metrics are not available without that.

Please do let us know what could be done here?

@harish876 did it build successfully on your end?

https://github.com/apache/incubator-resilientdb/actions/runs/13852115218/job/38763569382

harish876 · 2025-03-14T10:42:18Z

Yes @cjcchen , it did build with warnings albeit, but all TC's were a go. I will go through the failed checks.

harish876 · 2025-03-15T08:54:20Z

Hi @cjcchen , verified the build and unit tests again. I removed our profiling command, which needs to be installed. I will convert our setup to a sidecar. Please find my deployment results. I also have enabled LevelDB here, please do let me know if that is fine by you

cjcchen · 2025-03-15T11:15:56Z

Hi @cjcchen , verified the build and unit tests again. I removed our profiling command, which needs to be installed. I will convert our setup to a sidecar. Please find my deployment results. I also have enabled LevelDB here, please do let me know if that is fine by you

"common/lru/lru_cache.cpp:21:1: note: 'std::string' is defined in header ''; did you forget to '#include '?
"

it reports this. I think you just need to include to the header.

harish876 · 2025-03-15T12:06:52Z

Done @cjcchen

harish876 added 14 commits November 15, 2024 22:05

global and local build config with profiling flags.

3f963bb

integrated basic memory hook for RSS calculation

804dcd1

edited script to generate callgraph using gprof

b67fa88

enabling level db as default storage engine

4404af5

systemd telegraf script to stream prometheus node_exporter data to In…

33c16f2

…fluxDB.

added lmdb storage engine interface and partially implemented.

7c35fe8

bootstrapping script to run perf tools

ccade23

adding process exporter to bootstrapping script

c625f78

Added LRU Cache and Observabiity stats

e1ce45d

Merge pull request #1 from harish876/memory-hooks-integration

676a5d3

MemLens - V1 project commits

lry cache controlled using settings

491b254

Merge branch 'apache:master' into master

4adc82f

removing lmdb implementation

7eb9fff

removing unecessary scripts

d8cbe16

apratimshukla6 reviewed Feb 13, 2025

View reviewed changes

cjcchen reviewed Feb 13, 2025

View reviewed changes

.bazelrc Show resolved Hide resolved

cjcchen reviewed Feb 13, 2025

View reviewed changes

chain/storage/leveldb.cpp Show resolved Hide resolved

cjcchen reviewed Feb 13, 2025

View reviewed changes

common/lru/lru_cache.cpp

@@ -0,0 +1,101 @@

#include "lru_cache.h"

Copy link
Copy Markdown

Contributor

cjcchen Feb 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add some UTs.

harish876 added 5 commits February 17, 2025 04:01

Resolving comments for build flags and memory leak

71975f2

resolved error. options.block_cache_ integration still not working

35ed786

added UT's for TC's. removed logs from stats and changed lru implemen…

b9b3363

…tation var names

reverting configs and removing unwanted files

0d02a14

removing unwanted tst logs

8348689

cjcchen reviewed Feb 21, 2025

View reviewed changes

platform/statistic/stats.cpp Outdated

}

}

long getRSS() {

Copy link
Copy Markdown

Contributor

cjcchen Feb 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

long getRSS() {

long GetRSS() {

cjcchen reviewed Feb 21, 2025

View reviewed changes

platform/statistic/stats.cpp Show resolved Hide resolved

cjcchen reviewed Feb 21, 2025

View reviewed changes

harish876 added 4 commits March 7, 2025 03:35

refactor: rename GetMetrics to UpdateMetrics and improve cache handli…

0a85b15

…ng in ResLevelDB

feat: add LevelDB block cache support and corresponding tests.

ccb268d

feat: enhance LevelDB block cache with configurable capacity and upda…

4cd5b88

…te metrics return type

refactor: update transaction summary variable names for consistency

a642ef8

build: optimize compilation flags by adding optimization level

08203e3

cjcchen reviewed Mar 8, 2025

View reviewed changes

build: remove debugging flags from kv_service and api_tools builds

94d2a72

build: remove profiling flags from set_random_data binary

a759dda

cjcchen approved these changes Mar 14, 2025

View reviewed changes

build: remove pyroscope monitoring from kv service startup script

a0a136f

build: include string header in lru_cache implementation

32b5758

cjcchen merged commit d34e9f5 into apache:master Mar 20, 2025
2 checks passed

Conversation

harish876 commented Feb 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary:

Todo:

Uh oh!

apratimshukla6 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cjcchen commented Feb 13, 2025

Uh oh!

harish876 commented Feb 13, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cjcchen commented Feb 21, 2025

Uh oh!

harish876 commented Feb 27, 2025

Uh oh!

harish876 commented Mar 7, 2025

Uh oh!

cjcchen commented Mar 7, 2025

Uh oh!

harish876 commented Mar 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cjcchen commented Mar 8, 2025

Uh oh!

harish876 commented Mar 14, 2025

Uh oh!

cjcchen commented Mar 14, 2025

Uh oh!

cjcchen commented Mar 14, 2025

Uh oh!

harish876 commented Mar 14, 2025

Uh oh!

harish876 commented Mar 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cjcchen commented Mar 15, 2025

Uh oh!

harish876 commented Mar 15, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

harish876 commented Feb 12, 2025 •

edited

Loading

harish876 commented Mar 15, 2025 •

edited

Loading