FO-284: Save and Prune versions automatically on commit #7

kevinssgh · 2021-09-01T18:29:32Z

Added function to save copy of batch at given height for a versioned history of the state.
Added functionality to prune the auxiliary version data outside a given window
Added unit tests to cover this functionality

…outside of the version window

ws4charlie · 2021-09-01T22:42:02Z

src/state/chain_state.rs

+        );
+
+        //Query aux values that are older than the window size to confirm batches were pruned
+        for i in 1..10 {


Have we pruned [1, 10) or [1, 10] in this case?

okay, I guess the window contains the current height and it actually doesn't.

The current height at this point is 20, the window is 10. This means that versions 1 - 9 were pruned. So the current version is in the window.

ws4charlie · 2021-09-01T22:45:44Z

src/state/chain_state.rs

+        }
+        //Build range keys for window limits
+        let window_start_height = (height - self.ver_window).to_string();
+        let pruning_height = (height - self.ver_window - 1).to_string();


Do we prune only one single commit? I was considering what will happen if a Node previously configured its window length as 1000 but he modified it to 500 and restart the node. In this case we probably have to prune 500+ commit batches.

You're right, there will be extra versions in the aux db if this happens. I was trying to reduce the complexity of this pruning logic, so it's really a question of whether this library is responsible for cleaning up data for an application that has changed configurations.

I was just thinking, how would you know what the previous window size was? And then how far back you need to iterate and delete. Probably need a maximum amount to look back and delete. This can be done when initializing the chainstate... but yes, we can discuss this in the morning if that's ok.

Sure thing. Let's talk.

ws4charlie · 2021-09-01T22:50:58Z

src/state/chain_state.rs

