Skip to content

feat(log): optimize open db log #5385

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Aug 10, 2023
Merged

feat(log): optimize open db log #5385

merged 2 commits into from
Aug 10, 2023

Conversation

lurais
Copy link
Contributor

@lurais lurais commented Jul 28, 2023

What does this PR do?
Add suggestion when open db fail.

Why are these changes required?
To improve user-friendliness.

@halibobo1205 halibobo1205 linked an issue Jul 31, 2023 that may be closed by this pull request
Give suggestion about db corrupt error info #5382
Closed
@lvs007 lvs007 requested a review from halibobo1205 August 1, 2023 06:24
@lurais lurais force-pushed the feature/optimize_db_corrupt_log branch from 6c43e7f to 66cba65 Compare August 4, 2023 11:15
halibobo1205
halibobo1205 reviewed Aug 7, 2023
View reviewed changes
chainbase/src/main/java/org/tron/common/storage/leveldb/LevelDbDataSourceImpl.java Outdated
}
} catch (IOException e) {
if (e.getMessage().contains("Corruption:")) {
throw new RuntimeException(String.format("Can't initialize database: %s, please delete " +
Copy link
Contributor

Choose a reason for hiding this comment

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

Please do not throw an exception here. If you do not know what to do, please refer https://github.com/tronprotocol/java-tron/blob/develop/framework/src/main/java/org/tron/core/db/Manager.java#L572. And, please keep the changes to the RocksDB.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please do not throw an exception here. If you do not know what to do, please refer https://github.com/tronprotocol/java-tron/blob/develop/framework/src/main/java/org/tron/core/db/Manager.java#L572. And, please keep the changes to the RocksDB.

Just exit when open db fail is ok.@halibobo1205

@lurais lurais force-pushed the feature/optimize_db_corrupt_log branch 2 times, most recently from 865ea1b to 41536aa Compare August 8, 2023 06:56
halibobo1205
halibobo1205 reviewed Aug 8, 2023
View reviewed changes
chainbase/src/main/java/org/tron/common/storage/leveldb/LevelDbDataSourceImpl.java Outdated
}
} catch (IOException e) {
if (e.getMessage().contains("Corruption:")) {
logger.error("Database {} corrupted, please delete database directory({}) " +
Copy link
Contributor

Choose a reason for hiding this comment

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

please refer https://github.com/tronprotocol/java-tron/blob/develop/framework/src/main/java/org/tron/core/db/Manager.java#L572

Copy link
Contributor Author

Choose a reason for hiding this comment

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

please refer https://github.com/tronprotocol/java-tron/blob/develop/framework/src/main/java/org/tron/core/db/Manager.java#L572

@halibobo1205 Just print the same message as L572? Or just exit when any exception is caught?

Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Exit when any db exception is caught
  2. Give a tip just like L572 when db is corrupted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. Exit when any db exception is caught
  2. Give a tip just like L572 when db is corrupted.

@halibobo1205 done.

feat(log): optimize open db log
3dacceb 
Add suggestion when open db fail.
@lurais lurais force-pushed the feature/optimize_db_corrupt_log branch from 41536aa to 3dacceb Compare August 8, 2023 09:08
tomatoishealthy
tomatoishealthy approved these changes Aug 9, 2023
View reviewed changes
Copy link
Contributor

@tomatoishealthy tomatoishealthy left a comment

Choose a reason for hiding this comment

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

LGTM, fix the CI check

halibobo1205
halibobo1205 reviewed Aug 9, 2023
View reviewed changes
framework/src/test/java/org/tron/common/storage/leveldb/LevelDbDataSourceImplTest.java Outdated
@@ -107,6 +109,13 @@ public void testReset() {
Args.getInstance().getOutputDirectory(), "test_reset");
dataSource.resetDb();
assertEquals(0, dataSource.allKeys().size());
assertEquals("LEVELDB", dataSource.getEngine());
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a test to cover this PR, do not add tests that is not relevant in this PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please add a test to cover this PR, do not add tests that is not relevant in this PR

done. @halibobo1205

halibobo1205
halibobo1205 reviewed Aug 9, 2023
View reviewed changes
framework/src/test/java/org/tron/common/storage/leveldb/RocksDbDataSourceImplTest.java Outdated
@@ -97,6 +99,13 @@ public void testReset() {
Args.getInstance().getOutputDirectory(), "test_reset");
dataSource.resetDb();
assertEquals(0, dataSource.allKeys().size());
assertEquals("ROCKSDB", dataSource.getEngine());
Copy link
Contributor

Choose a reason for hiding this comment

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

Comments as above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comments as above

done. @halibobo1205

@halibobo1205 halibobo1205 added this to the GreatVoyage-v4.7.3 milestone Aug 9, 2023
@codecov-commenter
Copy link

Codecov Report

Merging #5385 (8e289cd) into develop (9a65bb6) will increase coverage by 0.00%.
Report is 3 commits behind head on develop.
The diff coverage is 40.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    #5385   +/-   ##
==========================================
  Coverage      60.92%   60.92%           
- Complexity      9231     9238    +7     
==========================================
  Files            839      839           
  Lines          50029    50038    +9     
  Branches        5574     5576    +2     
==========================================
+ Hits           30479    30487    +8     
- Misses         17165    17169    +4     
+ Partials        2385     2382    -3
Files Changed Coverage Δ
.../common/storage/rocksdb/RocksDbDataSourceImpl.java 68.65% <0.00%> (+1.73%) ⬆️
.../common/storage/leveldb/LevelDbDataSourceImpl.java 75.87% <54.54%> (+3.02%) ⬆️

... and 4 files with indirect coverage changes

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

@lurais lurais force-pushed the feature/optimize_db_corrupt_log branch 2 times, most recently from e968b90 to d860a4e Compare August 9, 2023 15:44
@lurais lurais force-pushed the feature/optimize_db_corrupt_log branch from d860a4e to 6f082a8 Compare August 10, 2023 03:41
@halibobo1205 halibobo1205 merged commit 61c6177 into tronprotocol:develop Aug 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tomatoishealthy tomatoishealthy tomatoishealthy approved these changes

@halibobo1205 halibobo1205 halibobo1205 approved these changes

Assignees

@lurais lurais

Labels
topic:log
Projects
None yet
Milestone
GreatVoyage-v4.7.3
Development

Successfully merging this pull request may close these issues.

Give suggestion about db corrupt error info
4 participants
@lurais @codecov-commenter @tomatoishealthy @halibobo1205