feat(db): optimize old rewards withdrawal #5406

halibobo1205 · 2023-08-09T04:11:17Z

close #5354

halibobo1205 · 2023-08-09T07:04:05Z

@wangzichichi PTAL, https://buildkite.com/tronprotocol/build-on-centos-linux-release-7-dot-4-1708-core/builds/5978

tomatoishealthy · 2023-08-11T05:50:59Z

framework/src/main/java/org/tron/core/db/common/iterator/AbstractIterator.java

Are these removals part of the optimization reward calculation? If not, better fix it in another PR, just suggestion

There's a class conflict.

tomatoishealthy · 2023-08-11T09:45:29Z

chainbase/src/main/java/org/tron/core/service/RewardCalService.java

+    Metrics.histogramObserve(requestTimer);
+  }
+
+  private Protocol.Account getAccount(byte[] address) {


Is it possible to get the account using accountStore.getFromRoot() directly？

DB Cache pollution due to bulk queries, like accountStore.getFromRoot()

tomatoishealthy · 2023-08-11T10:32:26Z

chainbase/src/main/java/org/tron/core/db/common/iterator/DBIterator.java

+  default UnmodifiableIterator<Entry<byte[], byte[]>> prefixQueryAfterThat
+      (byte[] key, byte[] afterThat) {
+    this.seek(afterThat == null ? key : afterThat);
+    return Iterators.filter(this, entry -> Bytes.indexOf(entry.getKey(), key) == 0);


Here I have doubt, whether this iterator will still access all data in the database but just return the data that matched the filter.
If so, it will do a lot of useless read-op?

Not All, iterates over all data after the prefix. I can't think of a better way for now. It takes 2 minutes to iterate the database.

Use the default iterator and move the filter logic into startRewardCal, or re-implement a prefix iterator.
Seems 2 minutes is not a big deal, depends on you

Prefix does not work is fixed.

halibobo1205 · 2023-08-11T14:30:40Z

   System.out.println((long) 128.9999999999999857891452847979962825775146484375); // will print 129
   System.out.println((long) 128.9999999999999857891452847979962825775146484374999999999999999999); // will print 128

halibobo1205 · 2023-08-30T03:04:25Z

chainbase/src/main/java/org/tron/core/service/MortgageService.java

+
+  private long getOldReward(long beginCycle, long oldEndCycle, AccountCapsule accountCapsule) {
+    long cacheData = rewardCalService.getReward(accountCapsule.createDbKey(), beginCycle);
+    long reward = 0;


Phase I only calculates without using, cache enabled for the next release.

xxo1shine · 2023-08-29T04:53:35Z

chainbase/src/main/java/org/tron/core/service/RewardCalService.java

+    return null;
+  }
+
+  long computeReward(long cycle, Protocol.Account account) {


Why rewrite this method instead of calling the original method?

Avoid circular dependencies.

codecov-commenter · 2023-08-30T03:36:25Z

Codecov Report

Merging #5406 (00e349b) into develop (f9c42d6) will increase coverage by 0.08%.
Report is 1 commits behind head on develop.
The diff coverage is 75.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    #5406      +/-   ##
=============================================
+ Coverage      61.23%   61.32%   +0.08%     
- Complexity      9323     9374      +51     
=============================================
  Files            842      841       -1     
  Lines          50147    50275     +128     
  Branches        5580     5591      +11     
=============================================
+ Hits           30710    30831     +121     
+ Misses         17029    17028       -1     
- Partials        2408     2416       +8

Files Changed	Coverage Δ
...in/java/org/tron/common/prometheus/MetricKeys.java	`0.00% <ø> (ø)`
...a/org/tron/core/db/common/iterator/DBIterator.java	`40.00% <40.00%> (ø)`
...in/java/org/tron/core/service/MortgageService.java	`63.94% <60.00%> (-2.94%)`	⬇️
...n/java/org/tron/core/service/RewardCalService.java	`68.13% <68.13%> (ø)`
...ron/core/db/common/iterator/RockStoreIterator.java	`78.94% <80.00%> (+5.41%)`	⬆️
...ain/java/org/tron/core/store/RewardCacheStore.java	`85.71% <85.71%> (ø)`
...rg/tron/core/db/common/iterator/StoreIterator.java	`91.66% <96.55%> (+3.66%)`	⬆️
...a/org/tron/common/prometheus/MetricsHistogram.java	`83.33% <100.00%> (+0.35%)`	⬆️

... and 8 files with indirect coverage changes

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

tomatoishealthy · 2023-08-31T03:47:01Z

chainbase/src/main/java/org/tron/core/db/common/iterator/StoreIterator.java

@@ -13,23 +14,22 @@ public final class StoreIterator implements org.tron.core.db.common.iterator.DBI
  private final DBIterator dbIterator;
  private boolean first = true;


Is there a simpler way by just setting first false when invoking seek() & seekToLast() to solve the problem of pointer confusion?

Yes, that's exactly how it's handled.

…31 20:00:00

jwrct · 2024-01-04T03:36:32Z

chainbase/src/main/java/org/tron/core/store/DynamicPropertiesStore.java

@@ -2523,6 +2525,10 @@ public boolean useNewRewardAlgorithm() {
    return getNewRewardAlgorithmEffectiveCycle() != Long.MAX_VALUE;
  }

+  public boolean useNewRewardAlgorithmFromStart() {
+    return getNewRewardAlgorithmEffectiveCycle() == 1;


Can this value possibly equal 1, even on a new chain?

Yes, It can be turned on anytime.

jwrct · 2024-01-04T10:44:24Z

chainbase/src/main/java/org/tron/core/service/RewardViCalService.java

+      try {
+        lock.await();
+      } catch (InterruptedException e) {
+        logger.error("rewardViCalService lock error: {}", e.getMessage());


If an InterruptedException occurs, the program will continue to execute, but with an additional error log. Is this the expected behavior?

tomatoishealthy · 2024-01-05T06:36:13Z

chainbase/src/main/java/org/tron/core/service/RewardViCalService.java

+          this.clearUp(true);
+          logger.info("rewardViCalService is already done");
+        } else {
+          if (this.getLatestBlockHeaderNumber() > lastBlockNumber) {


Is getLatestSolidifiedBlockNum & lastSolidifiedBlockNumber better?

This service is depend on root dbs.

yanghang8612

LGTM

halibobo1205 · 2024-02-28T12:53:52Z

Review task on 2.28:

Check getNewRewardAlgorithmEffectiveCycle() == 0
Code clear: DBIterator, metric
simplify lazy logging with slf4j: 1. is there a need to do a if log is debug enabled check 2. lazy-evaluation-for-logging-in-java-8 3. lazy-logging-with-slf4j 4. simplify-lazy-logging-with-slf4j
rewardViRoot: 1. Configurable for Nile 2. Check the log when released.
rewardViCalTask maybeRun exception deal carefully
Boundary Value Analysis: beginCycle - 1 & endCycle - 1

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

halibobo1205 requested review from jwrct and tomatoishealthy August 9, 2023 07:04

lvs007 requested a review from xxo1shine August 10, 2023 07:42

halibobo1205 force-pushed the feat/old_reward_cal_opt branch from c2a11c6 to 3fc4dbc Compare August 10, 2023 07:57

halibobo1205 self-assigned this Aug 10, 2023

tomatoishealthy reviewed Aug 11, 2023

View reviewed changes

tomatoishealthy approved these changes Aug 14, 2023

View reviewed changes

jwrct approved these changes Aug 18, 2023

View reviewed changes

xxo1shine reviewed Aug 29, 2023

View reviewed changes

halibobo1205 force-pushed the feat/old_reward_cal_opt branch from 3fc4dbc to 69b5391 Compare August 30, 2023 03:02

halibobo1205 force-pushed the feat/old_reward_cal_opt branch from 732d605 to 00e349b Compare August 30, 2023 13:37

halibobo1205 requested review from tomatoishealthy, jwrct and xxo1shine August 30, 2023 13:38

tomatoishealthy reviewed Aug 31, 2023

View reviewed changes

xxo1shine approved these changes Aug 31, 2023

View reviewed changes

tomatoishealthy approved these changes Aug 31, 2023

View reviewed changes

jwrct approved these changes Aug 31, 2023

View reviewed changes

halibobo1205 changed the base branch from develop to release_v4.7.3 September 11, 2023 13:39

halibobo1205 added the status:on-hold label Sep 14, 2023

srbaeza-ai mentioned this pull request Oct 19, 2023

```java srbaeza-ai/Silver-win#1

Closed

halibobo1205 modified the milestones: GreatVoyage-v4.7.3, GreatVoyage-v4.8 Oct 30, 2023

halibobo1205 force-pushed the feat/old_reward_cal_opt branch from 00e349b to 7c6235f Compare November 8, 2023 10:12

halibobo1205 added 17 commits January 4, 2024 10:46

check if the delegation is still valid

309cf54

change getting data from root db

0435e58

use account db to scan instead of delegation for vote before 2019-10-…

6576d48

…31 20:00:00

change old cal to new cal

7a7bcfe

add a proposal for old reward opt

f0c38df

ensure checkpoint write-back

a7246e6

fix the case that require a service restart for cache computation

b47b18f

add reward-vi root check and query

e255a40

enhances the proposal activation checking logic

4f831af

remove unnecessary queries

6c26b53

add proposal [ALLOW_OLD_REWARD_OPT]

4fdfb46

rename service

6008ecc

remove query reward-vi root from node-info

74cd4fe

if cal is not done, wait it when quering

5bfc303

fix Proposal test

2c1861e

fix sonar code smells

cd76dd3

rename getMerkleHash to getHash

320493a

halibobo1205 force-pushed the feat/old_reward_cal_opt branch from 3203edf to 320493a Compare January 4, 2024 02:51

jwrct reviewed Jan 4, 2024

View reviewed changes

halibobo1205 added 2 commits January 4, 2024 15:20

wait for the reward calculation for test

2c5db36

fix concurrency issue

6dff8d4

jwrct reviewed Jan 4, 2024

View reviewed changes

halibobo1205 added 2 commits January 4, 2024 19:32

rethrow InterruptedException

d85cce8

ensure that the checkpoint flush is over before startRewardCal

5d6f09c

tomatoishealthy reviewed Jan 5, 2024

View reviewed changes

yanghang8612 approved these changes Jan 5, 2024

View reviewed changes

halibobo1205 merged commit 52b3327 into tronprotocol:develop Jan 5, 2024
7 checks passed

halibobo1205 removed the status: on-going label Mar 1, 2024

halibobo1205 mentioned this pull request Mar 4, 2024

feat(*): optimize old rewards withdrawal #5742

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(db): optimize old rewards withdrawal #5406

feat(db): optimize old rewards withdrawal #5406

halibobo1205 commented Aug 9, 2023

halibobo1205 commented Aug 9, 2023

tomatoishealthy Aug 11, 2023

halibobo1205 Aug 11, 2023

tomatoishealthy Aug 11, 2023

halibobo1205 Aug 11, 2023

tomatoishealthy Aug 11, 2023

halibobo1205 Aug 11, 2023

tomatoishealthy Aug 11, 2023

halibobo1205 Aug 30, 2023

halibobo1205 commented Aug 11, 2023

This comment was marked as resolved.

halibobo1205 Aug 30, 2023

xxo1shine Aug 29, 2023

halibobo1205 Aug 29, 2023

halibobo1205 Aug 30, 2023

codecov-commenter commented Aug 30, 2023 •

edited

Loading

tomatoishealthy Aug 31, 2023

halibobo1205 Aug 31, 2023

jwrct Jan 4, 2024

halibobo1205 Jan 4, 2024

jwrct Jan 4, 2024

halibobo1205 Jan 4, 2024

tomatoishealthy Jan 5, 2024

halibobo1205 Jan 5, 2024

yanghang8612 left a comment

halibobo1205 commented Feb 28, 2024

		@@ -13,23 +14,22 @@ public final class StoreIterator implements org.tron.core.db.common.iterator.DBI
		private final DBIterator dbIterator;
		private boolean first = true;

feat(db): optimize old rewards withdrawal #5406

feat(db): optimize old rewards withdrawal #5406

Conversation

halibobo1205 commented Aug 9, 2023

halibobo1205 commented Aug 9, 2023

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

halibobo1205 commented Aug 11, 2023

This comment was marked as resolved.

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 30, 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

Choose a reason for hiding this comment

yanghang8612 left a comment

Choose a reason for hiding this comment

halibobo1205 commented Feb 28, 2024

codecov-commenter commented Aug 30, 2023 •

edited

Loading