Improve operator tracing and make it E2E work #11360

xiaoxmeng · 2024-10-28T05:59:35Z

Differential Revision: D64946367

This PR improves the operator trace framework and replay tool implementation:
(1) Let driver execution framework to handle the trace input collection and summary file generation.
The finish trace is either called when trace limit exceeds or on operator close instead of no
more input. So it can capture more information in summary like peak memory usage. By removing
the trace input collection into the driver framework it eases the trace input collection for a spilling
operator. We add to capture the peak memory and input rows in summary so it helps identify the
hot operator or task on replay debugging.
(2) Change the trace storage layout to have root with query id for traces from different tasks
(3) Abstract the replay tool function into TraceReplayRunner class and derive in Meta internal code repo
to handle the Meta internal env setup and keep most common part in TraceReplayRunner for OSS
(4) A couple of fixes in trace replay tool to make it E2E function in Meta for table writer use case
(5) A couple of file/class renaming to make the file/class name to be more specific as currently we only
support operator level trace collection and replay

netlify · 2024-10-28T05:59:51Z

✅ Deploy Preview for meta-velox canceled.

Name	Link
🔨 Latest commit	`7994bda`
🔍 Latest deploy log	https://app.netlify.com/sites/meta-velox/deploys/6721cb87ca64f400080a2524

facebook-github-bot · 2024-10-28T05:59:59Z

This pull request was exported from Phabricator. Differential Revision: D64946367

facebook-github-bot · 2024-10-28T06:05:43Z

This pull request was exported from Phabricator. Differential Revision: D64946367

…ator#11360) Summary: Pull Request resolved: facebookincubator#11360 Differential Revision: D64946367

facebook-github-bot · 2024-10-28T06:14:27Z

This pull request was exported from Phabricator. Differential Revision: D64946367

…ator#11360) Summary: Pull Request resolved: facebookincubator#11360 Differential Revision: D64946367

duanmeng

@xiaoxmeng LGTM, thanks for your refactor.

facebook-github-bot · 2024-10-29T06:45:13Z

This pull request was exported from Phabricator. Differential Revision: D64946367

Summary: Pull Request resolved: facebookincubator#11360 This PR improves the operator trace framework and replay tool implementation: (1) Let driver execution framework to handle the trace input collection and summary file generation. The finish trace is either called when trace limit exceeds or on operator close instead of no more input. So it can capture more information in summary like peak memory usage. By removing the trace input collection into the driver framework it eases the trace input collection for a spilling operator. We add to capture the peak memory and input rows in summary so it helps identify the hot operator or task on replay debugging. (2) Change the trace storage layout to have root with query id for traces from different tasks (3) Abstract the replay tool function into TraceReplayRunner class and derive in Meta internal code repo to handle the Meta internal env setup and keep most common part in TraceReplayRunner for OSS (4) A couple of fixes in trace replay tool to make it E2E function in Meta for table writer use case (5) A couple of file/class renaming to make the file/class name to be more specific as currently we only support operator level trace collection and replay Differential Revision: D64946367

facebook-github-bot · 2024-10-29T06:55:44Z

This pull request was exported from Phabricator. Differential Revision: D64946367

Summary: Pull Request resolved: facebookincubator#11360 This PR improves the operator trace framework and replay tool implementation: (1) Let driver execution framework to handle the trace input collection and summary file generation. The finish trace is either called when trace limit exceeds or on operator close instead of no more input. So it can capture more information in summary like peak memory usage. By removing the trace input collection into the driver framework it eases the trace input collection for a spilling operator. We add to capture the peak memory and input rows in summary so it helps identify the hot operator or task on replay debugging. (2) Change the trace storage layout to have root with query id for traces from different tasks (3) Abstract the replay tool function into TraceReplayRunner class and derive in Meta internal code repo to handle the Meta internal env setup and keep most common part in TraceReplayRunner for OSS (4) A couple of fixes in trace replay tool to make it E2E function in Meta for table writer use case (5) A couple of file/class renaming to make the file/class name to be more specific as currently we only support operator level trace collection and replay Differential Revision: D64946367

facebook-github-bot · 2024-10-29T07:02:39Z

This pull request was exported from Phabricator. Differential Revision: D64946367

