kaxil commented on code in PR #76283:
URL: https://github.com/apache/airflow/pull/76283#discussion_r4468247871
##########
airflow-core/docs/core-concepts/task-state-store.rst:
##########
@@ -295,6 +295,22 @@ Mapped tasks
When a task is dynamically mapped (``task.expand(...)``), each map index has
its own task state store namespace. ``clear()`` clears only the current index's
store.
+.. warning::
+
+ The map index is positional, not an identity: index ``N`` names whatever
item happens to sit at
+ position ``N`` in the expanded input, and :doc:`the order of expansion is
not guaranteed
+ </authoring-and-scheduling/dynamic-task-mapping>`. If the upstream list
changes between attempts
+ (an insertion, a deletion, a reordering), index ``N`` names a different
item than it did before.
+
+ Clearing a task instance discards its state by default, so the next
attempt starts over and this
+ does not arise. It matters when state outlives the re-expansion: if you
clear with
+ ``keep_task_state``, or use one of the clear paths that still keep state
unconditionally (see
+ :doc:`resumable-tasks`), index ``N`` reads the state written by whatever
item was at index ``N``
+ on the previous attempt. Nothing detects the mismatch and the task still
reports success. In the
+ external-job pattern this means reconnecting to a job submitted for a
different item.
Review Comment:
The page calls this section `External job resumption` and the Synchronous
tasks section already links it as `` `External job resumption`_ ``. Could this
use the same link, so it's clear which pattern is meant?
##########
airflow-core/docs/core-concepts/task-state-store.rst:
##########
@@ -295,6 +295,22 @@ Mapped tasks
When a task is dynamically mapped (``task.expand(...)``), each map index has
its own task state store namespace. ``clear()`` clears only the current index's
store.
+.. warning::
+
+ The map index is positional, not an identity: index ``N`` names whatever
item happens to sit at
+ position ``N`` in the expanded input, and :doc:`the order of expansion is
not guaranteed
Review Comment:
The linked sentence is about the cross product of several `expand()`
arguments, and for a fixed input the index is plain arithmetic over it
(`_expand_mapped_field`), so it doesn't move on its own. Cited this broadly it
reads as if order can change by itself, which would make the last line ("only
keep task state ... when the expanded input is unchanged") unsafe too. Could
this say the upstream may return its items in a different order on a rerun
instead?
--
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]
