feat: overlay type definitions and operations #648
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
rphmeier
force-pushed
the
rh-overlay-definition-and-ops
branch
from
15a022c to
3479f60
Compare
rphmeier
force-pushed
the
rh-overlay-definition-and-ops
branch
from
3479f60 to
735bb7f
Compare
| } | ||
|
|
||
| impl Index { | ||
| // Prune all items with a sequence number less than the minimum |
There was a problem hiding this comment.
missing "." at the end?
| #[derive(Debug, Clone, Copy, PartialEq)] | ||
| pub struct InvalidAncestors; | ||
|
|
||
| /// A live overlay which is being used as a parent. |
| self.pages_by_seqn.push_front((seqn, key)); | ||
| break; | ||
| } | ||
| Some((seqn, key)) => { |
There was a problem hiding this comment.
I suggest to change key to page_id, less confusing
| .filter(|&got_seqn| got_seqn != seqn && got_seqn >= min) | ||
| { | ||
| // key has been updated since this point. reinsert. | ||
| self.pages.insert(key, got_seqn); |
There was a problem hiding this comment.
Same as above, probably related to a copy-paste being the same code as values_by_seqn pruning
| //! | ||
| //! However, creating a new [`LiveOverlay`] requires the user to provide strong references to each | ||
| //! of the ancestors which are still alive. It is still the user's responsibility to ensure | ||
| //! that all live ancestors are provided, or else data will go silently missing. |
There was a problem hiding this comment.
I struggle to see how a user can create a chain of Overlay with some missing Overlay, leading to silently miss out on some data. I have a some ideas based on how you envisioned the usage in the follow-up pr.
The user will be able to start a session and update NOMT with an Overlay as an output (first session created, so no overlays are available to be submitted). From now on, new overlays can be created again from the same previous state or on top of fresh new overlays. Each new overlay will contain a vector of weak references to all ancestors, making two things possible:
- Upon the creation of
LiveOverlay, the provided Overlay iterator is checked against the vector of weak references contained in the parent of the new overlay. - During the commit of an overlay, some logic could be added to ensure that an overlay is not committed twice or that all ancestors have already been committed. Something like storing a vector of all weak references to childs of an overlay could make this possible (modulo some efficiency, I didn't think about yet to this aspect)
That is to say, the comment seems not strictly correct to me, the user will probably be able to not commit an overlay and thus may silently lose some data (but probably this could be solved in some way). However, they will not be able to silently create an overlay on top of a broken chain of overlays, because an error will be thrown as a result.
pepyakin
force-pushed
the
graphite-base/648
branch
from
8b0ef5c to
3c6c0c7
Compare
pepyakin
force-pushed
the
rh-overlay-definition-and-ops
branch
from
735bb7f to
57178b0
Compare
pepyakin
force-pushed
the
rh-overlay-definition-and-ops
branch
from
57178b0 to
56e5220
Compare
This defines the
Overlaytypes. I tried to make it passably fast at an algorithmic level and will follow up with integrations!