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

Support for Merge in Integrated BlobDB with base values #8292

Closed

akankshamahajan15 wants to merge 10 commits into facebook:master from akankshamahajan15:blob_db

Contributor

akankshamahajan15 commented May 13, 2021 •

edited

Loading

Summary: This PR add support for Merge operation in Integrated BlobDB with base values(i.e DB::Put). Merged values can be retrieved through DB::Get, DB::MultiGet, DB::GetMergeOperands and Iterator operation.

Test Plan: Add new unit tests

Reviewers:

Subscribers:

Tasks:

Tags:

facebook-github-bot added the CLA Signed label

akankshamahajan15 force-pushed the blob_db branch 3 times, most recently from fa7ef06 to e582a81 Compare

May 13, 2021 05:09

akankshamahajan15 requested a review from ltamasi

May 13, 2021 16:57

Contributor

facebook-github-bot commented May 13, 2021

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

ltamasi reviewed

View reviewed changes

Contributor

ltamasi left a comment

Thanks for the PR @akankshamahajan15! This is awesome

db/blob/db_blob_index_test.cc Outdated Show resolved Hide resolved

db/blob/db_blob_index_test.cc Outdated Show resolved Hide resolved

db/blob/db_blob_index_test.cc Outdated Show resolved Hide resolved

db/blob/db_blob_index_test.cc Outdated Show resolved Hide resolved

db/blob/db_blob_index_test.cc Outdated Show resolved Hide resolved

db/db_iter.cc Outdated Show resolved Hide resolved

db/db_iter.cc Outdated Show resolved Hide resolved

db/version_set.cc Outdated Show resolved Hide resolved

table/get_context.cc Outdated Show resolved Hide resolved

akankshamahajan15 force-pushed the blob_db branch from e582a81 to f3c2581 Compare

June 7, 2021 17:31

Contributor

facebook-github-bot commented Jun 7, 2021

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

1 similar comment

Contributor

facebook-github-bot commented Jun 7, 2021

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

akankshamahajan15 force-pushed the blob_db branch from 7719d6d to 29cf180 Compare

June 8, 2021 00:15

Contributor

facebook-github-bot commented Jun 8, 2021

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

akankshamahajan15 force-pushed the blob_db branch from 29cf180 to 9a1a28d Compare

June 8, 2021 18:35

Contributor

facebook-github-bot commented Jun 8, 2021

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

akankshamahajan15 force-pushed the blob_db branch from 9a1a28d to d111120 Compare

June 9, 2021 01:23

Contributor

facebook-github-bot commented Jun 9, 2021

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

akankshamahajan15 force-pushed the blob_db branch from d111120 to 10233b2 Compare

June 9, 2021 01:26

Contributor

facebook-github-bot commented Jun 9, 2021

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

akankshamahajan15 force-pushed the blob_db branch from 10233b2 to 9928766 Compare

June 9, 2021 18:24

Contributor

facebook-github-bot commented Jun 9, 2021

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

Contributor

facebook-github-bot commented Jun 9, 2021

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

akankshamahajan15 requested a review from ltamasi

June 9, 2021 20:16

ltamasi reviewed

View reviewed changes

Contributor

ltamasi left a comment

Thanks again @akankshamahajan15 ! The patch looks good to me, just some minor comments

db/blob/blob_file_merge_callback.cc Outdated Show resolved Hide resolved

db/blob/blob_file_merge_callback.cc Outdated Show resolved Hide resolved

db/blob/blob_file_merge_callback.h Outdated Show resolved Hide resolved

db/blob/db_blob_index_test.cc Outdated Show resolved Hide resolved

db/version_set.cc Outdated Show resolved Hide resolved

db/version_set.cc Outdated Show resolved Hide resolved

table/get_context.cc Outdated Show resolved Hide resolved

akankshamahajan15 added 7 commits

June 9, 2021 18:15


Dedup merge code in GetContext

fe4f450

Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:


Support Merge with base values for Integrated BlobDB Get

51f36e9

Summary:

Test Plan: Add new unit test

Reviewers:

Subscribers:

Tasks:

Tags:


Suppport Merge with base values for Integrated BlobDB MultiGet

f212f78

Summary:

Test Plan: Add new unit test

Reviewers:

Subscribers:

Tasks:

Tags:


Dedup merge code in DBIter

1a30c01

Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:


Support Merge with base values for Integrated BlobDB for Iterator

aec07a1

Summary:

Test Plan: Add new test

Reviewers:

Subscribers:

Tasks:

Tags:


Addressed comments

a52dcd5

Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:


Addressed comments

bb69ce0

Summary: Add Callback class to get the blob value and support
DB::GetMergeOperands

Test Plan: Add unit test

Reviewers:

Subscribers:

Tasks:

Tags:


Fix CircleCI failures

f8a7d27

Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

akankshamahajan15 force-pushed the blob_db branch from 9928766 to d0084ca Compare

June 10, 2021 01:16

Contributor

facebook-github-bot commented Jun 10, 2021

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


Addressed comments

6bb45ed

Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

akankshamahajan15 force-pushed the blob_db branch from d0084ca to 6bb45ed Compare

June 10, 2021 01:23

Contributor

facebook-github-bot commented Jun 10, 2021

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

ltamasi approved these changes

View reviewed changes

db/blob/blob_fetcher.cc Outdated Show resolved Hide resolved

db/blob/db_blob_index_test.cc Outdated Show resolved Hide resolved

table/get_context.h Outdated Show resolved Hide resolved


Addressed minor comments

ae03c3d

Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Contributor

facebook-github-bot commented Jun 10, 2021

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

Contributor

facebook-github-bot commented Jun 10, 2021

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

facebook-github-bot closed this in

3897ce3

Contributor

facebook-github-bot commented Jun 10, 2021

@akankshamahajan15 merged this pull request in 3897ce3.

facebook-github-bot added the Merged label

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