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

implement repo tombstone handler #340

Merged
merged 3 commits into from
Oct 2, 2023
Merged

implement repo tombstone handler #340

merged 3 commits into from
Oct 2, 2023

Conversation

whyrusleeping
Copy link
Collaborator

No description provided.

@ericvolp12
Copy link
Collaborator

ericvolp12 commented Sep 25, 2023

Do we wanna update the ListRepos endpoint to filter out taken down and tombstoned accounts?
Might be helpful to have conditional indexes that filter out those as well so paging over it is fast and easy.

@whyrusleeping
Copy link
Collaborator Author

yeah, probably also want to add checks in other places too now that im thinking about it

@bnewbold
Copy link
Collaborator

I can't really remember what the repo event stream semantics are supposed to be, and I can't find our old design notes on this. From looking at the PDS code, it gets emitted when the repo is deleted from that specific PDS. I don't think the semantics are "repo is deleted for all time" (that would be a PLC identity tombstone, which is separate).

I think what the BGS needs to do on tombstone from a PDS is just

  • delete all repo data from BGS, and not serve sync requests any more (this PR does that)
  • emit the tombstone event for downstream folks (this PR does that)

This PR currently also records the tombstone as a flag and prevents any future event processing. I don't think we need to do that: if an account deletes their PDS account, then a couple months later decides to re-create on the same or a new PDS, and pushes records/events, that is all fine and the BGS should start processing and re-emitting events.

@dholms
Copy link
Contributor

dholms commented Sep 26, 2023

I'm not sure we fully specced out the exact semantics of the tombstone

But my thinking is that it is different from "account migration", "takedown", or "temporarily stop indexing me".

Tombstone to me implies "account deletion with no expectation of return".

If a user was migrating from PDS A -> PDS B, PDS A would not send a tombstone event, it would send an "account_migration" event.

That being said, a did can return after being tombstoned (if for instance, the tombstone was sent incorrectly or by a malicious PDS). Therefore any flag should not be permanent.

If a did is tombstoned, future events from that DID, future events should be ignored if:

  • the DID does not exist
  • the PDS in the DID doc is not hosting the repo for the DID

However, if that repo is "back online", then the BGS should un-tombstone the DID & re-index it just as it would a new repo that it came across

@bnewbold
Copy link
Collaborator

Yeah, they way I would summarize it is that after a tombstone it should be like the account never existed, and any new info is as-if coming from a brand new account.

Identity caches should probably be busted/flushed, for example, so that on any future events there is a full resolution of DID and handle from scratch.

Copy link
Collaborator

@ericvolp12 ericvolp12 left a comment

Choose a reason for hiding this comment

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

LGTM, I'm gonna do a pass on error messages and status codes after this

@whyrusleeping whyrusleeping merged commit cf60dd2 into main Oct 2, 2023
6 checks passed
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