Re: [PR] Add max_trigger_to_select_per_loop conf to respect Triggerer HA setup [airflow]
potiuk merged PR #58803: URL: https://github.com/apache/airflow/pull/58803 -- 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]
Re: [PR] Add max_trigger_to_select_per_loop conf to respect Triggerer HA setup [airflow]
potiuk commented on PR #58803: URL: https://github.com/apache/airflow/pull/58803#issuecomment-3629623955 Very nice! -- 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]
Re: [PR] Add max_trigger_to_select_per_loop conf to respect Triggerer HA setup [airflow]
jscheffl commented on code in PR #58803:
URL: https://github.com/apache/airflow/pull/58803#discussion_r2596442686
##
airflow-core/src/airflow/models/trigger.py:
##
@@ -112,6 +113,8 @@ class Trigger(Base):
callback = relationship("Callback", back_populates="trigger",
uselist=False)
+max_trigger_to_select_per_loop = conf.getint("triggerer",
"max_trigger_to_select_per_loop", fallback=10)
Review Comment:
plus here?
```suggestion
max_trigger_to_select_per_loop = conf.getint("triggerer",
"max_trigger_to_select_per_loop", fallback=50)
```
--
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]
Re: [PR] Add max_trigger_to_select_per_loop conf to respect Triggerer HA setup [airflow]
jscheffl commented on code in PR #58803: URL: https://github.com/apache/airflow/pull/58803#discussion_r2596442508 ## airflow-core/src/airflow/config_templates/config.yml: ## @@ -2324,6 +2324,15 @@ triggerer: type: float example: ~ default: "30" +max_trigger_to_select_per_loop: + description: | +Maximum number of triggers to select per loop. Set this notably lower than ``[triggerer] capacity`` +to keep load balanced across triggerers in HA deployments. +Benchmarks show that two triggerers can still claim about 1,000 triggers within one second by default. + version_added: 3.2.0 + type: integer + example: ~ + default: "10" Review Comment: Following your benchmark then 50 per default? ```suggestion default: "50" ``` -- 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]
Re: [PR] Add max_trigger_to_select_per_loop conf to respect Triggerer HA setup [airflow]
jason810496 commented on code in PR #58803: URL: https://github.com/apache/airflow/pull/58803#discussion_r2596425358 ## airflow-core/src/airflow/config_templates/config.yml: ## @@ -2324,6 +2324,15 @@ triggerer: type: float example: ~ default: "30" +max_trigger_to_select_per_loop: + description: | +Maximum number of triggers to select per loop. Set this notably lower than ``[triggerer] capacity`` +to keep load balanced across triggerers in HA deployments. +Benchmarks show that two triggerers can still claim about 1,000 triggers within one second by default. + version_added: 3.2.0 + type: integer + example: ~ + default: "10" Review Comment: Yes, I actually get the default value from `max_dagruns_to_create_per_loop`, which also default as `10`. I use another session (minimize the logging overhead in Triggerer) to run query to get triggers count group by Triggerer(`Job.id`). The change will only impact the [load_triggers](https://github.com/apache/airflow/blob/337aee8/airflow-core/src/airflow/jobs/triggerer_job_runner.py#L527) call in `TriggerSupervisor.run` not in the `TriggerRunner.arun`, and each loop _should_ take less than 1sec (between `0.1` to `1.01` seconds on my laptop). In my previous setup, I will switch between two terminal and try to run the Triggerer in the same time (due to manual setup delay for about 2 or 3 seconds between actual Triggerers run), I found that we should set to `10` to make to workload balanced enough as screenshot (480:520), or the workloads will be skew (about 3:7). -- 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]
Re: [PR] Add max_trigger_to_select_per_loop conf to respect Triggerer HA setup [airflow]
jscheffl commented on code in PR #58803: URL: https://github.com/apache/airflow/pull/58803#discussion_r2595190936 ## airflow-core/src/airflow/config_templates/config.yml: ## @@ -2324,6 +2324,15 @@ triggerer: type: float example: ~ default: "30" +max_trigger_to_select_per_loop: + description: | +Maximum number of triggers to select per loop. Set this notably lower than ``[triggerer] capacity`` +to keep load balanced across triggerers in HA deployments. +Benchmarks show that two triggerers can still claim about 1,000 triggers within one second by default. + version_added: 3.2.0 + type: integer + example: ~ + default: "10" Review Comment: How do you get to 10 as default being a good value? This would mean the triggerer need to loop 100 times to get 1000 jobs or 5 triggerers each loop 20 times to pick-up 1000 jobs? -- 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]
Re: [PR] Add max_trigger_to_select_per_loop conf to respect Triggerer HA setup [airflow]
jason810496 closed pull request #58803: Add max_trigger_to_select_per_loop conf to respect Triggerer HA setup URL: https://github.com/apache/airflow/pull/58803 -- 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]
