Page MenuHomePhabricator

Gaant-style execution plan visualization behind feature flag
ClosedPublic

Authored by bengotow on Jan 17 2020, 6:10 PM.

Details

Summary

This diff adds a brand new visualization to replace the linear execution plan view. It uses a Gaant-chart flow diagram to show the execution plan graph and the dependencies between nodes, and includes an optional time axis and the ability to use the graph query syntax to filter.

This work is still experimental and is hidden behind a feature flag, which is persistent and can be toggled from http://localhost:5000/flags.

This diff includes several changes to adjacent code:

  • The SplitView component now manages the root container as well as the childern and supports horizontal OR vertical panels.
  • The Run.tsx file has been split apart and cleaned up a bit to support rendering either an <ExecutionPlan> or <GaantChart>
  • The SolidQueryInput and SolidQueryImpl have been slightly generalized - they now use a set of "GraphQueryItems" instead of Solids, and the execution plan filtering uses the same code to filter EP nodes. This may be a futile exercise in the long term depending on how/if we expand the query language to include meta-selectors like +(status:failed)+, but we'll see.

Test Plan

Run Dagit tests

Diff Detail

Repository
R1 dagster
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

bengotow created this revision.Jan 17 2020, 6:10 PM
bengotow edited the summary of this revision. (Show Details)Jan 17 2020, 6:11 PM
alangenfeld added inline comments.Jan 17 2020, 6:27 PM
js_modules/dagit/src/GaantChart.tsx
206–239

nit: this feels p substantial to be an inline fn in this already giant function - consider refactoring

206–331

some more comments would be nice here since this is long and complex

js_modules/dagit/src/GraphQueryImpl.ts
30–32

nit: could this be moved closer to the recursion its relevant to

js_modules/dagit/src/Util.tsx
161–174

we could consider moving this to the dagster.yaml instance config instead - one annoying issue with this approach is that if you change ports you change URLS and get different localstorage states

bengotow updated this revision to Diff 8766.Jan 17 2020, 6:28 PM

Comment Gaant layout algo

bengotow added inline comments.Jan 17 2020, 6:31 PM
js_modules/dagit/src/GaantChart.tsx
206–239

Good call —I think I was avoiding adding a few arguments but this should probably be moved / documented separately.

206–331

Thanks! Went through and added more comments to this—definitely necessary because I'll 100% forget why some of this is here 😅

js_modules/dagit/src/GraphQueryImpl.ts
30–32

Interesting, not sure where this came from but I'll move it!

from use - if you execute the sleepy toy pipeline with the default config which uses inprocess engine - the four parallel steps happen sequentially which is not communicated in the waterfall timed view at all since we pin no the left alignment. Is this something worth addressing now or in a subsequent diff?

bengotow updated this revision to Diff 8767.Jan 17 2020, 7:42 PM
  • Use fewer large nested functions in Gaant layout code

Ahh @alangenfeld that's a good question—for the serial execution engine displaying the dependency graph isn't quite what we want. Could we make the execution plan to include the order it plans to execute steps? Right now the plan doesn't encode enough information for us to know what the true parallelism / timing will look like (at least I don't think?) We could detect the serial execution engine and show them in a waterfall, but we might get the order wrong I think?

just spitballing ideas one thing we could do is keep the box pinned but to the left but change where the shading starts? What i want visually communicated when these things are waiting on eachother for not-data reasons is:

prha added inline comments.Jan 21 2020, 9:57 PM
js_modules/dagit/src/Util.tsx
161–174

I think this works for now... I can move this to move to the dagit section in dagster.yaml in a separate diff (will need to expose on gql)

This revision is now accepted and ready to land.Jan 21 2020, 9:58 PM

A couple pieces of feedback you can address here or in a follow-on:

This layout feels quite "crowded", with the selector box infringing on the scrollbar.

Right now, especially when not on a small screen, the log viewer feels very cramped, and you are unable expand the log viewer.

bengotow updated this revision to Diff 9116.Wed, Jan 29, 8:25 PM
  • Rework CSS on config editor help
  • Fix min heights / behavior of the gaant UI when shrunk to 0 size
  • Add split panel controls for focusing first or second panels fast
  • Throw a few gradients in for good measure
  • Much more realistic timed-waterfall
bengotow updated this revision to Diff 9118.Wed, Jan 29, 8:28 PM
  • Merge branch 'master' of github.com:dagster-io/dagster into sfbg/gaant