Skip to content

Commit 86fdce0

Browse files
author
gibsondan
authored
Don't include descriptions and metadatas on job snapshots for outputs of asset job ops (#33092)
### Summary For assets, these descriptions and metadata can already be found on the asset graph, and can be quite large. Including them on the output metadata in the snapshot as well (especially for __ASSET_JOB and subsets therein) significantly bloats the size of the snapshot. On a representative large snapshot of a single dbt node, byte size is reduced by about 10x: (and the metadata is of course still available on the asset graph) ``` Original size: 52,271,581 bytes Stripped size: 4,774,884 bytes Size reduction: 47,496,697 bytes (90.87%) ``` ### Test Plan new test cases, sanity checked local UI in the job snapshot page ### Changelog Performance improvements when launching runs of asset jobs, by removing unnecessary op output metadata on the job snapshot that is computed for each run.
1 parent 2138fe8 commit 86fdce0

4 files changed

Lines changed: 122 additions & 17 deletions

File tree

python_modules/dagster/dagster/_core/definitions/job_definition.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -828,6 +828,10 @@ def as_ephemeral_job(
828828
asset_selection=frozenset(asset_selection) if asset_selection else None,
829829
)
830830

831+
@property
832+
def is_asset_job(self) -> bool:
833+
return bool(self.asset_layer and self.asset_layer.selected_asset_keys)
834+
831835
def _get_partitions_def(
832836
self, selected_asset_keys: Optional[Iterable[AssetKey]]
833837
) -> PartitionsDefinition:

python_modules/dagster/dagster/_core/snap/node.py

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
from collections.abc import Mapping, Sequence, Set
2-
from functools import cached_property
2+
from functools import cached_property, partial
33
from typing import Optional, Union
44

55
import dagster._check as check
@@ -135,14 +135,21 @@ def build_input_def_snap(input_def: InputDefinition) -> InputDefSnap:
135135
)
136136

137137

138-
def build_output_def_snap(output_def: OutputDefinition) -> OutputDefSnap:
138+
def build_output_def_snap(output_def: OutputDefinition, job_def: JobDefinition) -> OutputDefSnap:
139139
check.inst_param(output_def, "output_def", OutputDefinition)
140+
check.inst_param(job_def, "job_def", JobDefinition)
141+
142+
# Don't include verbose user-defined metadata(description/metadata fields) on the output
143+
# definition snapshots for asset jobs, since they can be quite verbose/large and they
144+
# map directly to information that is already available on the asset graph
145+
include_verbose_metadata = not job_def.is_asset_job
146+
140147
return OutputDefSnap(
141148
name=output_def.name,
142149
dagster_type_key=output_def.dagster_type.key,
143-
description=output_def.description,
150+
description=output_def.description if include_verbose_metadata else None,
144151
is_required=output_def.is_required,
145-
metadata=output_def.metadata,
152+
metadata=output_def.metadata if include_verbose_metadata else {},
146153
is_dynamic=output_def.is_dynamic,
147154
)
148155

@@ -223,9 +230,9 @@ def build_node_defs_snapshot(job_def: JobDefinition) -> NodeDefsSnapshot:
223230
graph_def_snaps = []
224231
for node_def in job_def.all_node_defs:
225232
if isinstance(node_def, OpDefinition):
226-
op_def_snaps.append(build_op_def_snap(node_def))
233+
op_def_snaps.append(build_op_def_snap(node_def, job_def))
227234
elif isinstance(node_def, GraphDefinition):
228-
graph_def_snaps.append(build_graph_def_snap(node_def))
235+
graph_def_snaps.append(build_graph_def_snap(node_def, job_def))
229236
else:
230237
check.failed(f"Unexpected NodeDefinition type {node_def}")
231238

