-
Notifications
You must be signed in to change notification settings - Fork 6.4k
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
Trace MultiGet Keys and CF_IDs to the trace file #8421
Conversation
@zhichao-cao has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
@@ -1880,6 +1880,16 @@ std::vector<Status> DBImpl::MultiGet( | |||
} | |||
#endif // NDEBUG | |||
|
|||
if (tracer_) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a few different MultiGet overloaded APIs. We should trace all of them.
@zhichao-cao has updated the pull request. You must reimport the pull request before landing. |
@zhichao-cao has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Can you update the test to also call the other 2 MultiGet APIs?
Sure, will update soon. |
15b403a
to
b39a15d
Compare
@zhichao-cao has updated the pull request. You must reimport the pull request before landing. |
@zhichao-cao has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
@zhichao-cao merged this pull request in 82a70e1. |
Tracing the MultiGet information including timestamp, keys, and CF_IDs to the trace file for analyzing and replay.
test plan: make check, add test to trace_analyzer_test