andygrove commented on PR #2038:
URL: 
https://github.com/apache/datafusion-ballista/pull/2038#issuecomment-4981618935

   It looks like per-partition metrics break with this PR. Here is what my AI 
tells me ...
   
   Right now OperatorMetricsSet on the wire is a flat list of metrics with no 
partition field. The per-partition granularity we show in the UI has always 
been derived from the fact that a task's index and its partition id were the 
same number. In execution_stage.rs we stamp every metric a task reports with 
Some(partition), and under this PR that value is now the task index rather than 
a partition id.
   
   The effect is that a task covering a slice of N partitions files all N 
partitions' metrics under one task-index key, so they get merged into a single 
bucket. The read side in handlers.rs stays self-consistent so nothing errors, 
but the per-partition detail is gone before it is ever stored. In practice that 
means skew inside a slice becomes invisible. If one partition of sixteen 
carries most of the rows, the stage view shows one large task with no way to 
see which partition drove it, and that is exactly the view Spark-model users 
lean on to debug skew.
   
   At one partition per task this all looks fine, which is what makes it easy 
to miss. The regression only shows up once a slice covers more than one 
partition.
   
   I think the clean fix is to make this additive rather than inferred: add an 
optional partition field to OperatorMetric, have the executor label each metric 
with its local partition index, and have the scheduler map local to global 
through partition_slice when it folds task metrics into the stage. That keeps 
per-partition metrics working in the new model instead of collapsing them to 
per-task. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to