kaxil commented on code in PR #74204:
URL: https://github.com/apache/airflow/pull/74204#discussion_r4184845592


##########
providers/elasticsearch/src/airflow/providers/elasticsearch/log/es_task_handler.py:
##########
@@ -226,6 +230,29 @@ def _render_log_id(log_id_template: str, ti: TaskInstance 
| TaskInstanceKey, try
     )
 
 
+def _get_ti_id_fields(ti: TaskInstance | TaskInstanceKey) -> dict[str, str]:
+    # Airflow 2 task instances have no id.
+    return {"ti_id": str(ti_id)} if (ti_id := getattr(ti, "id", None)) else {}
+
+
+def _build_log_query(log_id: str, ti: RuntimeTI) -> list[dict[str, Any]]:
+    log_id_match = {"match_phrase": {"log_id": log_id}}
+    # Before 3.4 a cleared task instance gets a new id, which can differ from 
the id its logs were written under.
+    if not AIRFLOW_V_3_4_PLUS:
+        return [log_id_match]
+    return [
+        {
+            "bool": {
+                "should": [
+                    {"match_phrase": {"ti_id": str(ti.id)}},

Review Comment:
   Clear is fine, it always rotates the id. The path I meant is a failure from 
QUEUED on 3.3.x. `_process_executor_events` calls `handle_failure` on a FAILED 
event for a QUEUED TI, and in 3.3.2 `handle_failure` only calls 
`prepare_db_for_next_try` when the TI is RUNNING 
([taskinstance.py#L1913](https://github.com/apache/airflow/blob/3.3.2/airflow-core/src/airflow/models/taskinstance.py#L1913)),
 while `schedule_tis` still bumps `try_number` 
([dagrun.py#L2127](https://github.com/apache/airflow/blob/3.3.2/airflow-core/src/airflow/models/dagrun.py#L2127)).
 So a deferred task that ran try N as id A, then failed in QUEUED on resume, 
runs try N+1 on a worker still as A. #67372 fixed that (its test asserts a new 
UUID after a FAILED event from QUEUED), but it's only on main/v3-4-test, so 
you're right for 3.4.
   
   Agreed on keeping id-only matching. Would gating the writes on 
`AIRFLOW_V_3_4_PLUS` as well work? Then documents written on 3.3.x carry no 
`ti_id`, they fall into the `must_not exists` branch and match on `log_id`, and 
from 3.4 the id is per try.
   



-- 
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]

Reply via email to