chore: Add spilling metrics of SortMergeJoin #878
Conversation
| def getNumRows: Int = if (rowAddresses == null) { | ||
| 0 | ||
| } else { | ||
| rowAddresses.size | ||
| } |
There was a problem hiding this comment.
This is unrelated. Just when I tested several memory configs to try to trigger spilling, this method throws NPE sometimes.
| @@ -519,6 +519,8 @@ class CometExecSuite extends CometTestBase { | |||
| assert(metrics("peak_mem_used").value > 1L) | |||
| assert(metrics.contains("join_time")) | |||
| assert(metrics("join_time").value > 1L) | |||
| assert(metrics.contains("spill_count")) | |||
| assert(metrics("spill_count").value == 0) | |||
There was a problem hiding this comment.
Spilling is not triggered. But it should propagate related metrics.
There was a problem hiding this comment.
Can we also have a test that forces spilling?
There was a problem hiding this comment.
I've tried to do it locally, but not able trigger it yet.
There was a problem hiding this comment.
I will keep trying but wanted to merge these metrics first.
They are from DataFusion SortMergeJoin operator's metrics. Comet operators' metrics are propagated from DataFusion. |
|
Merged. Thanks @andygrove @comphead |
(cherry picked from commit f4400f5)
Which issue does this PR close?
Closes #.
Rationale for this change
We don't propagate spilling metrics of SortMergeJoin from DataFusion to Comet yet. This patch adds spilling related metrics to Comet SortMergeJoin operator.
What changes are included in this PR?
How are these changes tested?