Repository navigation
feat(pushsync): placement check that verifies where chunks land - #625
gacevicljubisa wants to merge 4 commits into
Conversation
martinconic
left a comment
martinconic
left a comment
There was a problem hiding this comment.
Two blockers on the config side, both inline below. The check logic itself looks sound against the semantics of ethersphere/bee#5390; I'll follow up separately on smaller logic points (radius validation against CommittedDepth, deferred-mode polling, outcome counting).
| # of them empty, so that out-of-depth storing and shallow receipts can occur | ||
| bee-local-dns: | ||
| _inherit: "bee-local" | ||
| minimum-storage-radius: 3 |
There was a problem hiding this comment.
Blocker: this changes the cluster that all of Bee's CI runs on.
Bee's .github/workflows/beekeeper.yml deploys local-dns from beekeeper master's config/local.yaml and runs ci-pushsync-chunks, ci-retrieval, ci-manifest, ci-redundancy, ci-feed, ci-soc, ci-pss, etc. against it. bee-local-autotls, bootnode-local-dns-autotls and bee-local-ultralight-autotls also _inherit: "bee-local-dns", so local-dns-autotls (beekeeper-autotls.yml) gets this too.
At radius 3 there are 8 neighborhoods; 5 random full nodes leave at least 3 of them empty (~4 on average), so roughly half of all random chunks go to an empty neighborhood:
- Bee v2.8.2: the closest node stores them outside its AOR ("want self"), and the shallow receipt is swallowed after retries. CI still passes, just slower.
- After bee#5390: the forwarder returns
ErrOutOfDepthStoringand the origin doesn't store it either. Direct uploads fail withErrCouldNotSync, and deferred chunks are reportedCouldNotSyncand never sync. About half the chunks in every other CI check would fail.
Suggestion: leave bee-local-dns (and the bee count) untouched, and add a dedicated cluster (e.g. local-dns-placement) with its own bee-config (bee-local-placement with minimum-storage-radius: 3) for the placement checks.
(Side note: the radius on bootnode-local-dns below has no effect, because Bee doesn't store pushed chunks on a node in bootnode mode: FullNodeMode && !BootnodeMode in node.go.)
| bee-config: bee-local-dns | ||
| config: local-dns | ||
| count: 3 | ||
| count: 5 |
There was a problem hiding this comment.
Same concern as the minimum-storage-radius comment below: this bumps the node count of the local-dns cluster Bee's CI uses (more resources and time on every Bee PR, and it changes what ci-full-connectivity/ci-settlements etc. run against). Please keep it at 3 here and set the count on the dedicated placement cluster instead.
| - bootnode | ||
| timeout: 5m | ||
| type: pushsync | ||
| ci-pushsync-placement: |
There was a problem hiding this comment.
Blocker: on current Bee (v2.8.2), this check fails by design.
On v2.8.2, an upload to an empty neighborhood is stored outside any AOR ("want self" or a swallowed shallow receipt) and still reports success, so the check classifies it as false_success and fails. It can only pass against ethersphere/bee#5390, where those uploads fail (ErrCouldNotSync / CouldNotSync) and land in reported_failure. The metrics in placementBeeMetrics (bee_pushsync_out_of_depth_storing, bee_pusher_total_could_not_sync) also only exist in #5390 and silently read 0 otherwise.
The PR description is empty, so this dependency isn't documented anywhere. Please:
- state the dependency on bee#5390 in the PR description, and
- don't wire
ci-pushsync-placement/ci-pushsync-placement-deferredinto Bee's CI until #5390 is released.
2 participants
No description provided.