+                {
+                    // Add the key to new window limit height
+                    batch.push((
+                        new_window_limit


Would it be better that we add a tag before the height number, like "V100_key1" or "VER100_key2", to group up all commit history. Besides these commit history data, the only key that also lives in aux partition for now is the "Height".

It's not really necessary to add a tag, its more about preference since there are no other keys being stored in the db (other than the Height). It is a simple change if you think it is necessary

By adding a Tag, we would be elegant and be able to get all the commit batches by a range query "VER_" "VER~". Then the first record located contains lowest height, and the last record contains the highest height.

ws4charlie · 2021-09-01T22:55:23Z

src/state/chain_state.rs

+                    .exists_aux(new_window_limit.push(key[1].as_bytes()).as_ref())
+                    .unwrap_or(false)
+                {
+                    // Add the key to new window limit height


Are you rolling up the KVs in "pruning_height" into "pruning_height+1"(window_start_height). But how did you merge/combine a the KV update (1_key1_valueB ==> 2_key2_valueB)?

Yes, the condition above is checking if the key already exists in pruning_height + 1, if it doesn't then the key is rolled into the new window. Not sure what the question is.

The question was how "pruning_height_key1_someValue" gets converted into "pruning_height+1_key1_someValue" and then rolled into the new window while there is ALREADY a "pruning_height+1_key1_someOtherValue" living in there.

If you see this condition: !self.exists_aux(new_window_limit.push(key[1].as_bytes()).as_ref())

I'm checking if it doesn't exist in the new window before attempting to move the Key and value. So this actually doesn't do anything if that key is already in the new window.

…fter a node restart.

ws4charlie · 2021-09-03T19:51:05Z

src/state/chain_state.rs

+    /// This is to keep a versioned history of KV pairs.
+    fn build_aux_batch(&self, height: u64, batch: &mut KVBatch) -> Result<KVBatch> {
+        let height_str = height.to_string();
+        let prefix = Prefix::new("VER".as_bytes()).push(height_str.as_bytes());


Can we wrap up this prefix building logic into a private func. I will need it as well when I export a snapshot on a specific height. Like fn version_prefix(height: u64) -> Prefix {...}.

ws4charlie · 2021-09-03T19:54:19Z

src/state/chain_state.rs

+            .map(|(k, v)| {
+                (
+                    prefix.clone().push(k.clone().as_slice()).as_ref().to_vec(),
+                    v.clone(),


if v == None, are we storing nothing in aux db? When pruning/replay works, how would we delete the value from aux db.

ws4charlie · 2021-09-03T19:56:41Z

src/state/chain_state.rs

+            IterOrder::Desc,
+            &mut |(k, _v)| -> bool {
+                //Delete the key from aux db
+                batch.push((k, None));


Should we delete commits history or should we roll up the edge (on oldest side, which is the base state) all the way to new edge (of reduced window)

Added some helper functions

Added unit test to confirm functionality

ws4charlie · 2021-09-10T17:07:28Z

src/state/chain_state.rs

+        let cs_new = chain_state::ChainState::new(fdb_new, "test_db".to_string(), new_window_size);
+
+        //Confirm keys older than new window size have been deleted
+        for i in 1..(number_of_batches - new_window_size - 1) {


shouldn't the range be 1..(number_of_batches - new_window_size)

I believe there is one extra version saved as the base chain-state but I can double check this to be sure.

ws4charlie · 2021-09-10T17:22:16Z

src/state/chain_state.rs

+        }
+
+        //Get batch for state at H = current_height - ver_window
+        let batch = self.build_state(current_height - self.ver_window);


Just thought of an edge case when there is a "TOMBSTONE" in H ( = current_height - ver_window). The returned batch will be committed to aux as expected, but seems the TOMBSTONE will stay undeleted?

It's ok, when we query the value it will be seen as None. The actual value will be deleted when prune_aux_batch is called as the window slides.

ktmlm · 2021-09-11T00:49:45Z

The checkpoint/version managements of rocksdb is implemented in a file level, not file-system block level, so we should do enough tests for this feature.

The effects are more like overlayfs, not the snapshots style of ZFS/BTRFS, etc.

they are light weight compared with full-copy, but not light weight enough.

Refactored Build aux batch to ignore versioning if Ver_window == 0

…BSTONE

ws4charlie · 2021-09-15T19:40:51Z

The checkpoint/version managements of rocksdb is implemented in a file level, not file-system block level, so we should do enough tests for this feature.

The effects are more like overlayfs, not the snapshots style of ZFS/BTRFS, etc.

they are light weight compared with full-copy, but not light weight enough.

Yes yes. RocksDB snapshot is being used at the level of Merk library to take a copy. Storage-level versioning here is done in an incremental way like Git Commit history. The EVM JSON RPCs needs versioned data.

ws4charlie · 2021-09-15T20:16:55Z

looks good now.

Added test cases for creating auxiliary batch and for pruning values …

613debd

…outside of the version window

kevinssgh requested review from harshadptl and ws4charlie September 1, 2021 18:29

ws4charlie reviewed Sep 1, 2021

View reviewed changes

kevinssgh added 3 commits September 3, 2021 14:56

Added Feature to clean Aux DB in case the window size has decreased a…

f5dbfc5

…fter a node restart.

Merge branch 'develop' into dev-versioning

c2f6b51

Fixed merge errors

874d31b

ws4charlie reviewed Sep 3, 2021

View reviewed changes

kevinssgh added 3 commits September 7, 2021 20:41

Fixed key design for version iteration.

9d3a912

Added function to build state from height 1

c98f602

Added some helper functions

Added Get key by version interface

a140646

Added unit test to confirm functionality

ws4charlie reviewed Sep 10, 2021

View reviewed changes

kevinssgh added 7 commits September 13, 2021 11:24

Added some comments to describe new functions

842e843

Refactored Build aux batch to ignore versioning if Ver_window == 0

Exposed get value by version to the store traits

10164da

Updated get value by version implementation

400ad08

Updated Readme with versioning info.

af15ab4

fix spelling errors.

bee1dde

Set lower bound on get value by version.

27ce6c0

Don't need to roll up keys from previous window if the value is a TOM…

74662d5

…BSTONE

ws4charlie approved these changes Sep 15, 2021

View reviewed changes

kevinssgh merged commit f29d0ae into develop Sep 15, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FO-284: Save and Prune versions automatically on commit #7

FO-284: Save and Prune versions automatically on commit #7

kevinssgh commented Sep 1, 2021

ws4charlie Sep 1, 2021

kevinssgh Sep 1, 2021

ws4charlie Sep 2, 2021

kevinssgh Sep 2, 2021

ws4charlie Sep 1, 2021

kevinssgh Sep 1, 2021

kevinssgh Sep 1, 2021

ws4charlie Sep 2, 2021

ws4charlie Sep 1, 2021

kevinssgh Sep 1, 2021 •

edited

ws4charlie Sep 2, 2021

ws4charlie Sep 1, 2021

kevinssgh Sep 1, 2021

ws4charlie Sep 2, 2021

kevinssgh Sep 2, 2021

ws4charlie Sep 3, 2021

ws4charlie Sep 3, 2021

ws4charlie Sep 3, 2021

ws4charlie Sep 10, 2021

kevinssgh Sep 10, 2021

ws4charlie Sep 10, 2021

kevinssgh Sep 10, 2021

ktmlm commented Sep 11, 2021 •

edited

ws4charlie commented Sep 15, 2021

ws4charlie commented Sep 15, 2021

FO-284: Save and Prune versions automatically on commit #7

FO-284: Save and Prune versions automatically on commit #7

Conversation

kevinssgh commented Sep 1, 2021

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

kevinssgh Sep 1, 2021 • edited

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

Choose a reason for hiding this comment

ktmlm commented Sep 11, 2021 • edited

ws4charlie commented Sep 15, 2021

ws4charlie commented Sep 15, 2021

kevinssgh Sep 1, 2021 •

edited

ktmlm commented Sep 11, 2021 •

edited