Skip to content

Conversation

frankmcsherry
Copy link
Member

This PR adds an implementation of Lattice for timely's Antichain type. We are unable to add one for AntichainRef as we cannot produce Self types for it without borrowed data. Perhaps a more exotic implementation would unify this stuff, but for the moment just trying out this limited form.

The Timestamp trait is removed from Lattice. It existed in order to inherit the minimum() method, though Lattice itself does not require this. This could break some external code, but the change was relatively recent (post 0.11, I believe).

The PR only introduces the implementation, and does not tidy anything in the code yet. There should probably be a few passes of that, and potentially conversion of some Vec<T> types into Antichain<T> types where it is known that they should be such (e.g. probably in Description?).

Note: this will be broken until TimelyDataflow/timely-dataflow#322 lands in the timely repo.

cc @LorenzSelv

@frankmcsherry
Copy link
Member Author

This became substantially wordier, changing many Vec<T> types into Antichain<T> and many &[T] into AntichainRef<T>. There are certainly remaining moments where changes should be made, but several logical complications were simplified down to PartialOrder::less_equal(foo, bar).

@LorenzSelv, do you have a sense for how breaking these changes are for you / ETHZ folks? I can ping @utaal too (I guess mentioning him does that). The main externally visible change, I think, is that rather than using &[Time] to pilot around traces, compaction, etc., one uses AntichainRef<Time>, and you can always get one from a slice with AntichainRef::new.

No rush to merge this, especially if you all find it more disruptive than helpful.

@LorenzSelv
Copy link
Contributor

At the moment we are (I am) not using differential at all: we recently decided to port the trace datastructure (nice way to say copy-pasted it) from differential to a different repository where we are implementing the fault-tolerance mechanism as library code on top of timely 0.11.

Before, we were depending on a different fork of differential, so it wouldn't have caused problems anyway.

Not sure if anyone else was using it except for me.

@utaal
Copy link
Member

utaal commented Mar 13, 2020

This looks good to me, and I agree we don't expect disruption. We may decide to port durability to the new trace implementation, but that would be a larger exercise nonetheless.

Thanks for checking, no blockers on merging on our side.

@frankmcsherry frankmcsherry merged commit 42f3113 into master Apr 29, 2020
@antiguru antiguru deleted the antichain_lattice branch February 13, 2025 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants