Skip to content

Replace usages of task context logger with the log table - #40867

Merged
dstandish merged 12 commits into
apache:mainfrom
astronomer:replace-usages-of-task-context-logger-with-the-log-table
Jul 19, 2024
Merged

Replace usages of task context logger with the log table#40867
dstandish merged 12 commits into
apache:mainfrom
astronomer:replace-usages-of-task-context-logger-with-the-log-table

Conversation

@dstandish

@dstandishdstandish commented Jul 18, 2024

Copy link
Copy Markdown
Contributor

Shipping messages to TI logs, which unfortunately in many cases means uploading to s3, is inefficient, and not really something the scheduler should be doing.

Instead we can use an old but underutilized feature: the Log table.

I have to do some trickery to save log events from executors, namely store them in a queue and consume them in the scheduler loop.

@boring-cyborgboring-cyborgBot added area:API Airflow's REST/HTTP API area:Executors-core LocalExecutor & SequentialExecutor area:providers area:Scheduler including HA (high availability) scheduler area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues provider:amazon AWS/Amazon - related issues provider:celery labels Jul 18, 2024
@dstandish
dstandish requested a review from vincbeckJuly 18, 2024 14:04
vincbeck
vincbeck approved these changes Jul 18, 2024
View reviewed changes

@vincbeckvincbeck left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@vincbeck

vincbeck commented Jul 18, 2024

Copy link
Copy Markdown
Contributor

Some tests need to be updated though

vincbeck
vincbeck reviewed Jul 18, 2024
View reviewed changes
Comment threadairflow/jobs/scheduler_job_runner.py Outdated
Comment threadairflow/jobs/scheduler_job_runner.py Outdated
Comment threadairflow/providers/amazon/aws/executors/batch/batch_executor.py Outdated
Comment threadairflow/providers/amazon/aws/executors/ecs/ecs_executor.py Outdated
Comment threadairflow/providers/amazon/aws/executors/ecs/ecs_executor.py Outdated
@dstandish
dstandishforce-pushed the replace-usages-of-task-context-logger-with-the-log-table branch from 167cf37 to 9225fbfCompareJuly 18, 2024 21:32
@dstandishdstandish added this to the Airflow 2.10.0 milestone Jul 18, 2024
@dstandish
dstandishforce-pushed the replace-usages-of-task-context-logger-with-the-log-table branch from a069dac to 764930eCompareJuly 18, 2024 22:43
@dstandish
dstandishforce-pushed the replace-usages-of-task-context-logger-with-the-log-table branch 2 times, most recently from c098177 to 26d5331CompareJuly 18, 2024 23:45
ephraimbuddy
ephraimbuddy reviewed Jul 19, 2024
View reviewed changes
Comment threadairflow/jobs/scheduler_job_runner.py
@potiukpotiuk closed this Jul 19, 2024
@potiukpotiuk reopened this Jul 19, 2024
dstandish
dstandish commented Jul 19, 2024
View reviewed changes
Comment threadairflow/executors/base_executor.py
@dstandish
dstandish merged commit f684a58 into apache:mainJul 19, 2024
@dstandish
dstandish deleted the replace-usages-of-task-context-logger-with-the-log-table branch July 19, 2024 19:41
@ephraimbuddyephraimbuddy added the type:improvement Changelog: Improvements label Jul 22, 2024
romsharon98 pushed a commit to romsharon98/airflow that referenced this pull request Jul 26, 2024
@dstandish@vincbeck@romsharon98
Replace usages of task context logger with the log table (apache#40867)
11edadc
Use Log table instead of task context logger
The task context logger is inefficient; Log is better for this reason ---------
Co-authored-by: Vincent <97131062+vincbeck@users.noreply.github.com>
This was referenced Jul 28, 2024
Closed
Closed
@fritz-astronomerfritz-astronomer mentioned this pull request Aug 7, 2024
Open
2 tasks
@o-nikolaso-nikolas mentioned this pull request Aug 30, 2024
Closed
2 tasks
@dstandishdstandish mentioned this pull request Dec 16, 2024
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ephraimbuddyephraimbuddyephraimbuddy left review comments
@vincbeckvincbeckvincbeck approved these changes
@hussein-awalahussein-awalaAwaiting requested review from hussein-awala
@eladkaleladkalAwaiting requested review from eladkal
@o-nikolaso-nikolasAwaiting requested review from o-nikolas
@ryanahamiltonryanahamiltonAwaiting requested review from ryanahamilton
@ashbashbAwaiting requested review from ashb
@bbovenzibbovenziAwaiting requested review from bbovenzi
@pierrejeambrunpierrejeambrunAwaiting requested review from pierrejeambrun
@kaxilkaxilAwaiting requested review from kaxil
@XD-DENGXD-DENGAwaiting requested review from XD-DENG

Assignees

No one assigned

Labels

area:APIAirflow's REST/HTTP APIarea:Executors-coreLocalExecutor & SequentialExecutorarea:providersarea:Schedulerincluding HA (high availability) schedulerarea:UIRelated to UI/UX. For Frontend Developers.area:webserverWebserver related Issuesprovider:amazonAWS/Amazon - related issuesprovider:celerytype:improvementChangelog: Improvements

Projects

None yet

Milestone

Airflow 2.10.0

Development

Successfully merging this pull request may close these issues.

4 participants

@dstandish@vincbeck@ephraimbuddy@potiuk