Every self-fence reports `NodeLeaseExpired` so a replaced/removed lease reads as a TTL expiry
sandros94 opened ·
Same context as issue #182: benchmarking v0.4.0 (`a52f990`) directly, findings validated by an agent (as I'm not fluent enough in ruts, yet), all offline.
`HaltReason` has a single variant ([types.rs#L239-L244](https://github.com/denoland/celld/blob/a52f9905425bc41134d817694bdc2c50bcc5e856/crates/logic/types.rs#L239-L244)) and `fence_node` emits it on every path ([lib.rs#L2810-L2817](https://github.com/denoland/celld/blob/a52f9905425bc41134d817694bdc2c50bcc5e856/crates/logic/lib.rs#L2810-L2817)), so the immediate fence on a missing/different read-back ([lib.rs#L2087-L2092](https://github.com/denoland/celld/blob/a52f9905425bc41134d817694bdc2c50bcc5e856/crates/logic/lib.rs#L2087-L2092)) logs the same line as the watchdog ([actor.rs#L3734-L3740](https://github.com/denoland/celld/blob/a52f9905425bc41134d817694bdc2c50bcc5e856/crates/celld/actor.rs#L3734-L3740)).
### Reproduction
just 60s TTL, delete the node's own lease object after acquisition, wait for the next renewal.
``` 11:59:04.263 acquire applied; expires_ms=1788264004255 11:59:05.457 lease object removed out of band 11:59:27.595 renewal started; prior_authority_headroom_ms=36660 11:59:27.596 renewal rejected 11:59:27.596 SELF-FENCE: node lease not renewed within TTL — halting (exit 3) ```
Fencing at once is right, the record did move. It's the reported cause that is off: 36.7s of the TTL were still there, and [`docs/guarantees.md`](https://github.com/denoland/celld/blob/a52f9905425bc41134d817694bdc2c50bcc5e856/docs/guarantees.md#L208-L224) says the line names the cause.
### Additional
Related wish while I'm here: when a node fences there's no way to tell from its own output whether the read-back came back missing, different, or as a read error. The S3 side is only visible if you put something in front of the store, which is a lot of setup for a diagnosis. Some built-in request-level logging for the store adapter, opt-in, even just the outcome class on the lease path, would make these self-diagnosable.
WDYT?
bartlomiejuassigned bartlomieju2 months ago on GitHub
- BA
This is now fixed and will be released in the new version.
bartlomiejureferenced fd7b96b993f52 months ago on GitHub
bartlomiejureferenced b61356b29deb2 months ago on GitHub
bartlomiejureferenced 7ac68e0bd2a72 months ago on GitHub
bartlomiejureferenced 10cb1303dac72 months ago on GitHub