ferruzzi commented on code in PR #76527:
URL: https://github.com/apache/airflow/pull/76527#discussion_r4518281935
##########
airflow-core/src/airflow/config_templates/config.yml:
##########
@@ -3189,6 +3189,35 @@ triggerer:
type: integer
example: ~
default: "30"
+ watcher_log_to_stdout:
+ description: |
+ Also write an asset watcher's lines to the triggerer's standard
output, in addition to the watcher's
+ own log file. Platform log shippers that read the triggerer's output
rely on this. A deferred task's
+ and a callback's trigger lines go to their task instance's and
callback's logs, as before.
+ version_added: 3.5.0
+ type: boolean
+ example: ~
+ default: "True"
+ watcher_log_max_open_files:
+ description: |
+ Maximum number of asset watcher log files a triggerer keeps open at
once. When another watcher logs,
+ the least recently written file is closed, and it is reopened,
appending, when its watcher logs
+ again. Set to 0 for no limit. Either way it is at most a quarter of
the triggerer's open-files limit
+ (``ulimit -n``).
+ version_added: 3.5.0
+ type: integer
+ example: ~
+ default: "256"
+ watcher_log_max_bytes:
+ description: |
+ Maximum size in bytes of an asset watcher's local log file. When it is
reached the older lines are
+ moved to a ``rotated`` directory next to it, replacing the previous
rotated file, so a watcher uses at
+ most twice this much local disk on each triggerer: 4 MiB by default,
about 13,000 lines. With remote
+ logging, each rotated file is uploaded before it is replaced. Set to 0
to disable the cap.
Review Comment:
I think your retry is the right call, but it widens the hold on rotation.
`_rotated_upload_done` returns `False` while an upload is in flight and again
when it submits the retry. That's what gates the size check at
`triggerer_job_runner.py:669`, so the live file keeps growing for the whole
window. During a store outage that is every watcher, because they share
`_rotated_pool`.
I don't think the gate should change: `os.replace` runs in the pool now, so
`may_rotate` is also the only thing stopping the next rotation clobbering a
file mid-rename. I think we need to soften the promise that the docs make to
match this change. Something like:
> Target maximum size in bytes of an asset watcher's local log file. When
it is reached the older lines are moved to a ``rotated`` directory next to it,
replacing the previous rotated file, so a watcher normally uses at most twice
this much local disk on each triggerer: 4 MiB by default, about 13,000 lines.
With remote logging, each rotated file is uploaded before it is replaced, and a
rotation waits for that upload to be confirmed, so while the remote store is
unreachable the live file can grow past this. Set to 0 to disable the cap.
--
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]
