Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update documents-instances.md #170

Merged
merged 2 commits into from
Jun 1, 2022
Merged

Update documents-instances.md #170

merged 2 commits into from
Jun 1, 2022

Conversation

sandreae
Copy link
Member

Include changes from p2panda/p2panda#319

Copy link
Member

@cafca cafca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may have been a bit vague when I asked whether that PR should have a handbook update. What I was thinking about is the question of what a document view looks like for deleted documents. Currently we have

a document view has a value for all fields that are defined by its document's schema

After p2panda/p2panda#319 deleted documents still have a view id but no field values. As the behavior of deleted documents still feel a bit underspecified in p2panda I would like to be precise in that area if possible.

What do you think about

  • a document view has a value for all fields that are defined by its document's schema unless the document has been deleted
  • a deleted document's view has no values for its fields

open questions:

  • what happens when a deleted document receives update operations? as it is deleted its fields have no value by definition but does the view id still change? or is it not possible to apply any operation to a deleted document?
  • what if there are multiple delete operations? does each one create a new document view id or is the document "frozen" after the first one?

@@ -31,4 +31,6 @@ id: documents
- if a limited size identifier is required, the document view's _hash id_ can be used
- to construct the hash id sort the graph tips of a document view id, concatenate their byte values and hash the result using YASMF.
- a document view has a value for all fields that are defined by its document's schema
- a document view has the _operation id_ for the _operation_ which transported each fields value
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels a bit like an implementation feature, doesn't it? As this section is describing the concept of document views in general I would phrase it a bit differently if we feel it's important to include it here, e.g.

implementations should ensure that for every field of a document view it is possible to identify the operation in which that field was last modified

Even then I don't think we actually need this requirement for p2panda in general.

@adzialocha
Copy link
Member

adzialocha commented May 26, 2022

What do you think about

  • a document view has a value for all fields that are defined by its document's schema unless the document has been deleted
  • a deleted document's view has no values for its fields

Super, this sounds good to me! 👍

open questions:

  • what happens when a deleted document receives update operations? as it is deleted its fields have no value by definition but does the view id still change? or is it not possible to apply any operation to a deleted document?
  • what if there are multiple delete operations? does each one create a new document view id or is the document "frozen" after the first one?

Good questions!

Maybe the rule should be: Consider a document deleted as soon as any DELETE operation was sighted during materialization.

Another rule: It's not allowed to publish a DELETE or UPDATE operation pointing at a DELETE operation as the previous one.

With these rules there can't be two DELETE operations within one operation graph branch, neither an UPDATE after a DELETE - but in separte, multiple branches it might still happen. In this scenario our winning hash will be sorted last in the topological ordering meaning that if some DELETE or UPDATE branches arrive later, they might change the order and indeed add another document view id.

We can't ignore future operations operations and freeze the id, as that would lead to indeterministic behavior across nodes (nodes might wrongly arrive at different state even though they have the same operations, they just came in in different order). Also, it should not be a problem, as document view ids is something for pinned relations only as far as I can tell right now?

It's an interesting behavior as "deleted" documents can somehow still be "alive". It doesn't change the document itself, neither it affects relations, so I think that all is good here, they will just be deleted in any case. Pinned relations are different though and might still go on by pointing at a branch of the operation graph which never seen a DELETE operation ever, even though one exists somewhere else in another branch .. pinned relations are immutable and we can't suddenly make data "deleted" retroactively in that case.

We can and maybe should add a third rule which should reject new entries from a client if a node knows the related document got deleted, but that's mainly just a nice warning, of course if the node didn't know that yet we might still arrive at that above scenario anyhow.

I guess with this you could engineer a scenario where a schema consists of pinned relations and users might use that to keep documents alive, even though they are officially deleted.

@adzialocha
Copy link
Member

This is a little bit of a rabbit hole, happy to get cafcas additions in this PR and discuss this in a separate boiling pot issue 😅

@sandreae
Copy link
Member Author

@cafca yeh, sorry, I got the wrong end of the stick on this handbook update. I like all your suggestions for changes above, also about scope of what is in the spec, what is implementation specific/optional. I'll make all these changes first thing next week 👏

@adzialocha
Copy link
Member

This is a little bit of a rabbit hole, happy to get cafcas additions in this PR and discuss this in a separate boiling pot issue sweat_smile

Okay, I moved this discussion here: #172

@sandreae sandreae requested a review from cafca June 1, 2022 09:16
@adzialocha adzialocha merged commit 51effb2 into main Jun 1, 2022
@adzialocha adzialocha deleted the update-document-view-docs branch June 1, 2022 09:55
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.

None yet

3 participants