Fixes recovery serde #143
Fixes recovery serde #143
Conversation
Makes all types for recovery serialization and deserialization explicit and fixes them to match each other for each recoverable operator. This will fix panics during recovery loading. This was also fixed in #137 but I'm breaking it out here separately because I keep discoverying smaller recovery bugs and am fixing them separately.
| let acc = resume_acc_bytes.map(|resume_acc_bytes| resume_acc_bytes.de()); | ||
| let acc = resume_acc_bytes | ||
| .map(StateBytes::de::<Option<TdPyAny>>) | ||
| .flatten(); |
There was a problem hiding this comment.
Why wasn't the flatten() call needed before?
There was a problem hiding this comment.
It was actually needed, hence the panic, but type inference made the compiler assume the wrong types since it can't know what's in those bytes.
The previous version without the flatten was causing the compiler to infer the incorrect type for the deserialization step. So for reduce window, snapshot was serializing with StateBytes::ser(&self.acc) which was inferred (correctly) to be StateBytes::ser::<Option<TdPyAny>>(&self.acc) but the builder was deserializing with resume_acc_bytes.map(|resume_acc_bytes| resume_acc_bytes.de()) which was inferred to be resume_acc_bytes.map(|resume_acc_bytes| resume_acc_bytes.de::<TdPyAny>()) but by (bad) luck the final type of acc was correct, so it was like cool this must be correct.
By making the deserialization type explicit and match the serialization type, now if you forget the flatten it's a type error.
Makes all types for recovery serialization and deserialization
explicit and fixes them to match each other for each recoverable
operator.
This will fix panics during recovery loading.
This was also fixed in #137
but I'm breaking it out here separately because I keep discoverying
smaller recovery bugs and am fixing them separately.