Store txns in DB #1735

KaustubhShamshery · 2019-07-22T07:26:10Z

Description

Shard and DS nodes store processed txns in LevelDB

Note: processedTxnTmp stores processed txns, however, these may not be confirmed.

Backward Compatibility

This is not a breaking change
This is a breaking change

Review Suggestion

Status

Implementation

ready for review

Integration Test (Core Team)

local machine test

small-scale cloud test

sandipbhoir · 2019-07-22T07:57:34Z

can you verify if the persistence with txBodiesTmp from normal shard node is available in recovered node after recovery?

codecov-io · 2019-07-22T08:30:37Z

Codecov Report

Merging #1735 into master will decrease coverage by 1.14%.
The diff coverage is 6.66%.

@@            Coverage Diff             @@
##           master    #1735      +/-   ##
==========================================
- Coverage    33.5%   32.36%   -1.15%     
==========================================
  Files         270      270              
  Lines       33140    35055    +1915     
==========================================
+ Hits        11105    11347     +242     
- Misses      22035    23708    +1673

Impacted Files	Coverage Δ
src/libPersistence/BlockStorage.h	`84.61% <ø> (ø)`	⬆️
src/common/Constants.h	`100% <ø> (ø)`	⬆️
src/libPersistence/BlockStorage.cpp	`27.64% <0%> (-0.48%)`	⬇️
src/libNode/MicroBlockPreProcessing.cpp	`0% <0%> (ø)`	⬆️
src/common/Constants.cpp	`99.75% <100%> (ø)`	⬆️
src/libConsensus/ConsensusCommon.h	`16.66% <0%> (-8.34%)`	⬇️
tests/POW/test_POW.cpp	`84.02% <0%> (-0.19%)`	⬇️
src/libServer/LookupServer.cpp	`0.08% <0%> (-0.03%)`	⬇️
...rc/libDirectoryService/FinalBlockPreProcessing.cpp	`0% <0%> (ø)`	⬆️
src/libDirectoryService/DSBlockPreProcessing.cpp	`0% <0%> (ø)`	⬆️
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 04b3398...38f7a20. Read the comment docs.

ShengguangXiao · 2019-07-24T03:20:32Z

constants.xml

@@ -262,6 +262,7 @@
        <ENABLE_REPOPULATE>true</ENABLE_REPOPULATE>
        <REPOPULATE_STATE_IN_DS>0</REPOPULATE_STATE_IN_DS>
        <REPOPULATE_STATE_PER_N_DS>10</REPOPULATE_STATE_PER_N_DS>
+        <NUM_STORE_TX_BODIES>5</NUM_STORE_TX_BODIES>


Change NUM_STORE_TX_BODIES to STORE_TX_BODIES_BLK_INTERVAL will be more understandable

sandipbhoir · 2019-07-30T01:24:53Z

src/libPersistence/BlockStorage.cpp

-           RefreshDB(SHARD_STRUCTURE) & RefreshDB(STATE_DELTA) &
-           RefreshDB(TEMP_STATE) & RefreshDB(DIAGNOSTIC_NODES) &
-           RefreshDB(DIAGNOSTIC_COINBASE) & RefreshDB(STATE_ROOT) &
+           RefreshDB(TX_BODY) & RefreshDB(MICROBLOCK) &


Should this be for TX_BODY_TMP instead?

yes, thanks

sandipbhoir · 2019-07-30T04:49:58Z

what would be the behavior if same processed txns are already commited to txBodies db and after recovery of shard node we have them in both txBodies and txBodyTmp.

KaustubhShamshery · 2019-07-30T04:50:50Z

what would behavior if same processed txns are already commited to txBodies db and after recovery of shard node we have them in both txBodies and txBodyTmp.

They would not affect each other, they are different level DBs.

sandipbhoir · 2019-07-30T04:53:42Z

what would behavior if same processed txns are already commited to txBodies db and after recovery of shard node we have them in both txBodies and txBodyTmp.

They would not affect each other, they are different level DBs.

Ofcorse, but normally processed txns are removed from Tmp after moving to txBodies. In this case, we have it in both. Just curious if that breaks any logic later. If its ok, then all good.

KaustubhShamshery · 2019-07-30T04:55:47Z

what would behavior if same processed txns are already commited to txBodies db and after recovery of shard node we have them in both txBodies and txBodyTmp.

They would not affect each other, they are different level DBs.

Ofcorse, but normally processed txns are removed from Tmp after moving to txBodies. In this case, we have it in both. Just curious if that breaks any logic later. If its ok, then all good.

That is in case of lookup nodes, both of these were not being used in shard nodes before.

kaikawaliu

TxBodyTmp is used for cleaning the transactions processed in the latest DS epoch. It's not used for storage purpose. I suggest making another db instance for this specific usage.

src/libNode/MicroBlockPreProcessing.cpp

sandipbhoir · 2019-08-01T04:50:31Z

src/libNode/MicroBlockPreProcessing.cpp

