jvstein commented on code in PR #68304:
URL: https://github.com/apache/airflow/pull/68304#discussion_r3396358595
##########
airflow-core/src/airflow/config_templates/config.yml:
##########
@@ -779,6 +779,18 @@ logging:
type: string
example: ~
default: "False"
+ object_store_write_mode:
Review Comment:
I can implement the changes in the other providers now. I was planning to
followup on that after this PR.
I thought the chance of truncating logs as needing a targeted fix.
##########
providers/amazon/src/airflow/providers/amazon/aws/log/s3_task_handler.py:
##########
@@ -121,7 +119,7 @@ def write(
try:
if append and self.s3_log_exists(remote_log_location):
old_log = self.s3_read(remote_log_location)
- if old_log:
+ if old_log and not log.startswith(old_log):
Review Comment:
There are two competing use cases:
- A rescheduled task lands repeatedly on the same worker (e.g. low replica
celery worker sets). Before #67144, the logs were duplicated in a runaway
fashion that can cause worker OOM.
- A shared logs volume across all workers is used along with S3 remote
logging. Before #67144, those logs were always massively duplicated on
reschedule tasks. After, those logs get truncated.
In the former use case, the `startswith` check gets more expensive as the
logs grow if the task never lands on another worker (e.g. due to celery queue
settings). The added expense prevents the duplicated logs and lowers the OOM
chance.
In the latter use case, the check grows more expensive over time, but
there's a way to opt out and set the config option.
I used the local log file truncation initially to avoid this comparison
cost, but it caused the shared logs volume problem.
--
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]
