Correctly record the RawBytesRead and scan time metric in DirectBufferedInput #9801
Conversation
facebook-github-bot
added
the
CLA Signed
✅ Deploy Preview for meta-velox canceled.
|
|
cc @ulysses-you |
JkSelf
changed the title
Yuhta
added
the
ready-to-merge
|
@Yuhta Can you help to merge? Thanks. |
|
@mbasmanova Can you help to merge? Thanks. |
|
@mbasmanova @Yuhta Can you help to merge? Thanks for your help. |
Yuhta
removed
the
ready-to-merge
|
I ran q77 of tpcds with on 1TB scale using gluten/velox as execution layer with this change, and I see Is this difference expected? |
|
@JkSelf Can you fix the conflicts ? |
@ayushi-agarwal In Velox, the raw input bytes refer to the size of data in memory, whereas the standard Spark UI displays the size of data on disk. |
|
@kagamiori has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
| // A quick sanity check for memory usage reporting. Check that peak total | ||
| // memory usage for the project node is > 0. | ||
| auto planStats = toPlanStats(task->taskStats()); | ||
| auto scanNodeId = plan->id(); |
There was a problem hiding this comment.
Hi @JkSelf, plan is used after move at line 297.
Could you please also rebase on top of the latest main after fixing this? Rebasing would help with the merge. Thanks!
There was a problem hiding this comment.
@kgpai Thanks for your catching. I have fixed and rebased. Thanks.
|
@kagamiori has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@kagamiori merged this pull request in 6b32339. |
|
Conbench analyzed the 1 benchmark run on commit There were no benchmark performance regressions. 🎉 The full Conbench report has more details. |
Follow up #8545