@@ -641,9 +646,22 @@ void Node::ProcessTransactionWhenShardBackup() {

  cv_TxnProcFinished.notify_all();

+  PutTxnsInTempDataBase(t_processedTransactions);


We need to consider the fact that the txns in leveldb for backup may not be confirmed transaction.

Store txns in DB

9093037

KaustubhShamshery requested review from sandipbhoir and kaikawaliu and removed request for sandipbhoir July 22, 2019 07:26

KaustubhShamshery self-assigned this Jul 22, 2019

KaustubhShamshery added the Untested Haven't been tested yet label Jul 22, 2019

add constant

135e117

add to constant file

02b4ca0

KaustubhShamshery force-pushed the feature/normal_node_store_txns branch from cefa5e7 to 02b4ca0 Compare July 22, 2019 08:28

remove placeholder

a8e1b33

ansnunez added this to PRs in development in Core via automation Jul 23, 2019

ShengguangXiao reviewed Jul 24, 2019

View reviewed changes

KaustubhShamshery added Ready Ready for review and removed Untested Haven't been tested yet labels Jul 29, 2019

change constant name

5d01af1

sandipbhoir reviewed Jul 30, 2019

View reviewed changes

fix RefreshAll

9f65b36

KaustubhShamshery requested review from sandipbhoir and ShengguangXiao July 30, 2019 04:40

sandipbhoir approved these changes Jul 30, 2019

View reviewed changes

ansnunez moved this from PRs in development to PRs needing review (please help!) in Core Jul 30, 2019

kaikawaliu suggested changes Jul 30, 2019

View reviewed changes

change db name

b5f87c2

KaustubhShamshery force-pushed the feature/normal_node_store_txns branch from 11eb102 to 17d7839 Compare July 30, 2019 15:40

fix refresh and reset

31f162b

KaustubhShamshery force-pushed the feature/normal_node_store_txns branch from 17d7839 to 31f162b Compare July 30, 2019 15:53

add name to reset

edee0c9

ShengguangXiao approved these changes Jul 31, 2019

View reviewed changes

fix db pointer

38f7a20

KaustubhShamshery requested a review from kaikawaliu July 31, 2019 08:59

KaustubhShamshery added the on hold label Jul 31, 2019

sandipbhoir reviewed Jul 31, 2019

View reviewed changes

src/libNode/MicroBlockPreProcessing.cpp Show resolved Hide resolved

KaustubhShamshery removed the on hold label Jul 31, 2019

KaustubhShamshery requested a review from sandipbhoir July 31, 2019 09:55

fix name & change location for saving

1235e2f

kaikawaliu approved these changes Aug 1, 2019

View reviewed changes

Core automation moved this from PRs needing review to PRs approved - ready to merge! Aug 1, 2019

sandipbhoir reviewed Aug 1, 2019

View reviewed changes

sandipbhoir approved these changes Aug 1, 2019

View reviewed changes

ansnunez merged commit dd76c5c into master Aug 2, 2019

Core automation moved this from PRs approved - ready to merge! to PRs done (merged) Aug 2, 2019

n-hutton deleted the feature/normal_node_store_txns branch August 8, 2022 10:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Store txns in DB #1735

Store txns in DB #1735

KaustubhShamshery commented Jul 22, 2019 •

edited

sandipbhoir commented Jul 22, 2019

codecov-io commented Jul 22, 2019 •

edited

ShengguangXiao Jul 24, 2019

sandipbhoir Jul 30, 2019

KaustubhShamshery Jul 30, 2019

sandipbhoir commented Jul 30, 2019 •

edited

KaustubhShamshery commented Jul 30, 2019

sandipbhoir commented Jul 30, 2019

KaustubhShamshery commented Jul 30, 2019 •

edited

kaikawaliu left a comment

sandipbhoir Aug 1, 2019

		@@ -641,9 +646,22 @@ void Node::ProcessTransactionWhenShardBackup() {

		cv_TxnProcFinished.notify_all();

		PutTxnsInTempDataBase(t_processedTransactions);

Store txns in DB #1735

Store txns in DB #1735

Conversation

KaustubhShamshery commented Jul 22, 2019 • edited

Description

Backward Compatibility

Review Suggestion

Status

Implementation

Integration Test (Core Team)

sandipbhoir commented Jul 22, 2019

codecov-io commented Jul 22, 2019 • edited

Codecov Report

ShengguangXiao Jul 24, 2019

Choose a reason for hiding this comment

sandipbhoir Jul 30, 2019

Choose a reason for hiding this comment

KaustubhShamshery Jul 30, 2019

Choose a reason for hiding this comment

sandipbhoir commented Jul 30, 2019 • edited

KaustubhShamshery commented Jul 30, 2019

sandipbhoir commented Jul 30, 2019

KaustubhShamshery commented Jul 30, 2019 • edited

kaikawaliu left a comment

Choose a reason for hiding this comment

sandipbhoir Aug 1, 2019

Choose a reason for hiding this comment

KaustubhShamshery commented Jul 22, 2019 •

edited

codecov-io commented Jul 22, 2019 •

edited

sandipbhoir commented Jul 30, 2019 •

edited

KaustubhShamshery commented Jul 30, 2019 •

edited