Re: [PR] Perf/fastapi orjson response optimization [airflow]
kaxil commented on PR #59149: URL: https://github.com/apache/airflow/pull/59149#issuecomment-3711725909 If you show evidence that you have tested it with real Snowflake instance, happy to re-open it @Arunodoy18 I am going to close your PRs -- Please review and test your changes with correct PR description. Using LLMs without those increase maintenance burdens and CI run time. Feel free to recreate focussed PRs following those guidelines. -- 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] Perf/fastapi orjson response optimization [airflow]
kaxil closed pull request #59149: Perf/fastapi orjson response optimization URL: https://github.com/apache/airflow/pull/59149 -- 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] Perf/fastapi orjson response optimization [airflow]
tirkarthi commented on PR #59149: URL: https://github.com/apache/airflow/pull/59149#issuecomment-3633181816 I tried `airflow api-server -w 10` on my local laptop with sqlite database using hey tool for a 17kb response compressed to 2kb. Below are the numbers after a few tries so that the workers are warmed up. I saw at max 5% improvement in average time and sometimes the comparison was almost equal. I also don't see the requests per second improving as noted in the fastapi issue where it was noted that serialization using json_encoder was blocking. So the serialization cost looks negligible compared to the overall response time. On main ``` hey -n 1000 -c 100 -H 'Cookie: session=7a96019f-07e1-49c6-80c7-846a22b1c8d8._SINWWBZKzkqxPaPaDengiEdir0; _token=eyJhbGciOiJIUzUxMiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxIiwianRpIjoiN2Y5N2Y2MWI3NDVlNGU4NDlmYjJmYTg2NWNkNzQ4NDgiLCJpc3MiOltdLCJhdWQiOiJhcGFjaGUtYWlyZmxvdyIsIm5iZiI6MTc2NTI5NjY5MywiZXhwIjoxNzY1MzAwMjkzLCJpYXQiOjE3NjUyOTY2OTN9._ShMZGsOJvDX4e3oUrDSDBlD1yNevDXu4kZ0fodnvCaDOT3ASOGSYu-QmVwhSoovqRi8RtpcETzuHMyO_fmmRw' 'http://localhost:8000/ui/dags?dag_runs_limit=14&limit=15&offset=0&exclude_stale=true&order_by=dag_display_name' Summary: Total: 7.3286 secs Slowest: 1.4896 secs Fastest: 0.0540 secs Average: 0.7018 secs Requests/sec: 136.4515 ``` With PR ``` hey -n 1000 -c 100 -H 'Cookie: session=7a96019f-07e1-49c6-80c7-846a22b1c8d8._SINWWBZKzkqxPaPaDengiEdir0; _token=eyJhbGciOiJIUzUxMiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxIiwianRpIjoiN2Y5N2Y2MWI3NDVlNGU4NDlmYjJmYTg2NWNkNzQ4NDgiLCJpc3MiOltdLCJhdWQiOiJhcGFjaGUtYWlyZmxvdyIsIm5iZiI6MTc2NTI5NjY5MywiZXhwIjoxNzY1MzAwMjkzLCJpYXQiOjE3NjUyOTY2OTN9._ShMZGsOJvDX4e3oUrDSDBlD1yNevDXu4kZ0fodnvCaDOT3ASOGSYu-QmVwhSoovqRi8RtpcETzuHMyO_fmmRw' 'http://localhost:8000/ui/dags?dag_runs_limit=14&limit=15&offset=0&exclude_stale=true&order_by=dag_display_name' Summary: Total: 7.7158 secs Slowest: 1.5425 secs Fastest: 0.0665 secs Average: 0.7186 secs Requests/sec: 129.6038 ``` -- 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] Perf/fastapi orjson response optimization [airflow]
vincbeck commented on code in PR #59149: URL: https://github.com/apache/airflow/pull/59149#discussion_r2603319608 ## airflow-core/src/airflow/api_fastapi/core_api/routes/public/dags.py: ## @@ -129,7 +135,7 @@ def get_dags( readable_dags_filter: ReadableDagsFilterDep, session: SessionDep, is_favorite: QueryFavoriteFilter, -) -> DAGCollectionResponse: +): Review Comment: Instead of removing it, why not `ORJSONResponse`? -- 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] Perf/fastapi orjson response optimization [airflow]
tirkarthi commented on PR #59149: URL: https://github.com/apache/airflow/pull/59149#issuecomment-3633016112 Thanks @Arunodoy18 for the PR. Do you have any benchmarks using load testing tools? @pierrejeambrun We are trying out an internal change similar to the PR to see if the change gives noticeable improvement in performance in terms of serialization time as part of the overall response cycle. -- 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] Perf/fastapi orjson response optimization [airflow]
prdai commented on code in PR #59149: URL: https://github.com/apache/airflow/pull/59149#discussion_r2596221860 ## airflow-core/src/airflow/api_fastapi/common/responses.py: ## Review Comment: without creating a custom class, can't the default ORJSONResponse from FastAPI be used? https://fastapi.tiangolo.com/advanced/custom-response/?h=orjsonresponse#use-orjsonresponse -- 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] Perf/fastapi orjson response optimization [airflow]
Arunodoy18 commented on PR #59149: URL: https://github.com/apache/airflow/pull/59149#issuecomment-3621804662 I have resolved the issue. Please assign reviewers to the following and let me know if some following changes is need to be done . Thank you. -- 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]
