feat: implemented incoming receipts handling in resharding #9474

wacban · 2023-09-01T10:59:24Z

This PR implements proper handling of incoming receipts in resharding.
Conceptually there are two parts, that take place when iterating back through the chain and collecting incoming receipts.

detect when the shard layout changes and adjust the shard id to its parent shard it
when picking up the receipts only pick ones that have receiver in the target shard id

For example in the following scenario:

resharding 3->3', 4'
block 10 is the last with the old shard layout
block 10 has some missing chunks and some incoming receipts with receivers in shard 3
The implementation when getting incoming receipts for 4' will do the following:
adjust the shard id from 4' to its parent 3
get the incoming receipts for 3
filter out only the receipts that have receiver_id falling into 4' in the new shard layout

This scenario is covered by the integration test implemented in #9467.

jakmeier · 2023-09-04T12:15:57Z

I see @pugachAG you have assigned yourself as a reviewer. Thank you for reviewing! I will remove myself, ping me if you think my input specifically is needed somewhere.

pugachAG

overall looks good to me, but please take a look at my comments before merging

pugachAG · 2023-09-04T12:00:35Z

chain/chain/src/store.rs

        mut block_hash: CryptoHash,
        last_chunk_height_included: BlockHeight,
    ) -> Result<Vec<ReceiptProofResponse>, Error> {
+        let _span =
+            tracing::debug_span!(target: "store", "get_incoming_receipts_for_shard").entered();


target: "store" is used in store crate, I think here target: "chain" would be more appropriate (applies to the all instances in this function)

chain/chain/src/store.rs

pugachAG · 2023-09-04T12:18:53Z

tools/state-viewer/src/apply_chain_range.rs

@@ -192,9 +192,11 @@ fn apply_block_from_range(
            }
        };

+        // TODO(wacban) OK, has epoch manager handy


would be nice to have a bit more descriptive comment here 😃

Opsie daisy :)

pugachAG · 2023-09-04T12:23:18Z

chain/chain/src/store.rs

                    );
-                    ret.push(ReceiptProofResponse(block_hash, receipt_proofs));
+
+                    // If the shard layout changed we need to filter to make


nit: consider moving that part to a separate function, something like

fn filter_incoming_receipts_for_shard(parent_shard_receipts: &[ReceiptProof], ...) -> Vec<ReceiptProof> { ... }

wacban · 2023-09-01T13:03:03Z

chain/chain/src/store.rs

+                        // TODO(resharding) adjust the shard proof accordingly
+                        // currently this only matters for state sync
+                        let receipt_proof = ReceiptProof(filtered_receipts, shard_proof);
+                        filtered_receipt_proofs.push(receipt_proof);


Note for the rightfully concerned reviewer.

This method has two important usages:

From the get_apply_chunk_job_new_chunk where the proofs are immediately discarded and only the receipts are used

nearcore/chain/chain/src/chain.rs

Line 3985 in 6762c06

let receipt_proof_response = &self.store().get_incoming_receipts_for_shard(

From the compute_state_response_header where it will likely break things. This method is used for state sync which currently is not supported together with resharding but it will need to be fixed, hence the TODO.

nearcore/chain/chain/src/chain.rs

Line 2929 in 6762c06

let incoming_receipts_proofs = self.store.get_incoming_receipts_for_shard(

wacban · 2023-09-05T07:41:05Z

tools/state-viewer/src/apply_chain_range.rs

@@ -192,9 +192,11 @@ fn apply_block_from_range(
            }
        };

+        // TODO(wacban) OK, has epoch manager handy


Opsie daisy :)

This reverts commit 01f1f8e.

This reverts commit 73488ba.

This reverts commit ac5e40a.

wacban changed the title ~~Waclaw resharding incoming fix~~ feat: implemented incoming receipts handling in resharding Sep 1, 2023

wacban marked this pull request as ready for review September 1, 2023 13:16

wacban requested a review from a team as a code owner September 1, 2023 13:16

wacban requested review from jakmeier and removed request for a team September 1, 2023 13:16

wacban mentioned this pull request Sep 4, 2023

🔷 [Tracking issue] Resharding v2 #8992

Closed

pugachAG self-requested a review September 4, 2023 09:26

jakmeier removed their request for review September 4, 2023 12:16

pugachAG approved these changes Sep 4, 2023

View reviewed changes

wacban commented Sep 5, 2023

View reviewed changes

Base automatically changed from waclaw-resharding-incoming to master September 6, 2023 12:05

wacban added 9 commits September 6, 2023 12:24

Revert "undo some hardcoded changes for debugging"

54aaf39

This reverts commit 01f1f8e.

Revert "remove debug logs"

efc3029

This reverts commit 73488ba.

get the epoch manager

b9501e0

impl

2610694

Revert "Revert "undo some hardcoded changes for debugging""

b899a94

This reverts commit ac5e40a.

cleanup

6ea1dd8

cleanup

a9ba603

cleanup

b79df2b

code review

f5d461b

wacban force-pushed the waclaw-resharding-incoming-fix branch from 73e2d28 to f5d461b Compare September 6, 2023 12:25

wacban enabled auto-merge September 6, 2023 12:27

wacban added this pull request to the merge queue Sep 6, 2023

Merged via the queue into master with commit 95b8f30 Sep 6, 2023

wacban deleted the waclaw-resharding-incoming-fix branch September 6, 2023 14:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: implemented incoming receipts handling in resharding #9474

feat: implemented incoming receipts handling in resharding #9474

wacban commented Sep 1, 2023 •

edited

Loading

jakmeier commented Sep 4, 2023

pugachAG left a comment

pugachAG Sep 4, 2023

pugachAG Sep 4, 2023

wacban Sep 5, 2023

pugachAG Sep 4, 2023

wacban Sep 1, 2023 •

edited by pugachAG

Loading

wacban Sep 5, 2023

feat: implemented incoming receipts handling in resharding #9474

feat: implemented incoming receipts handling in resharding #9474

Conversation

wacban commented Sep 1, 2023 • edited Loading

jakmeier commented Sep 4, 2023

pugachAG left a comment

Choose a reason for hiding this comment

pugachAG Sep 4, 2023

Choose a reason for hiding this comment

pugachAG Sep 4, 2023

Choose a reason for hiding this comment

wacban Sep 5, 2023

Choose a reason for hiding this comment

pugachAG Sep 4, 2023

Choose a reason for hiding this comment

wacban Sep 1, 2023 • edited by pugachAG Loading

Choose a reason for hiding this comment

wacban Sep 5, 2023

Choose a reason for hiding this comment

wacban commented Sep 1, 2023 •

edited

Loading

wacban Sep 1, 2023 •

edited by pugachAG

Loading