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

Update IOTrace operations in stackable_db.h #7514

Closed

Conversation

akankshamahajan15
Copy link
Contributor

Summary: Update IOTrace operations in stackabledb.h and also trace few
other IO operations.

Test Plan: make check -j64

Reviewers:

Subscribers:

Tasks:

Tags:

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

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

Summary: Update IOTrace operations in stackabledb.h and also trace few
other IO operations.

Test Plan: make check -j64

Reviewers:

Subscribers:

Tasks:

Tags:
@facebook-github-bot
Copy link
Contributor

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

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

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

Copy link
Contributor

@anand1976 anand1976 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@mrambacher mrambacher left a comment

Choose a reason for hiding this comment

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

These changes follow the style of the existing code. I created #7504 to document some of the issues with that style.

LGTM, with the aforementioned caveat.

@facebook-github-bot
Copy link
Contributor

@akankshamahajan15 merged this pull request in 7b65666.

Copy link
Contributor

@mrambacher mrambacher left a comment

Choose a reason for hiding this comment

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

Are there any tests to cover this code?

@akankshamahajan15
Copy link
Contributor Author

Are there any tests to cover this code?

I added tests in io_tracer_test to cover testing of writing records in IOTracer and io_tracer_parser_test to cover writing the records through calling DB::StartIOTracer and then DB::EndIOTracer and parsing those records.

codingrhythm pushed a commit to SafetyCulture/rocksdb that referenced this pull request Mar 5, 2021
Summary:
Update IOTrace operations in stackabledb.h and also trace few
other IO operations.

Pull Request resolved: facebook#7514

Test Plan: make check -j64

Reviewed By: anand1976

Differential Revision: D24151202

Pulled By: akankshamahajan15

fbshipit-source-id: 112cd3d2041f8c6398b7b0ba1a783b8c93224d4a
levichen94 pushed a commit to bytedance/terarkdb that referenced this pull request Jun 18, 2021
Summary:
Update IOTrace operations in stackabledb.h and also trace few
other IO operations.

Pull Request resolved: facebook/rocksdb#7514

Test Plan: make check -j64

Reviewed By: anand1976

Differential Revision: D24151202

Pulled By: akankshamahajan15

fbshipit-source-id: 112cd3d2041f8c6398b7b0ba1a783b8c93224d4a
Signed-off-by: Changlong Chen <[email protected]>
mm304321141 pushed a commit to bytedance/terarkdb that referenced this pull request Jun 23, 2021
Summary:
Update IOTrace operations in stackabledb.h and also trace few
other IO operations.

Pull Request resolved: facebook/rocksdb#7514

Test Plan: make check -j64

Reviewed By: anand1976

Differential Revision: D24151202

Pulled By: akankshamahajan15

fbshipit-source-id: 112cd3d2041f8c6398b7b0ba1a783b8c93224d4a
Signed-off-by: Changlong Chen <[email protected]>
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.

4 participants