[compare B] Operator extra links per attempt: the link opts in - #30
Closed
1fanwang wants to merge 1 commit into
Closed
[compare B] Operator extra links per attempt: the link opts in#301fanwang wants to merge 1 commit into
1fanwang wants to merge 1 commit into
Conversation
Approach B for apache#71471: the link decides, rather than core keeping a row per attempt for every link. BaseOperatorLink gains keeps_a_link_per_attempt and xcom_key_for_try, and only links that set the flag get extra rows. The flag has to survive DAG serialization, since the api-server reads links through XComOperatorLink, which is rebuilt from {name: xcom_key} and knows nothing about the original class. That mapping now carries {key, per_attempt} for opted-in links and stays a bare string otherwise. Signed-off-by: 1fanwang <1fannnw@gmail.com>
Owner
Author
|
Reopened against apache/airflow as a draft so all three options sit on the issue. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TL;DR — operator extra links don't work per attempt (apache#71471). After a retry the button
under attempt 1 opens attempt 2's logs, which bites any provider linking to per-attempt logs
in EMR, Glue, Databricks, Dataproc or Livy. This is approach B of three: the link opts in,
rather than core keeping a row per attempt for every link.
One of three shapes for the same fix, drafted so they can be read side by side: A keeps a row
per attempt for every link (apache#71518), B lets the link opt in, C puts the row in
the task state store.
How
BaseOperatorLinkgainskeeps_a_link_per_attemptandxcom_key_for_try. Links that set theflag get a row per attempt and are exempt from the retry clear; everything else behaves as it
does today.
The flag has to survive DAG serialization, and that is the cost. The api-server never sees a
provider's link class — it rebuilds every link as
XComOperatorLinkfrom a{name: xcom_key}mapping that carries a string and nothing else. That mapping now carries
{"key": ..., "per_attempt": true}for opted-in links and stays a bare string otherwise.Testing
Read path against a real metastore on 3.2.2, with rows present for both attempts, asking an
opted-in link and a plain one for each attempt.
Raw output
Line 3 is the before state on the same path: a link that does not opt in still returns the
latest attempt's URL for attempt 1, which is today's behaviour and today's bug. Lines 1 and 2
are the same read with the flag set.
Key derivation on the same runtime:
Serialization round-trip, the risky part:
An existing serialized DAG reads back unchanged.
Versus A
Same per-attempt key and same purge exemption underneath, plus an opt-in. Links whose URL can
be recomputed from
try_numberwrite nothing extra, which A cannot express. The cost is aserialization format change and one more concept in the public link API.