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 GC issue for concurrent directory reuse after deletion #601

Merged
merged 12 commits into from Sep 13, 2020

Conversation

julio-lopez
Copy link
Collaborator

No description provided.

// Options used during Environment Setup.
type Options struct {
NewRepositoryOptions func(*repo.NewRepositoryOptions)
OpenOptions func(*repo.Options)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The motivation is to be able to pass *repo.Options to inject time in another test harness.

@@ -100,6 +100,9 @@ func (e *Environment) Close(ctx context.Context, t *testing.T) {
}
}

// may need to remove the maintenance lock
os.Remove(e.configFile() + ".mlock")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For tests that run maintenance tasks, this file lingers around. Mmm, perhaps it should be removed on disconnect. Currently here to prevent the check below from failing a test.


// Run maintenance again
th.fakeTime.Advance(maintenance.DefaultParams().SnapshotGC.MinContentAge + time.Hour)
err = Run(ctx, th.Repository, maintenance.ModeFull, true)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Currently fails here.

@julio-lopez julio-lopez changed the title Test/gc dir reuse Test for directory reuse after GC Sep 11, 2020
err = Run(ctx, th.Repository, maintenance.ModeFull, true)
require.NoError(t, err)

info, err := r2.(*repo.DirectRepository).Content.ContentInfo(ctx, content.ID(s2.RootObjectID()))
Copy link
Contributor

@jkowalski jkowalski Sep 11, 2020

Choose a reason for hiding this comment

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

perhaps add comment saying we are intentionally interleaving snapshot and maintenance and not flushing in order to create dangling reference to a deleted content.

Copy link
Contributor

Choose a reason for hiding this comment

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

btw same exact thing will happen when you don't reuse a directory but a file:

create dir1/file1
create snaphot dir1
create dir2
move dir1/file1 to dir2/file2
delete snapshot dir1
delete dir1
snapshot dir2

@julio-lopez julio-lopez marked this pull request as ready for review September 13, 2020 01:03
@julio-lopez
Copy link
Collaborator Author

Test timeout on Darwin.

@julio-lopez julio-lopez merged commit 64b6018 into kopia:master Sep 13, 2020
@julio-lopez julio-lopez deleted the test/gc-dir-reuse branch September 13, 2020 02:28
@julio-lopez julio-lopez changed the title Test for directory reuse after GC Fix GC issue for concurrent directory reuse after deletion Sep 13, 2020
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

2 participants