@@ -250,12 +257,15 @@ def _by_name(
250257
return snap.name
251258

252259

253-
def build_graph_def_snap(graph_def: GraphDefinition) -> GraphDefSnap:
260+
def build_graph_def_snap(graph_def: GraphDefinition, job_def: JobDefinition) -> GraphDefSnap:
254261
check.inst_param(graph_def, "graph_def", GraphDefinition)
255262
return GraphDefSnap(
256263
name=graph_def.name,
257264
input_def_snaps=sorted(map(build_input_def_snap, graph_def.input_defs), key=_by_name),
258-
output_def_snaps=sorted(map(build_output_def_snap, graph_def.output_defs), key=_by_name),
265+
output_def_snaps=sorted(
266+
map(partial(build_output_def_snap, job_def=job_def), graph_def.output_defs),
267+
key=_by_name,
268+
),
259269
description=graph_def.description,
260270
tags=graph_def.tags,
261271
config_field_snap=(
@@ -278,12 +288,14 @@ def build_graph_def_snap(graph_def: GraphDefinition) -> GraphDefSnap:
278288
)
279289

280290

281-
def build_op_def_snap(op_def: OpDefinition) -> OpDefSnap:
291+
def build_op_def_snap(op_def: OpDefinition, job_def: JobDefinition) -> OpDefSnap:
282292
check.inst_param(op_def, "op_def", OpDefinition)
283293
return OpDefSnap(
284294
name=op_def.name,
285295
input_def_snaps=sorted(map(build_input_def_snap, op_def.input_defs), key=_by_name),
286-
output_def_snaps=sorted(map(build_output_def_snap, op_def.output_defs), key=_by_name),
296+
output_def_snaps=sorted(
297+
map(partial(build_output_def_snap, job_def=job_def), op_def.output_defs), key=_by_name
298+
),
287299
description=op_def.description,
288300
tags=op_def.tags,
289301
required_resource_keys=sorted(op_def.required_resource_keys),

python_modules/dagster/dagster_tests/core_tests/snap_tests/test_node_def_snap.py

Lines changed: 95 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,18 @@
11
import dagster as dg
22
from dagster._core.snap import DependencyStructureIndex, GraphDefSnap, build_graph_def_snap
3-
from dagster._core.snap.node import OpDefSnap, build_op_def_snap
3+
from dagster._core.snap.node import OpDefSnap, build_node_defs_snapshot, build_op_def_snap
44

55

66
def test_basic_op_definition():
77
@dg.op
88
def noop_op(_):
99
pass
1010

11-
op_snap = build_op_def_snap(noop_op)
11+
@dg.job
12+
def my_job():
13+
noop_op()
14+
15+
op_snap = build_op_def_snap(noop_op, my_job)
1216

1317
assert op_snap
1418
assert dg.deserialize_value(dg.serialize_value(op_snap), OpDefSnap) == op_snap
@@ -35,7 +39,11 @@ def kitchen_sink_op(_, arg_two, arg_one): # out of order to test positional_inp
3539
assert arg_two
3640
raise Exception("should not execute")
3741

38-
kitchen_sink_op_snap = build_op_def_snap(kitchen_sink_op)
42+
@dg.job
43+
def my_job():
44+
kitchen_sink_op()
45+
46+
kitchen_sink_op_snap = build_op_def_snap(kitchen_sink_op, my_job)
3947

4048
assert kitchen_sink_op_snap
4149
assert kitchen_sink_op_snap.name == "kitchen_sink_op"
@@ -86,7 +94,7 @@ def noop_op(_):
8694
def comp_graph():
8795
noop_op()
8896

89-
comp_solid_meta = build_graph_def_snap(comp_graph)
97+
comp_solid_meta = build_graph_def_snap(comp_graph, comp_graph.to_job())
9098

9199
assert isinstance(comp_solid_meta, GraphDefSnap)
92100
assert (
@@ -107,7 +115,7 @@ def take_one(_, one):
107115
def comp_graph():
108116
take_one(return_one())
109117

110-
comp_solid_meta = build_graph_def_snap(comp_graph)
118+
comp_solid_meta = build_graph_def_snap(comp_graph, comp_graph.to_job())
111119

112120
assert isinstance(comp_solid_meta, GraphDefSnap)
113121
assert (
@@ -134,7 +142,7 @@ def take_many(_, items):
134142
def comp_graph(this_number):
135143
take_many([return_one(), this_number, return_one.alias("return_one_also")()])
136144

137-
comp_solid_meta = build_graph_def_snap(comp_graph)
145+
comp_solid_meta = build_graph_def_snap(comp_graph, comp_graph.to_job())
138146

139147
assert isinstance(comp_solid_meta, GraphDefSnap)
140148
assert (
@@ -146,3 +154,84 @@ def comp_graph(this_number):
146154
assert index.get_invocation("take_many")
147155
assert index.get_upstream_outputs("take_many", "items")[0].node_name == "return_one"
148156
assert index.get_upstream_outputs("take_many", "items")[1].node_name == "return_one_also"
157+
158+
159+
def test_asset_job_omits_output_description_and_metadata():
160+
@dg.asset(description="my description", metadata={"foo": "bar"})
161+
def my_asset():
162+
return 1
163+
164+
@dg.asset
165+
def my_other_asset(my_asset):
166+
return my_asset + 1
167+
168+
# Test with define_asset_job
169+
asset_job = dg.define_asset_job("my_asset_job", selection=[my_asset, my_other_asset])
170+
defs = dg.Definitions(assets=[my_asset, my_other_asset], jobs=[asset_job])
171+
job_def = defs.get_job_def("my_asset_job")
172+
173+
assert job_def.is_asset_job
174+
175+
# Build snapshots and find the my_asset op snap
176+
node_defs_snapshot = build_node_defs_snapshot(job_def)
177+
my_asset_op_snap = next(s for s in node_defs_snapshot.op_def_snaps if s.name == "my_asset")
178+
output_snap = my_asset_op_snap.get_output_snap("result")
179+
180+
assert output_snap.description is None
181+
assert output_snap.metadata == {}
182+
183+
# Test with implicit global asset job from Definitions
184+
defs_implicit = dg.Definitions(assets=[my_asset, my_other_asset])
185+
implicit_job = defs_implicit.get_implicit_global_asset_job_def()
186+
187+
assert implicit_job.is_asset_job
188+
189+
node_defs_snapshot_implicit = build_node_defs_snapshot(implicit_job)
190+
my_asset_op_snap_implicit = next(
191+
s for s in node_defs_snapshot_implicit.op_def_snaps if s.name == "my_asset"
192+
)
193+
output_snap_implicit = my_asset_op_snap_implicit.get_output_snap("result")
194+
195+
assert output_snap_implicit.description is None
196+
assert output_snap_implicit.metadata == {}
197+
198+
# Test with graph_asset - verifies build_graph_def_snap also strips metadata
199+
@dg.op
200+
def inner_op():
201+
return 1
202+
203+
@dg.graph_asset(description="graph asset description", metadata={"graph_key": "graph_value"})
204+
def my_graph_asset():
205+
return inner_op()
206+
207+
defs_with_graph = dg.Definitions(assets=[my_graph_asset])
208+
graph_asset_job = defs_with_graph.get_implicit_global_asset_job_def()
209+
210+
assert graph_asset_job.is_asset_job
211+
212+
node_defs_snapshot_graph = build_node_defs_snapshot(graph_asset_job)
213+
# Graph assets create a GraphDefSnap, not an OpDefSnap
214+
my_graph_asset_snap = next(
215+
s for s in node_defs_snapshot_graph.graph_def_snaps if s.name == "my_graph_asset"
216+
)
217+
graph_output_snap = my_graph_asset_snap.get_output_snap("result")
218+
219+
assert graph_output_snap.description is None
220+
assert graph_output_snap.metadata == {}
221+
222+
# Verify that non-asset jobs still include description and metadata
223+
@dg.op(out=dg.Out(description="op description", metadata={"key": "value"}))
224+
def my_op():
225+
return 1
226+
227+
@dg.job
228+
def my_regular_job():
229+
my_op()
230+
231+
assert not my_regular_job.is_asset_job
232+
233+
my_op_snap = build_op_def_snap(my_op, my_regular_job)
234+
regular_output_snap = my_op_snap.get_output_snap("result")
235+
236+
assert regular_output_snap.description == "op description"
237+
assert regular_output_snap.metadata == {"key": dg.TextMetadataValue("value")}

python_modules/libraries/dagster-dbt/dagster_dbt_tests/core/test_asset_decorator.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ def test_snapshot_id(
266266
assert len(set(results)) == 1
267267

268268
# this should only update if the dbt project or asset producing code changes
269-
assert results[0] == "29a3a4ac386555a3e738867b8b25765ffb17a145"
269+
assert results[0] == "4712da99dfd67a3964ed52a0b7852af831b55b20"
270270

271271

272272
@pytest.mark.parametrize("name", [None, "custom"])

0 commit comments

Comments
 (0)