Creating a new database table calledYDocSnapshot that stores repo snapshots at the request of users.
The implement features a strict database consistency, once receiving a request, it reads theyDocBlob in theRepo table and inserts it intoYDocSnapshot. It avoids conflicts betweenRepo.yDocBlob in the event of network interruption.
"Rename/restore/delete" operations will be added after discussion.

Test

Attach the shell of theapi docker container
Runnpx prisma migrate dev
Restartapi container

Sen Wang added2 commits

August 17, 2023 19:35

Add repo snapshots

1e3c096

refine gql and clean up

1b63040

senwang86 requested a review fromlihebi

August 18, 2023 04:32

lihebi reviewed

Aug 18, 2023

View reviewed changes

api/src/typedefs.ts Outdated

		repoId: String
		createdAt: String
		message: String
		yDocBlob: String

Copy link

Collaborator

Choose a reason for hiding this comment

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

This blob doesn’t need to be sent to the front end.

Copy link

CollaboratorAuthor

Choose a reason for hiding this comment

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

This blob doesn’t need to be sent to the front end.

SG, I was thinking about the "restore" operation, I am not exactly sure if it's better to pass theyDocBlob in a single query.

Copy link

Collaborator

Choose a reason for hiding this comment

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

Restore should probably happen in the backend yjs server, and front end will retrieve from yjs. I’ll think about this.

lihebi reviewed

Aug 18, 2023

View reviewed changes

api/src/resolver_repo.ts Outdated

		id: await nanoid(),
		yDocBlob: repo.yDocBlob,
		message: message,
		repoId: repoId,

Copy link

Collaborator

Choose a reason for hiding this comment

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

Use theconnect clause to connect the two tables.

lihebi reviewed

Aug 18, 2023

View reviewed changes