Summary: Pull Request resolved: facebookincubator#11360 This PR improves the operator trace framework and replay tool implementation: (1) Let driver execution framework to handle the trace input collection and summary file generation. The finish trace is either called when trace limit exceeds or on operator close instead of no more input. So it can capture more information in summary like peak memory usage. By removing the trace input collection into the driver framework it eases the trace input collection for a spilling operator. We add to capture the peak memory and input rows in summary so it helps identify the hot operator or task on replay debugging. (2) Change the trace storage layout to have root with query id for traces from different tasks (3) Abstract the replay tool function into TraceReplayRunner class and derive in Meta internal code repo to handle the Meta internal env setup and keep most common part in TraceReplayRunner for OSS (4) A couple of fixes in trace replay tool to make it E2E function in Meta for table writer use case (5) A couple of file/class renaming to make the file/class name to be more specific as currently we only support operator level trace collection and replay Differential Revision: D64946367

velox/exec/Driver.cpp

velox/tool/trace/TraceReplayer.cpp

velox/tool/trace/TraceReplayRunner.cpp

tanjialiang · 2024-10-29T18:52:03Z

velox/exec/TraceUtil.h

@@ -29,6 +30,52 @@ namespace facebook::velox::exec::trace {
 /// Creates a directory to store the query trace metdata and data.
 void createTraceDirectory(const std::string& traceDir);

+/// Returns the trace directory for a given query.
+std::string getQueryTraceDirectory(


s/get/compose/

I will keep it as get

tanjialiang · 2024-10-29T18:52:09Z

velox/exec/TraceUtil.h

+    const std::string& queryId);
+
+/// Returns the trace directory for a given query task.
+std::string getTaskTraceDirectory(


ditto, and below

velox/exec/TraceUtil.cpp

velox/exec/PartitionedOutput.cpp

tanjialiang

Thanks for the refactoring, LGTM

Summary: Pull Request resolved: facebookincubator#11360 This PR improves the operator trace framework and replay tool implementation: (1) Let driver execution framework to handle the trace input collection and summary file generation. The finish trace is either called when trace limit exceeds or on operator close instead of no more input. So it can capture more information in summary like peak memory usage. By removing the trace input collection into the driver framework it eases the trace input collection for a spilling operator. We add to capture the peak memory and input rows in summary so it helps identify the hot operator or task on replay debugging. (2) Change the trace storage layout to have root with query id for traces from different tasks (3) Abstract the replay tool function into TraceReplayRunner class and derive in Meta internal code repo to handle the Meta internal env setup and keep most common part in TraceReplayRunner for OSS (4) A couple of fixes in trace replay tool to make it E2E function in Meta for table writer use case (5) A couple of file/class renaming to make the file/class name to be more specific as currently we only support operator level trace collection and replay Reviewed By: tanjialiang Differential Revision: D64946367

facebook-github-bot · 2024-10-29T20:52:11Z

This pull request was exported from Phabricator. Differential Revision: D64946367

Summary: Pull Request resolved: facebookincubator#11360 This PR improves the operator trace framework and replay tool implementation: (1) Let driver execution framework to handle the trace input collection and summary file generation. The finish trace is either called when trace limit exceeds or on operator close instead of no more input. So it can capture more information in summary like peak memory usage. By removing the trace input collection into the driver framework it eases the trace input collection for a spilling operator. We add to capture the peak memory and input rows in summary so it helps identify the hot operator or task on replay debugging. (2) Change the trace storage layout to have root with query id for traces from different tasks (3) Abstract the replay tool function into TraceReplayRunner class and derive in Meta internal code repo to handle the Meta internal env setup and keep most common part in TraceReplayRunner for OSS (4) A couple of fixes in trace replay tool to make it E2E function in Meta for table writer use case (5) A couple of file/class renaming to make the file/class name to be more specific as currently we only support operator level trace collection and replay Reviewed By: tanjialiang Differential Revision: D64946367

facebook-github-bot · 2024-10-30T00:14:17Z

This pull request was exported from Phabricator. Differential Revision: D64946367

Summary: Pull Request resolved: facebookincubator#11360 This PR improves the operator trace framework and replay tool implementation: (1) Let driver execution framework to handle the trace input collection and summary file generation. The finish trace is either called when trace limit exceeds or on operator close instead of no more input. So it can capture more information in summary like peak memory usage. By removing the trace input collection into the driver framework it eases the trace input collection for a spilling operator. We add to capture the peak memory and input rows in summary so it helps identify the hot operator or task on replay debugging. (2) Change the trace storage layout to have root with query id for traces from different tasks (3) Abstract the replay tool function into TraceReplayRunner class and derive in Meta internal code repo to handle the Meta internal env setup and keep most common part in TraceReplayRunner for OSS (4) A couple of fixes in trace replay tool to make it E2E function in Meta for table writer use case (5) A couple of file/class renaming to make the file/class name to be more specific as currently we only support operator level trace collection and replay Reviewed By: tanjialiang Differential Revision: D64946367

facebook-github-bot · 2024-10-30T00:22:12Z

This pull request was exported from Phabricator. Differential Revision: D64946367

Summary: Pull Request resolved: facebookincubator#11360 This PR improves the operator trace framework and replay tool implementation: (1) Let driver execution framework to handle the trace input collection and summary file generation. The finish trace is either called when trace limit exceeds or on operator close instead of no more input. So it can capture more information in summary like peak memory usage. By removing the trace input collection into the driver framework it eases the trace input collection for a spilling operator. We add to capture the peak memory and input rows in summary so it helps identify the hot operator or task on replay debugging. (2) Change the trace storage layout to have root with query id for traces from different tasks (3) Abstract the replay tool function into TraceReplayRunner class and derive in Meta internal code repo to handle the Meta internal env setup and keep most common part in TraceReplayRunner for OSS (4) A couple of fixes in trace replay tool to make it E2E function in Meta for table writer use case (5) A couple of file/class renaming to make the file/class name to be more specific as currently we only support operator level trace collection and replay Reviewed By: tanjialiang Differential Revision: D64946367

Summary: Pull Request resolved: facebookincubator#11360 This PR improves the operator trace framework and replay tool implementation: (1) Let driver execution framework to handle the trace input collection and summary file generation. The finish trace is either called when trace limit exceeds or on operator close instead of no more input. So it can capture more information in summary like peak memory usage. By removing the trace input collection into the driver framework it eases the trace input collection for a spilling operator. We add to capture the peak memory and input rows in summary so it helps identify the hot operator or task on replay debugging. (2) Change the trace storage layout to have root with query id for traces from different tasks (3) Abstract the replay tool function into TraceReplayRunner class and derive in Meta internal code repo to handle the Meta internal env setup and keep most common part in TraceReplayRunner for OSS (4) A couple of fixes in trace replay tool to make it E2E function in Meta for table writer use case (5) Simplify the trace control logic by throwing if hit trace limit instead of logging in trace summary. (6) Strict the check if trace plan not is not specified or has specified the wrong node as we are only supposed to use trace for debugging purpose instead of production query running. (7) A couple of file/class renaming to make the file/class name to be more specific as currently we only support operator level trace collection and replay Reviewed By: tanjialiang Differential Revision: D64946367

facebook-github-bot · 2024-10-30T04:08:02Z

This pull request was exported from Phabricator. Differential Revision: D64946367

facebook-github-bot · 2024-10-30T05:00:22Z

This pull request was exported from Phabricator. Differential Revision: D64946367

Summary: Pull Request resolved: facebookincubator#11360 This PR improves the operator trace framework and replay tool implementation: (1) Let driver execution framework to handle the trace input collection and summary file generation. The finish trace is either called when trace limit exceeds or on operator close instead of no more input. So it can capture more information in summary like peak memory usage. By removing the trace input collection into the driver framework it eases the trace input collection for a spilling operator. We add to capture the peak memory and input rows in summary so it helps identify the hot operator or task on replay debugging. (2) Change the trace storage layout to have root with query id for traces from different tasks (3) Abstract the replay tool function into TraceReplayRunner class and derive in Meta internal code repo to handle the Meta internal env setup and keep most common part in TraceReplayRunner for OSS (4) A couple of fixes in trace replay tool to make it E2E function in Meta for table writer use case (5) Simplify the trace control logic by throwing if hit trace limit instead of logging in trace summary. (6) Strict the check if trace plan not is not specified or has specified the wrong node as we are only supposed to use trace for debugging purpose instead of production query running. (7) A couple of file/class renaming to make the file/class name to be more specific as currently we only support operator level trace collection and replay Reviewed By: tanjialiang Differential Revision: D64946367

facebook-github-bot · 2024-10-30T05:06:40Z

This pull request was exported from Phabricator. Differential Revision: D64946367

Summary: Pull Request resolved: facebookincubator#11360 This PR improves the operator trace framework and replay tool implementation: (1) Let driver execution framework to handle the trace input collection and summary file generation. The finish trace is either called when trace limit exceeds or on operator close instead of no more input. So it can capture more information in summary like peak memory usage. By removing the trace input collection into the driver framework it eases the trace input collection for a spilling operator. We add to capture the peak memory and input rows in summary so it helps identify the hot operator or task on replay debugging. (2) Change the trace storage layout to have root with query id for traces from different tasks (3) Abstract the replay tool function into TraceReplayRunner class and derive in Meta internal code repo to handle the Meta internal env setup and keep most common part in TraceReplayRunner for OSS (4) A couple of fixes in trace replay tool to make it E2E function in Meta for table writer use case (5) Simplify the trace control logic by throwing if hit trace limit instead of logging in trace summary. (6) Strict the check if trace plan not is not specified or has specified the wrong node as we are only supposed to use trace for debugging purpose instead of production query running. (7) A couple of file/class renaming to make the file/class name to be more specific as currently we only support operator level trace collection and replay Reviewed By: tanjialiang Differential Revision: D64946367

facebook-github-bot · 2024-10-30T05:53:30Z

This pull request was exported from Phabricator. Differential Revision: D64946367

Summary: Pull Request resolved: facebookincubator#11360 This PR improves the operator trace framework and replay tool implementation: (1) Let driver execution framework to handle the trace input collection and summary file generation. The finish trace is either called when trace limit exceeds or on operator close instead of no more input. So it can capture more information in summary like peak memory usage. By removing the trace input collection into the driver framework it eases the trace input collection for a spilling operator. We add to capture the peak memory and input rows in summary so it helps identify the hot operator or task on replay debugging. (2) Change the trace storage layout to have root with query id for traces from different tasks (3) Abstract the replay tool function into TraceReplayRunner class and derive in Meta internal code repo to handle the Meta internal env setup and keep most common part in TraceReplayRunner for OSS (4) A couple of fixes in trace replay tool to make it E2E function in Meta for table writer use case (5) Simplify the trace control logic by throwing if hit trace limit instead of logging in trace summary. (6) Strict the check if trace plan not is not specified or has specified the wrong node as we are only supposed to use trace for debugging purpose instead of production query running. (7) A couple of file/class renaming to make the file/class name to be more specific as currently we only support operator level trace collection and replay Reviewed By: tanjialiang Differential Revision: D64946367

facebook-github-bot · 2024-10-30T06:00:40Z

This pull request was exported from Phabricator. Differential Revision: D64946367

facebook-github-bot · 2024-10-30T07:26:54Z

This pull request has been merged in df5cff5.

conbench-facebook · 2024-10-30T08:57:34Z

Conbench analyzed the 1 benchmark run on commit df5cff50.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

Summary: A follow-up fix PR for #11360 to address the linking error: ```cmake [1432/1435] Linking CXX executable velox/tool/trace/velox_query_replayer FAILED: velox/tool/trace/velox_query_replayer ... Undefined symbols for architecture arm64: "facebook::velox::tool::trace::TraceReplayRunner::run()", referenced from: _main in TraceReplayerMain.cpp.o "facebook::velox::tool::trace::TraceReplayRunner::init()", referenced from: _main in TraceReplayerMain.cpp.o "facebook::velox::tool::trace::TraceReplayRunner::TraceReplayRunner()", referenced from: _main in TraceReplayerMain.cpp.o "vtable for facebook::velox::tool::trace::TraceReplayRunner", referenced from: Reviewed By: xiaoxmeng Differential Revision: D65217021 Pulled By: kgpai fbshipit-source-id: 2e0e44d916a27853366d77d883f0add70993fdc0

xiaoxmeng requested review from assignUser and majetideepak as code owners October 28, 2024 05:59

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 28, 2024

facebook-github-bot added the fb-exported label Oct 28, 2024

xiaoxmeng added a commit to xiaoxmeng/velox that referenced this pull request Oct 28, 2024

Operator trace refactor and support more trace summary (facebookincub…

9f00ded

…ator#11360) Summary: Pull Request resolved: facebookincubator#11360 Differential Revision: D64946367

xiaoxmeng force-pushed the export-D64946367 branch from 97750ba to 9f00ded Compare October 28, 2024 06:05

xiaoxmeng added a commit to xiaoxmeng/velox that referenced this pull request Oct 28, 2024

Operator trace refactor and support more trace summary (facebookincub…

39d7f55

…ator#11360) Summary: Pull Request resolved: facebookincubator#11360 Differential Revision: D64946367

xiaoxmeng force-pushed the export-D64946367 branch from 9f00ded to 39d7f55 Compare October 28, 2024 06:14

duanmeng self-requested a review October 28, 2024 06:29

duanmeng approved these changes Oct 28, 2024

View reviewed changes

xiaoxmeng changed the title ~~Operator trace refactor and support more trace summary~~ Improve operator tracing and make it E2E work Oct 29, 2024

xiaoxmeng force-pushed the export-D64946367 branch from 39d7f55 to 39873ce Compare October 29, 2024 06:45

xiaoxmeng force-pushed the export-D64946367 branch from 39873ce to 4117e6f Compare October 29, 2024 06:55

xiaoxmeng force-pushed the export-D64946367 branch from 4117e6f to 586949b Compare October 29, 2024 07:02

tanjialiang reviewed Oct 29, 2024

View reviewed changes

tanjialiang approved these changes Oct 29, 2024

View reviewed changes

xiaoxmeng force-pushed the export-D64946367 branch from 586949b to 6dbf3dd Compare October 29, 2024 20:52

xiaoxmeng force-pushed the export-D64946367 branch from 8ac3433 to fed3960 Compare October 29, 2024 21:33

xiaoxmeng force-pushed the export-D64946367 branch from fed3960 to aa9ee2e Compare October 30, 2024 00:14

xiaoxmeng force-pushed the export-D64946367 branch from aa9ee2e to b31b1e0 Compare October 30, 2024 00:22

xiaoxmeng force-pushed the export-D64946367 branch from b31b1e0 to 28cf060 Compare October 30, 2024 04:08

xiaoxmeng force-pushed the export-D64946367 branch from 28cf060 to c0844e2 Compare October 30, 2024 05:00

xiaoxmeng force-pushed the export-D64946367 branch from c0844e2 to 8704d7a Compare October 30, 2024 05:06

xiaoxmeng force-pushed the export-D64946367 branch from 8704d7a to 9086caa Compare October 30, 2024 05:53

xiaoxmeng force-pushed the export-D64946367 branch from 9086caa to 7994bda Compare October 30, 2024 06:00

facebook-github-bot closed this in df5cff5 Oct 30, 2024

facebook-github-bot added the Merged label Oct 30, 2024

zuyu mentioned this pull request Oct 30, 2024

Fix velox_query_replayer linking error #11388

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve operator tracing and make it E2E work #11360

Improve operator tracing and make it E2E work #11360

xiaoxmeng commented Oct 28, 2024 •

edited

Loading

netlify bot commented Oct 28, 2024 •

edited

Loading

facebook-github-bot commented Oct 28, 2024

facebook-github-bot commented Oct 28, 2024

facebook-github-bot commented Oct 28, 2024

duanmeng left a comment

facebook-github-bot commented Oct 29, 2024

facebook-github-bot commented Oct 29, 2024

facebook-github-bot commented Oct 29, 2024

tanjialiang Oct 29, 2024

xiaoxmeng Oct 29, 2024

tanjialiang Oct 29, 2024

tanjialiang left a comment

facebook-github-bot commented Oct 29, 2024

facebook-github-bot commented Oct 30, 2024

facebook-github-bot commented Oct 30, 2024

facebook-github-bot commented Oct 30, 2024

facebook-github-bot commented Oct 30, 2024

facebook-github-bot commented Oct 30, 2024

facebook-github-bot commented Oct 30, 2024

facebook-github-bot commented Oct 30, 2024

facebook-github-bot commented Oct 30, 2024

conbench-facebook bot commented Oct 30, 2024

Improve operator tracing and make it E2E work #11360

Improve operator tracing and make it E2E work #11360

Conversation

xiaoxmeng commented Oct 28, 2024 • edited Loading

netlify bot commented Oct 28, 2024 • edited Loading

✅ Deploy Preview for meta-velox canceled.

facebook-github-bot commented Oct 28, 2024

facebook-github-bot commented Oct 28, 2024

facebook-github-bot commented Oct 28, 2024

duanmeng left a comment

Choose a reason for hiding this comment

facebook-github-bot commented Oct 29, 2024

facebook-github-bot commented Oct 29, 2024

facebook-github-bot commented Oct 29, 2024

tanjialiang Oct 29, 2024

Choose a reason for hiding this comment

xiaoxmeng Oct 29, 2024

Choose a reason for hiding this comment

tanjialiang Oct 29, 2024

Choose a reason for hiding this comment

tanjialiang left a comment

Choose a reason for hiding this comment

facebook-github-bot commented Oct 29, 2024

facebook-github-bot commented Oct 30, 2024

facebook-github-bot commented Oct 30, 2024

facebook-github-bot commented Oct 30, 2024

facebook-github-bot commented Oct 30, 2024

facebook-github-bot commented Oct 30, 2024

facebook-github-bot commented Oct 30, 2024

facebook-github-bot commented Oct 30, 2024

facebook-github-bot commented Oct 30, 2024

conbench-facebook bot commented Oct 30, 2024

xiaoxmeng commented Oct 28, 2024 •

edited

Loading

netlify bot commented Oct 28, 2024 •

edited

Loading