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

fix: at startup, clean up left projects that failed to delete #672

Merged
merged 1 commit into from
May 30, 2024

Conversation

EvanHahn
Copy link
Contributor

If you start to leave a project and the operation fails partway, we now finish the next time you try to load that project.

For example, imagine the following scenario:

  1. The user presses the "leave project" button.
  2. We mark them as having left the project, but haven't yet deleted any data.
  3. Their phone runs out of battery.

Previously, the project state would be messed up in this scenario. Now, we finish the cleanup before returning the project.

Fixes #583.

If you start to leave a project and the operation fails partway, we now
finish the next time you try to load that project.

For example, imagine the following scenario:

1. The user presses the "leave project" button.
2. We mark them as having left the project, but haven't yet deleted any
   data.
3. Their phone runs out of battery.

Previously, the project state would be messed up in this scenario. Now,
we finish the cleanup before returning the project.

Fixes [#583].

[#583]: #583
(dataStore) => dataStore.namespace !== 'auth'
)

await Promise.all(dataStoresToUnlink.map((ds) => ds.close()))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to close earlier in the process now, because of the following race condition:

  1. We open a project.
  2. We close the writer core (see line 636 below).
  3. We start indexing the writer core (via MultiCoreIndexer). This fails because the writer core has been closed.

(This was theoretically a problem before. Someone could load a project and immediately leave it.)

@EvanHahn EvanHahn marked this pull request as ready for review May 23, 2024 18:56
@EvanHahn EvanHahn requested a review from achou11 May 23, 2024 18:56
test-e2e/project-leave.js Show resolved Hide resolved
@@ -345,4 +347,51 @@ test('leaving a project while disconnected', async (t) => {
)
})

test('partly-left projects are cleaned up on startup', async (t) => {
Copy link
Member

Choose a reason for hiding this comment

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

it'd be nice to confirm that data (on disk) is actually cleaned up in this scenario. not sure how complicated that would be to test though

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Opened #693 to test this. (Doesn't test this specific cleanup case; let me know if you want me to do that.)

@EvanHahn EvanHahn merged commit 2589842 into main May 30, 2024
7 checks passed
@EvanHahn EvanHahn deleted the evanhahn/583 branch May 30, 2024 22:05
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.

At startup, clean up left projects that failed to delete
2 participants