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.

Sign up for GitHub

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

Jump to bottom

Expose blob file information through the EventListener interface #8675

Closed
wants to merge 15 commits into from
Closed

Expose blob file information through the EventListener interface #8675

wants to merge 15 commits into from

Conversation

akankshamahajan15
Copy link
Contributor

@akankshamahajan15 akankshamahajan15 commented Aug 18, 2021 •
edited
Loading

Summary: 1. Extend FlushJobInfo and CompactionJobInfo with information about the blob files generated by flush/compaction jobs. This PR add two structures BlobFileInfo and BlobFileGarbageInfo that contains the required information of blob files.
2. Notify the creation and deletion of blob files through OnBlobFileCreationStarted, OnBlobFileCreated, and OnBlobFileDeleted.
3. Test OnFile*Finish operations notifications with Blob Files.
4. Log the blob file creation/deletion events through EventLogger in Log file.

Test Plan: Add new unit tests in listener_test

Reviewers:

Subscribers:

Tasks:

Tags:

@facebook-github-bot
Copy link
Contributor

@akankshamahajan15 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

1 similar comment
@facebook-github-bot
Copy link
Contributor

@akankshamahajan15 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

mrambacher
mrambacher reviewed Aug 19, 2021
View reviewed changes
db/blob/blob_file_builder_test.cc Outdated Show resolved Hide resolved
db/db_impl/db_impl_compaction_flush.cc Outdated Show resolved Hide resolved
include/rocksdb/listener.h Outdated Show resolved Hide resolved
db/db_impl/db_impl_files.cc Outdated Show resolved Hide resolved
ltamasi
ltamasi reviewed Aug 20, 2021
View reviewed changes
Copy link
Contributor

@ltamasi ltamasi 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 the PR @akankshamahajan15 ! Please some (mostly minor) comments below

db/blob/blob_file_builder.cc Outdated Show resolved Hide resolved
db/blob/blob_file_builder.cc Outdated Show resolved Hide resolved
db/blob/blob_file_builder.cc Outdated Show resolved Hide resolved
db/db_impl/db_impl_compaction_flush.cc Outdated Show resolved Hide resolved
db/db_impl/db_impl_compaction_flush.cc Outdated Show resolved Hide resolved
include/rocksdb/listener.h Outdated Show resolved Hide resolved
include/rocksdb/listener.h Outdated Show resolved Hide resolved
include/rocksdb/listener.h Show resolved Hide resolved
include/rocksdb/listener.h Outdated Show resolved Hide resolved
include/rocksdb/listener.h Outdated Show resolved Hide resolved
ltamasi
ltamasi reviewed Aug 20, 2021
View reviewed changes
include/rocksdb/listener.h Outdated Show resolved Hide resolved
@facebook-github-bot
Copy link
Contributor

@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing.

@facebook-github-bot
Copy link
Contributor

@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing.

@facebook-github-bot
Copy link
Contributor

@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing.

1 similar comment
@facebook-github-bot
Copy link
Contributor

@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing.

@akankshamahajan15 akankshamahajan15 marked this pull request as draft August 26, 2021 16:27
@facebook-github-bot
Copy link
Contributor

@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing.

1 similar comment
@facebook-github-bot
Copy link
Contributor

@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing.

@facebook-github-bot
Copy link
Contributor

@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing.

@facebook-github-bot
Copy link
Contributor

@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing.

@akankshamahajan15 akankshamahajan15 marked this pull request as ready for review August 27, 2021 03:18
@facebook-github-bot
Copy link
Contributor

@akankshamahajan15 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

1 similar comment
@facebook-github-bot
Copy link
Contributor

@akankshamahajan15 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing.

@facebook-github-bot
Copy link
Contributor

@akankshamahajan15 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing.

1 similar comment
@facebook-github-bot
Copy link
Contributor

@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing.

ltamasi
ltamasi reviewed Sep 16, 2021
View reviewed changes
HISTORY.md Outdated Show resolved Hide resolved
ltamasi
ltamasi approved these changes Sep 16, 2021
View reviewed changes
Copy link
Contributor

@ltamasi ltamasi left a comment

Choose a reason for hiding this comment

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

Thanks again @akankshamahajan15, this looks awesome!

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

@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing.

@facebook-github-bot
Copy link
Contributor

@akankshamahajan15 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@akankshamahajan15
Add Job-level information of blob files in Event listener
c88cf34 
Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
@akankshamahajan15
Add file level information to Event listener
b772abd 
Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
@akankshamahajan15
Add comments
e8a6e0b 
Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
@akankshamahajan15
Test OnFile* operations with Blob files
7135ed5 
Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
@akankshamahajan15
Fix build failure
7ff9030 
Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
@akankshamahajan15
Add constructors for intialization in listener.h
801b6f2 
Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
@akankshamahajan15
Moving listener notification to BlobFileCompletionCallback
e5ffc92 
Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
@akankshamahajan15
Addressed minor comments
cd3e3dc 
Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
@akankshamahajan15
Add blob file creation reason
905cc79 
Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
@akankshamahajan15
Fix build failure
7a05f1b 
Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
@akankshamahajan15
Log the blob file creation/deletion in EventLogger
8c55b2d 
Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
@akankshamahajan15
Minor changes
4aee073 
Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
@akankshamahajan15
Addressed comments
84787af 
Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
@akankshamahajan15
Update history.md and comments
ee3894b 
Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
@akankshamahajan15
Addressed comments related to dbname parameter and update history.md
21cca90 
Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
@facebook-github-bot
Copy link
Contributor

@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing.

@facebook-github-bot
Copy link
Contributor

@akankshamahajan15 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

1 similar comment
@facebook-github-bot
Copy link
Contributor

@akankshamahajan15 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@akankshamahajan15 merged this pull request in d6aa8c4.

yoori pushed a commit to yoori/rocksdb that referenced this pull request Nov 26, 2023
@akankshamahajan15
Expose blob file information through the EventListener interface (#8675)
3586f9e 
Summary:
1. Extend FlushJobInfo and CompactionJobInfo with information about the blob files generated by flush/compaction jobs. This PR add two structures BlobFileInfo and BlobFileGarbageInfo that contains the required information of blob files.
 2. Notify the creation and deletion of blob files through OnBlobFileCreationStarted, OnBlobFileCreated, and OnBlobFileDeleted.
 3. Test OnFile*Finish operations notifications with Blob Files.
 4. Log the blob file creation/deletion events through EventLogger in Log file.

Pull Request resolved: facebook/rocksdb#8675

Test Plan: Add new unit tests in listener_test

Reviewed By: ltamasi

Differential Revision: D30412613

Pulled By: akankshamahajan15

fbshipit-source-id: ca51b63c6e8c8d0485a38c503572bc5a82bd5d07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrambacher mrambacher mrambacher left review comments

@ltamasi ltamasi ltamasi approved these changes

Assignees
No one assigned
Labels
CLA Signed Merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@akankshamahajan15 @facebook-github-bot @mrambacher @ltamasi