api/src/resolver_repo.ts Outdated

		*/
		async function getRepoSnapshots(_, { repoId }) {
		const snapshots = await prisma.yDocSnapshot.findMany({
		where: { repoId: repoId },

Copy link

Collaborator

Choose a reason for hiding this comment

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

{repo: {id: repoId}}

lihebi reviewed

Aug 18, 2023

View reviewed changes

api/prisma/schema.prisma Outdated

		createdAt DateTime @default(now())
		message String?
		yDocBlob Bytes?
		Repo Repo? @relation(fields: [repoId], references: [id])

Copy link

Collaborator

Choose a reason for hiding this comment

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

Lower case “repo” field name.

Sen Wang added3 commits

August 18, 2023 10:11

Address comments

4ff91b0

Add deleted filed in YDocSnapshot

880c04c

clean up

1dd7c37

lihebi reviewed

Aug 18, 2023

View reviewed changes

api/prisma/schema.prisma Outdated

		createdAt DateTime @default(now())
		message String?
		yDocBlob Bytes?
		deleted Boolean @default(false)

Copy link

Collaborator

Choose a reason for hiding this comment

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

I don't want to adddeleted here since it adds complexity. We could reply on DB backup if we really want to ensure data safety.

Copy link

CollaboratorAuthor

Choose a reason for hiding this comment

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

I am not so familiar with DB backup, any reference for its pros and cons? I am considering a scenario where users might decide to delete a snapshot, but then want to undo the deletion later. Furthermore, if there are 1,000 users performing these operations at different times, can DB backups gracefully address this scenario?

Copy link

Collaborator

lihebiAug 18, 2023

Choose a reason for hiding this comment

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

I'd skip this scenario for now. If the user deletes it, it is deleted.

Copy link

CollaboratorAuthor

senwang86Aug 18, 2023

Choose a reason for hiding this comment

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

I'd skip this scenario for now. If the user deletes it, it is deleted.

OK, let me revert the change.

lihebi reviewed

Aug 18, 2023

View reviewed changes

api/prisma/schema.prisma Outdated

		message String?
		yDocBlob Bytes?
		deleted Boolean @default(false)
		repo Repo? @relation(fields: [repoId], references: [id])

Copy link

Collaborator

lihebiAug 18, 2023•
edited
Loading

Choose a reason for hiding this comment

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

yDocBlob and repo/repoId sound to be required, not optional. I.e.,

   yDocBlob Bytes   repo: Repo   repoId: String

Sen Wang added2 commits

August 18, 2023 13:16

Revert "Add deleted filed in YDocSnapshot"

2ece924

This reverts commit880c04c.

Address comments

edb7d0f

lihebi reviewed

Aug 18, 2023

View reviewed changes

api/prisma/schema.prisma Outdated

Comment on lines 74 to 77

		yDocBlob Bytes?
		repo Repo? @relation(fields: [repoId], references: [id])
		repoId String?
		yDocBlob Bytes
		repo Repo @relation(fields: [repoId], references: [id])
		repoId String
		}

Copy link

Collaborator

lihebiAug 18, 2023

Choose a reason for hiding this comment

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

You need to generate a new DB migration to reflect this change.

Copy link

CollaboratorAuthor

senwang86Aug 18, 2023

Choose a reason for hiding this comment

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

bb35923 addes the prisma migration.

Update_prisma_schema

bb35923

Copy link

Collaborator

lihebi commentedAug 19, 2023

One last comment: merge the two migrations into one? I.e. remove the migrations and re-generate one.

clean prisma migration sql

d291948

Copy link

CollaboratorAuthor

senwang86 commentedAug 19, 2023

One last comment: merge the two migrations into one? I.e. remove the migrations and re-generate one.

d291948 address this.

Copy link

Collaborator

lihebi commentedAug 19, 2023

Awesome, thanks!

Copy link

Collaborator

lihebi commentedAug 20, 2023

Since this PR involves DB schema change, I'd like to ensure that the restoring would work under this design before merging.

I was thinking about how to restore a snapshot. Two methods:

Option 1 (not ideal): Directly replacing the ydoc to a previous state. This isn't ideal, as the peers see a different yDoc with different history, impossible to sync.
Option 2 (ideal): Add a new "restoring" changeset on top of the latest ydoc. This should be the ideal way.

In terms of implementation, I found that Yjs has a"Y.Snapshot". So instead of storing a whole YDocBlob into snapshots, we could store a Y.Snapshot binary and use Y.snapshot APIs for restoring. But this snapshot seems to only support method 1, which is not ideal.

To support method 2, we might need to implement a "restoring" changeset, like mentionedhere.

I'll need some time to think about it before we merge this PR and work on restoring.

Merge branch 'main' into UI_repo_snapshots

b3c03e0

Copy link

CollaboratorAuthor

senwang86 commentedAug 30, 2023•
edited
Loading

Realized that theYDocSnapshot needs a field ofuserId to separate the snapshots among various collaborators. WDYT,@lihebi ?

Copy link

Collaborator

lihebi commentedAug 30, 2023

I don't think so. The snapshot is associated with the repo. Each repo has only one line of snapshots, regardless of which user triggered it.

Copy link

CollaboratorAuthor

senwang86 commentedAug 31, 2023

I don't think so. The snapshot is associated with the repo. Each repo has only one line of snapshots, regardless of which user triggered it.

Will the snapshot list show snapshots from all other collaborators?

Copy link

Collaborator

lihebi commentedAug 31, 2023

Yes. There's only one snapshot line for the repo; everyone sees and operates on it. Keep it simple.

Copy link

Collaborator

lihebi commentedAug 31, 2023

There are more things to consider.

I've been thinking about how to make the revision system to:

play well with the Git system and
to be able to function on scopes

Also, the revision system code wouldn't need to be included in the public-facing desktop app. It would be exclusive to SaaS app, meaning the code would be in another repo.

Labels

None yet

2 participants

Movatterモバイル変換

[UI] Support repo snapshots#462

Are you sure you want to change the base?

[UI] Support repo snapshots#462

Uh oh!

Conversation

senwang86 commentedAug 18, 2023

Summary

Test

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lihebiAug 18, 2023• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lihebi commentedAug 19, 2023

Uh oh!

senwang86 commentedAug 19, 2023

Uh oh!

lihebi commentedAug 19, 2023

Uh oh!

lihebi commentedAug 20, 2023

Uh oh!

senwang86 commentedAug 30, 2023• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

lihebi commentedAug 30, 2023

Uh oh!

senwang86 commentedAug 31, 2023

Uh oh!

lihebi commentedAug 31, 2023

Uh oh!

lihebi commentedAug 31, 2023

Uh oh!

Uh oh!

lihebiAug 18, 2023•
edited
Loading

senwang86 commentedAug 30, 2023•
edited
Loading