feat(spice): Sketch the datatypes to use in the data distribution#16053
feat(spice): Sketch the datatypes to use in the data distribution#16053ssavenko-near wants to merge 2 commits into
Conversation
eae50ef to
aa98b12
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #16053 +/- ##
==========================================
+ Coverage 73.25% 73.27% +0.01%
==========================================
Files 857 863 +6
Lines 188641 189349 +708
Branches 188641 189349 +708
==========================================
+ Hits 138188 138740 +552
- Misses 46038 46180 +142
- Partials 4415 4429 +14
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| /// Everyone who contributed any unit — the culprit set for a semantic `Failed` | ||
| /// on the delivered (winning-commitment) data. | ||
| pub(crate) fn all(&self) -> Vec<AccountId> { | ||
| Vec::new() // sketch | ||
| } |
There was a problem hiding this comment.
I understood this to mean if there are competing commitments, we let the first internally consistent one "win", then if it turns out to be semantically incorrect, an upper layer will discredit the peers we got it from? Does it trigger a retry too?
There was a problem hiding this comment.
"First internally consistent one wins" - yes.
"Upper layer discredits the peers we got it from" - yes
"Does it trigger a retry too?" - yes, but only for locally recognized inconsistencies. If we deliver the endorsement for self-consistent but "wrong" witness, there won't be a retry.
| /// at/below the final execution head instead of scanning every item (which would | ||
| /// rebuild the global sweep this design deletes). Heights are captured at seed | ||
| /// time (`FetchItem::height`) — expiry never does per-item store lookups. | ||
| items_by_height: BTreeMap<BlockHeight, Vec<DataId>>, |
There was a problem hiding this comment.
is it possible draining based on this would evict a request for something that was needed for a higher height as well (eg, code which is by hash)?
There was a problem hiding this comment.
no. There's also a FetchItem::height field. If that's higher than the block height here we know the entry is stale.
| /// Contract accesses inconsistent with the witness's `contract_accesses_hash`. | ||
| AccessesInconsistent, |
There was a problem hiding this comment.
How does the code access list get requested in this model? Specifically how can we resolve the data dependency on block, witness, and code accesses together in this design?
There was a problem hiding this comment.
How does the code access list get requested in this model?
It is not. It is either pushed separately as an optimization, or included (always duplicated) as part of the witness.
There was a problem hiding this comment.
Which node(s) is supposed to do the pushing?
If we consider it an optimization, we need to think carefully whether malicious node(s) that are supposed to do the pushing can selectively delay some chunk endorsers from starting the work.
| /// per-`PeerId`/connection. This maps account→peer (Tier1 mapping) and pushes signals | ||
| /// into the network peer scorer, which owns actual banning. A producer with no |
There was a problem hiding this comment.
Does this generally mean this model does not support requesting data from non-validators? For example, during initial block sync pull for incoming receipts.
Or a node that tracks only some shards pulls which was offline for some time.
There was a problem hiding this comment.
I was assuming that a catching-up node can get everything it needs (execution results, receipts, code) from block/state sync without the distribution engine. So the engine is for live data only.
There was a problem hiding this comment.
If that is the case, then why do we need to have the requestor priorities? Maybe I didn't understand what you consider "engine"?
Non-spice block-sync code does not need to separately distribute receipts, since they are distributed automatically with the chunks / blocks.
I think an implementation for requesting this data will also need retries, knowing which nodes have the data, and avoiding requests from nodes sending bad data/non-responsive nodes, DoS protection, so it seems similar enough to the live case to consider them here.
There is also the case of a validator performing catchup after state sync for a shard that it needs to start validating in the next epoch. So maybe this is even an intermediate level of priority to respond to.
| pub(crate) global_priority_bytes: u64, | ||
| pub(crate) global_background_bytes: u64, |
There was a problem hiding this comment.
I wonder if priorty and background could be two separate instances of an actor or a similar design? Otherwise we'd need some kind of priority queue to ensure too many background requests (even within a budget) do not delay the validator push or responding to a pull request from a validator.
There was a problem hiding this comment.
I like to base on a single state machine. If we split into several actors we'll have to split the state. I also like the idea to let data distribution manager to define/alter the priority. So probably there's no way around having the priority queue.
There was a problem hiding this comment.
But that we can/should hide inside the spawner.
c5a6ada to
f9a686d
Compare
f9a686d to
dda90c7
Compare
2 participants
wip.
I am still only reasonably happy with
mod.rsanditem.rs. The rest has gaps. Working on those.The core idea: one place that knows what data I need, what I have, and who I got it from — a single per-node state machine (
SpiceDataManager) that is really a generic fetch engine.Witness, receipts, and code are the same "go fetch this" operation with three plug-in configs (what its ID is, who has it, how it's verified).
How data moves (push-primary, pull-recovery):
Availability is discovered, not announced.
We already know what we need (from the chain) and who produces it (structural).
The first slice that arrives is the signal it exists.
If you have none, you speculatively ask — and just wait for the push on recent blocks, since it's coming anyway.
Smarter retries.
The single 1-second timer becomes a per-item scheduler: don't request data that isn't produced yet (gate on execution progress), fetch fast when you know it exists, back off and fan out across a few high-score producers if stuck, and give up automatically once the block passes the final execution head.
Safety and fairness: