Skip to content

Conversation

universalmind303
Copy link
Contributor

@universalmind303 universalmind303 commented Jun 11, 2025

Changes Made

adds an opaque context object that we can use to pass in additional context when emitting the runtimestats events.

When using the new flotilla engine, this adds the following additional properties: [plan_id, stage_id, node_id, task_id]

Note for reviewers: (@colin-ho @srilman )

While this PR does introduce the extra information to associate an event back to stages/plans/nodes, it does not yet give us the capability to know the relationship between stages, or between nodes

For example, if we have a stage with 3 pipeline nodes, where (3) depends on both (1) and (2), we have no way to know that as we're not tracking the relationships. This is a very similar problem to #4485. In that case we were able to just walk the tree and get the relationship up front, but with our distributed plans we don't actually know what's going to happen next, so we need to map the relationships as they are formed.

Related Issues

Checklist

  • Documented in API Docs (if applicable)
  • Documented in User Guide (if applicable)
  • If adding a new documentation page, doc is added to docs/mkdocs.yml navigation
  • Documentation builds and is formatted properly (tag @/ccmao1130 for docs review)

Copy link

codecov bot commented Jun 11, 2025

Codecov Report

Attention: Patch coverage is 36.40449% with 283 lines in your changes missing coverage. Please review.

Project coverage is 77.54%. Comparing base (9321ad8) to head (4c07ec8).
Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
.../daft-distributed/src/pipeline_node/scan_source.rs 6.41% 73 Missing ⚠️
...-distributed/src/pipeline_node/in_memory_source.rs 7.01% 53 Missing ⚠️
...daft-distributed/src/pipeline_node/intermediate.rs 11.62% 38 Missing ⚠️
src/daft-distributed/src/pipeline_node/limit.rs 13.88% 31 Missing ⚠️
src/daft-local-execution/src/runtime_stats.rs 68.88% 28 Missing ⚠️
src/daft-distributed/src/plan/runner.rs 0.00% 16 Missing ⚠️
src/daft-distributed/src/scheduling/task.rs 31.25% 11 Missing ⚠️
src/daft-distributed/src/stage/mod.rs 21.42% 11 Missing ⚠️
src/daft-local-execution/src/run.rs 37.50% 5 Missing ⚠️
src/daft-distributed/src/python/ray/worker.rs 0.00% 4 Missing ⚠️
... and 10 more
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4520      +/-   ##
==========================================
- Coverage   78.07%   77.54%   -0.54%     
==========================================
  Files         843      844       +1     
  Lines      115071   115432     +361     
==========================================
- Hits        89837    89507     -330     
- Misses      25234    25925     +691     
Files with missing lines Coverage Δ
src/daft-distributed/src/pipeline_node/mod.rs 22.50% <ø> (ø)
...rc/daft-distributed/src/pipeline_node/translate.rs 99.05% <100.00%> (+0.08%) ⬆️
src/daft-distributed/src/plan/mod.rs 55.55% <ø> (ø)
src/daft-distributed/src/scheduling/dispatcher.rs 100.00% <100.00%> (ø)
...ft-distributed/src/scheduling/scheduler/default.rs 85.67% <100.00%> (+0.10%) ⬆️
...c/daft-distributed/src/scheduling/scheduler/mod.rs 92.56% <100.00%> (ø)
src/daft-distributed/src/scheduling/worker.rs 68.31% <100.00%> (ø)
src/daft-distributed/src/utils/joinset.rs 95.54% <100.00%> (+0.15%) ⬆️
daft/runners/flotilla.py 38.04% <0.00%> (ø)
src/daft-distributed/src/python/mod.rs 29.76% <0.00%> (ø)
... and 18 more

... and 24 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@universalmind303 universalmind303 merged commit a7b81b6 into main Jun 12, 2025
49 of 50 checks passed
@universalmind303 universalmind303 deleted the cory/flotilla-instrumentation branch June 12, 2025 01:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants