feature(db): optimize properties query frequency #5378

CarlChaoCarl · 2023-07-28T06:27:00Z

…into feature/optimize_properties_query_frequency_v6

tomatoishealthy · 2023-08-01T06:38:06Z

chainbase/src/main/java/org/tron/core/db2/core/SnapshotImpl.java

+    isOptimized = snapshot.isOptimized();
+    if (isOptimized) {
+      if (root == previous) {
+        Streams.stream(root.iterator()).forEach( e -> put(e.getKey(),e.getValue()));


reformat the code style, put the white space to the right place

These two if statements can be combined

tomatoishealthy · 2023-08-02T10:10:47Z

chainbase/src/main/java/org/tron/core/db2/core/SnapshotImpl.java

+      }
+    }
+     */
+


Why reserve these if they are useless?

I removed this part of the commented code

tomatoishealthy · 2023-08-02T10:16:01Z

chainbase/src/main/java/org/tron/core/db2/core/SnapshotImpl.java

+    SnapshotImpl fromImpl = (SnapshotImpl) from;
+    Streams.stream(fromImpl.db).forEach(e ->
+        {
+          if (db.get(e.getKey()) == null && e.getValue() != null ) {


code format

tomatoishealthy · 2023-08-03T04:02:30Z

chainbase/src/main/java/org/tron/core/db2/core/SnapshotImpl.java

@@ -83,6 +99,20 @@ public void merge(Snapshot from) {
    Streams.stream(fromImpl.db).forEach(e -> db.put(e.getKey(), e.getValue()));
  }

+  public void mergeFullData(Snapshot from) {


As I see, the merge method is used for merging the snapshotImpl which is behind itself, this method mergeFullData is used for merging the snapshotImpl which is ahead, and only one snapshot. it only can merge all data by another outside iteration.

So renaming the method name to mergeAhead or mergeBefore or mergeFront may be better?

OK, I renamed the mergeFullData function to mergeAhead

tomatoishealthy · 2023-08-03T04:10:55Z

chainbase/src/main/java/org/tron/core/db2/core/SnapshotManager.java

@@ -233,6 +233,12 @@ public synchronized void commit() {
    }

    --activeSession;
+
+    dbs.forEach(db -> {


Why set the reload logic in commit(), commit() only invoked by the end of pushing a block, not a transaction.

Are there other reasons for doing this, can you explain?

This is because if the transaction ends without a commit,
Revoke is needed at this time,
session.close is called,
Then call snapshotManager.revoke,
Then there is no need to perform reload logic.
So the reload logic is only executed when the transaction commits.

@tomatoishealthy @halibobo1205

tomatoishealthy · 2023-08-03T06:07:37Z

Any issue associated with this PR? So we can have a comprehensive background

CarlChaoCarl · 2023-08-03T09:30:02Z

Any issue associated with this PR? So we can have a comprehensive background

This Issue is associated with the current pr
#5375

halibobo1205 · 2023-08-09T08:26:45Z

@CarlChaoCarl Please fix CI.

…_properties_query_frequency_v6

CarlChaoCarl · 2023-08-09T09:45:59Z

@CarlChaoCarl Please fix CI.

OK, I have solved the CI problem

halibobo1205 · 2023-08-10T10:33:55Z

chainbase/src/main/java/org/tron/core/db2/core/SnapshotImpl.java

+    }
+    SnapshotImpl fromImpl = (SnapshotImpl) from;
+    Streams.stream(fromImpl.db).forEach(e -> {
+      if (db.get(e.getKey()) == null && e.getValue() != null) {


How would happen for e.getValue() == null?

It is necessary to count which keys are null and whether the access frequency is high

db.get(e.getKey()) == null && e.getValue() != null
Passing these two judgments means:

The current db does not have this key, that is, db.get(e.getKey()) is null,

At the same time, the snapshotimpl from has this key and the value is not empty, that is, e.getValue() != null.

@wubin01 @halibobo1205

How would happen for e.getValue() == null?

@halibobo1205 is it possible that key exists but value is null in the property database?

@yuekun0707 , It shouldn't be null.

…query_frequency_v6

codecov-commenter · 2023-08-14T04:43:12Z

Codecov Report

Merging #5378 (0f86735) into develop (ad728fa) will increase coverage by 0.00%.
The diff coverage is 95.00%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the GitHub App Integration for your organization. Read more.

@@            Coverage Diff             @@
##             develop    #5378   +/-   ##
==========================================
  Coverage      61.35%   61.35%           
- Complexity      9348     9352    +4     
==========================================
  Files            846      846           
  Lines          50211    50231   +20     
  Branches        5583     5584    +1     
==========================================
+ Hits           30805    30818   +13     
+ Misses         17002    17001    -1     
- Partials        2404     2412    +8

Files Changed	Coverage Δ
...src/main/java/org/tron/core/db2/core/Snapshot.java	`0.00% <ø> (ø)`
...main/java/org/tron/core/db2/core/SnapshotRoot.java	`73.87% <50.00%> (-0.44%)`	⬇️
.../java/org/tron/core/db2/core/AbstractSnapshot.java	`90.00% <100.00%> (+1.11%)`	⬆️
...main/java/org/tron/core/db2/core/SnapshotImpl.java	`91.56% <100.00%> (+1.56%)`	⬆️
...n/java/org/tron/core/db2/core/SnapshotManager.java	`74.58% <100.00%> (+0.85%)`	⬆️

... and 11 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

xxo1shine · 2023-08-02T03:39:05Z

chainbase/src/main/java/org/tron/core/db2/core/SnapshotImpl.java

+    SnapshotImpl fromImpl = (SnapshotImpl) from;
+    Streams.stream(fromImpl.db).forEach(e ->
+        {
+          if (db.get(e.getKey()) == null && e.getValue() != null ) {


Will there be a frequently accessed key with a value of null？

db.get(e.getKey()) == null && e.getValue() != null
Passing these two judgments means:

The current db does not have this key, that is, db.get(e.getKey()) is null,

At the same time, the snapshotimpl from has this key and the value is not empty, that is, e.getValue() != null.

@wubin01

xxo1shine · 2023-08-09T08:44:51Z

chainbase/src/main/java/org/tron/core/db2/core/SnapshotImpl.java

+    isOptimized = snapshot.isOptimized();
+    if (isOptimized) {
+      if (root == previous) {
+        Streams.stream(root.iterator()).forEach( e -> put(e.getKey(),e.getValue()));


These two if statements can be combined

xxo1shine · 2023-08-09T08:49:24Z

chainbase/src/main/java/org/tron/core/db2/core/SnapshotManager.java

+
+    dbs.forEach(db -> {
+      if (db.getHead().isOptimized()) {
+        db.getHead().reloadToMem();


isOptimized judged twice

OK,I get it

I have fixed this

xxo1shine · 2023-08-14T08:58:20Z

chainbase/src/main/java/org/tron/core/db2/core/SnapshotImpl.java

+    }
+    SnapshotImpl fromImpl = (SnapshotImpl) from;
+    Streams.stream(fromImpl.db).forEach(e -> {
+      if (db.get(e.getKey()) == null && e.getValue() != null) {


It is necessary to count which keys are null and whether the access frequency is high

tomatoishealthy · 2023-08-30T05:41:41Z

Better to add the unit test for these changes

…query_frequency_v6

…s://github.com/CarlChaoCarl/java-tron into feature/optimize_properties_query_frequency_v6

CarlChaoCarl · 2023-09-01T05:00:37Z

Better to add the unit test for these changes

@tomatoishealthy Yeah, I have added unit tests.

chaozhu added 3 commits May 30, 2023 12:05

feature(mechanism): optimize properties query frequency

1999fe2

feature(mechanism): optimize properties query frequency

79ca721

Merge branch 'develop' of https://github.com/chaosuper12321/java-tron …

5201ddb

…into feature/optimize_properties_query_frequency_v6

CarlChaoCarl mentioned this pull request Jul 28, 2023

Improve Query Performance of Property Store #5375

Closed

lvs007 requested review from tomatoishealthy, xxo1shine and halibobo1205 August 1, 2023 06:20

tomatoishealthy reviewed Aug 3, 2023

View reviewed changes

halibobo1205 changed the title ~~feature(mechanism): optimize properties query frequency~~ feature(db): optimize properties query frequency Aug 3, 2023

halibobo1205 added this to the GreatVoyage-v4.7.3 milestone Aug 9, 2023

chaozhu added 2 commits August 9, 2023 17:17

Merge remote-tracking branch 'upstream/develop' into feature/optimize…

59c5fe8

…_properties_query_frequency_v6

feature(mechanism): optimize properties query frequency

dced678

CarlChaoCarl closed this Aug 9, 2023

CarlChaoCarl reopened this Aug 9, 2023

halibobo1205 reviewed Aug 10, 2023

View reviewed changes

halibobo1205 assigned CarlChaoCarl Aug 10, 2023

Merge branch 'tronprotocol:develop' into feature/optimize_properties_…

a66f807

…query_frequency_v6

xxo1shine reviewed Aug 14, 2023

View reviewed changes

chaozhu and others added 2 commits August 30, 2023 15:18

feature(mechanism): optimize properties query frequency

3b40771

Merge branch 'tronprotocol:develop' into feature/optimize_properties_…

1ae4a53

…query_frequency_v6

xxo1shine approved these changes Aug 30, 2023

View reviewed changes

feature(mechanism): optimize properties query frequency

2ca5129

halibobo1205 approved these changes Aug 30, 2023

View reviewed changes

Merge branch 'tronprotocol:develop' into feature/optimize_properties_…

180b26f

…query_frequency_v6

chaozhu and others added 4 commits August 31, 2023 20:25

feature(mechanism): add unit test for properties db optimization

a8a7492

Merge branch 'tronprotocol:develop' into feature/optimize_properties_…

cc053b7

…query_frequency_v6

feature(mechanism): add unit test for properties db optimization

9430e13

Merge branch 'feature/optimize_properties_query_frequency_v6' of http…

0f86735

…s://github.com/CarlChaoCarl/java-tron into feature/optimize_properties_query_frequency_v6

tomatoishealthy approved these changes Sep 1, 2023

View reviewed changes

yuekun0707 approved these changes Sep 1, 2023

View reviewed changes

halibobo1205 merged commit 068764b into tronprotocol:develop Sep 4, 2023
7 checks passed

halibobo1205 added Improvement area:db status:delivery test labels Oct 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature(db): optimize properties query frequency #5378

feature(db): optimize properties query frequency #5378

CarlChaoCarl commented Jul 28, 2023 •

edited by halibobo1205

Loading

tomatoishealthy Aug 1, 2023

xxo1shine Aug 9, 2023

CarlChaoCarl Aug 9, 2023

tomatoishealthy Aug 2, 2023

CarlChaoCarl Aug 9, 2023

tomatoishealthy Aug 2, 2023

CarlChaoCarl Aug 9, 2023

tomatoishealthy Aug 3, 2023

CarlChaoCarl Aug 9, 2023

tomatoishealthy Aug 3, 2023

halibobo1205 Aug 7, 2023

CarlChaoCarl Aug 9, 2023

tomatoishealthy commented Aug 3, 2023

CarlChaoCarl commented Aug 3, 2023

halibobo1205 commented Aug 9, 2023

CarlChaoCarl commented Aug 9, 2023

halibobo1205 Aug 10, 2023 •

edited

Loading

xxo1shine Aug 14, 2023

CarlChaoCarl Aug 14, 2023

yuekun0707 Aug 17, 2023

halibobo1205 Aug 24, 2023

codecov-commenter commented Aug 14, 2023 •

edited

Loading

xxo1shine Aug 2, 2023

CarlChaoCarl Aug 14, 2023

xxo1shine Aug 9, 2023

xxo1shine Aug 9, 2023

CarlChaoCarl Aug 14, 2023

CarlChaoCarl Aug 30, 2023

xxo1shine Aug 14, 2023

tomatoishealthy commented Aug 30, 2023

CarlChaoCarl commented Sep 1, 2023

feature(db): optimize properties query frequency #5378

feature(db): optimize properties query frequency #5378

Conversation

CarlChaoCarl commented Jul 28, 2023 • edited by halibobo1205 Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomatoishealthy commented Aug 3, 2023

CarlChaoCarl commented Aug 3, 2023

halibobo1205 commented Aug 9, 2023

CarlChaoCarl commented Aug 9, 2023

halibobo1205 Aug 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Aug 14, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomatoishealthy commented Aug 30, 2023

CarlChaoCarl commented Sep 1, 2023

CarlChaoCarl commented Jul 28, 2023 •

edited by halibobo1205

Loading

halibobo1205 Aug 10, 2023 •

edited

Loading

codecov-commenter commented Aug 14, 2023 •

edited

Loading