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 orphaned files in SstFileManager #13015

Closed
wants to merge 1 commit into from

Conversation

nickbrekhus
Copy link

Summary: Close()ing a database now releases tracked files in SstFileManager. Previously this space would be leaked until the database was later reopened.

Differential Revision: D62590773

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D62590773

Copy link
Contributor

@jowlyzhang jowlyzhang left a comment

Choose a reason for hiding this comment

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

Thanks for adding this feature. This is a great addition. Do you mind also add a history entry in the unreleased_history directory.

cc @anand1976 this feels like a better solution to handle the leaking space from a SstFileManager's perspective issue. Compared to the alternative idea we discussed to implement similar things in DestroyDB. WDYT?

db/db_impl/db_impl.cc Outdated Show resolved Hide resolved
db/db_impl/db_impl.cc Outdated Show resolved Hide resolved
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D62590773

nickbrekhus pushed a commit to nickbrekhus/rocksdb that referenced this pull request Sep 16, 2024
Summary:
Pull Request resolved: facebook#13015

`Close()`ing a database now releases tracked files in `SstFileManager`. Previously this space would be leaked until the database was later reopened.

Differential Revision: D62590773
@jowlyzhang
Copy link
Contributor

There is a formatter inside of rocksdb repo, you can run make format inside of it to get it happy.

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D62590773

nickbrekhus pushed a commit to nickbrekhus/rocksdb that referenced this pull request Sep 17, 2024
Summary:
Pull Request resolved: facebook#13015

`Close()`ing a database now releases tracked files in `SstFileManager`. Previously this space would be leaked until the database was later reopened.

Differential Revision: D62590773
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D62590773

nickbrekhus pushed a commit to nickbrekhus/rocksdb that referenced this pull request Sep 17, 2024
Summary:
Pull Request resolved: facebook#13015

`Close()`ing a database now releases tracked files in `SstFileManager`. Previously this space would be leaked until the database was later reopened.

Differential Revision: D62590773
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D62590773

nickbrekhus pushed a commit to nickbrekhus/rocksdb that referenced this pull request Sep 17, 2024
Summary:
Pull Request resolved: facebook#13015

`Close()`ing a database now releases tracked files in `SstFileManager`. Previously this space would be leaked until the database was later reopened.

Differential Revision: D62590773
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D62590773

nickbrekhus pushed a commit to nickbrekhus/rocksdb that referenced this pull request Sep 18, 2024
Summary:
Pull Request resolved: facebook#13015

`Close()`ing a database now releases tracked files in `SstFileManager`. Previously this space would be leaked until the database was later reopened.

Differential Revision: D62590773
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D62590773

nickbrekhus pushed a commit to nickbrekhus/rocksdb that referenced this pull request Sep 18, 2024
Summary:
Pull Request resolved: facebook#13015

`Close()`ing a database now releases tracked files in `SstFileManager`. Previously this space would be leaked until the database was later reopened.

Differential Revision: D62590773
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D62590773

nickbrekhus pushed a commit to nickbrekhus/rocksdb that referenced this pull request Sep 18, 2024
Summary:
Pull Request resolved: facebook#13015

`Close()`ing a database now releases tracked files in `SstFileManager`. Previously this space would be leaked until the database was later reopened.

Differential Revision: D62590773
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D62590773

nickbrekhus pushed a commit to nickbrekhus/rocksdb that referenced this pull request Sep 18, 2024
Summary:
Pull Request resolved: facebook#13015

`Close()`ing a database now releases tracked files in `SstFileManager`. Previously this space would be leaked until the database was later reopened.

Differential Revision: D62590773
Copy link
Contributor

@jowlyzhang jowlyzhang left a comment

Choose a reason for hiding this comment

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

Thanks for adding this change! LGTM

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D62590773

nickbrekhus pushed a commit to nickbrekhus/rocksdb that referenced this pull request Sep 18, 2024
Summary:
Pull Request resolved: facebook#13015

`Close()`ing a database now releases tracked files in `SstFileManager`. Previously this space would be leaked until the database was later reopened.

Reviewed By: jowlyzhang

Differential Revision: D62590773
Summary:
Pull Request resolved: facebook#13015

`Close()`ing a database now releases tracked files in `SstFileManager`. Previously this space would be leaked until the database was later reopened.

Reviewed By: jowlyzhang

Differential Revision: D62590773
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D62590773

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 0611eb5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants