-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
feat: Add query trace TaskRunner #11927
Conversation
✅ Deploy Preview for meta-velox canceled.
|
6d0fbe2
to
ce18de9
Compare
d418eb8
to
56befee
Compare
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.
Can you add PR description? Thanks!
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.
@duanmeng LGTM % minors. Thanks!
@@ -325,6 +327,89 @@ TEST_F(HashJoinReplayerTest, partialDriverIds) { | |||
faultyFs->clearFileFaultInjections(); | |||
} | |||
|
|||
TEST_F(HashJoinReplayerTest, runner) { |
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.
Add runner test case for all the other operator types in followup? Thanks!
velox/tool/trace/TraceTaskRunner.h
Outdated
static std::shared_ptr<RowVector> copy( | ||
const std::vector<RowVectorPtr>& results); | ||
|
||
inline static std::atomic<uint64_t> cursorQueryId_ = 0; |
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.
s/cursorQueryId_/traceReplayQueryId_/
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.
It is unused, I've removed it.
56befee
to
f67fa34
Compare
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.
@duanmeng Thanks for the update!
@xiaoxmeng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
@xiaoxmeng merged this pull request in 7fdb321. |
Add a query trace TaskRunner to execute the replay task replacing
AssertQueryBuilder
.Use a flag named
copy_results
to control whether to copy the replay result (used in UT).