Skip to content

Conversation

@rawwar
Copy link
Contributor

@rawwar rawwar commented Sep 22, 2024

closes: #42380

@boring-cyborg boring-cyborg bot added the area:Scheduler including HA (high availability) scheduler label Sep 22, 2024
@rawwar
Copy link
Contributor Author

rawwar commented Sep 23, 2024

I've tested it locally with a standalone dag processor and its emitting the heartbeat for dag processor:
image

@rawwar rawwar marked this pull request as ready for review September 23, 2024 06:50
@potiuk potiuk merged commit c05dae7 into apache:main Oct 1, 2024
joaopamaral pushed a commit to joaopamaral/airflow that referenced this pull request Oct 21, 2024
---------

Signed-off-by: kalyanr <kalyan.ben10@live.com>
ellisms pushed a commit to ellisms/airflow that referenced this pull request Nov 13, 2024
---------

Signed-off-by: kalyanr <kalyan.ben10@live.com>
@shalberd
Copy link

shalberd commented Mar 12, 2025

@potiuk @rawwar will this make it into 2.10.5 tagged branch?
Dag processor has been able to separately run for a while now in Airflow 2.x, yet I cannot find this metrics export change in any release notes. Is there a particular reason for it missing in e.g. 2.10.5?

@rawwar
Copy link
Contributor Author

rawwar commented Mar 12, 2025

@potiuk @rawwar will this make it into 2.10.5 tagged branch? Dag processor has been able to separately run for a while now in Airflow 2.x, yet I cannot find this metrics export change in any release notes. Is there a particular reason for it missing in e.g. 2.10.5?

@ephraimbuddy , @jedcunningham , can this be backported?

@ephraimbuddy
Copy link
Contributor

@potiuk @rawwar will this make it into 2.10.5 tagged branch? Dag processor has been able to separately run for a while now in Airflow 2.x, yet I cannot find this metrics export change in any release notes. Is there a particular reason for it missing in e.g. 2.10.5?

@ephraimbuddy , @jedcunningham , can this be backported?

No. This is a feature. 2.10.x would only get bug fixes

@rawwar rawwar deleted the kalyan/dag_processor/add_heartbeat branch March 12, 2025 07:47
@shalberd
Copy link

shalberd commented Mar 12, 2025

@ephraimbuddy well, to me, this missing metric for a central component, visible in airflow GUI dashboard, but not in statsd / prometheus metrics, is a bugfix and not a feature ...
For something that has been a topic of interest and missing since at least last fall / autumn, I find it peculiar not to include it in airflow 2.x. There have been, 3 sub-2.x minor releases since last fall, and this here does not make it into any of them?

@potiuk
Copy link
Member

potiuk commented Mar 12, 2025

@ephraimbuddy well, to me, this missing metric for a central component, visible in airflow GUI dashboard, but not in statsd / prometheus metrics, is a bugfix and not a feature ... For something that has been a topic of interest and missing since at least last fall / autumn, I find it peculiar not to include it in airflow 2.x. There have been, 3 sub-2.x minor releases since last fall, and this here does not make it into any of them?

Nobody asked for it (including yourself, so this is no wonder). And at this point this is a question of really who should keep the burden of backporting - since you need it and it does not seem to be a huge problem for others, a viable path is that you patch your version (and migrate to Airflow 3 when it's ready).

@potiuk
Copy link
Member

potiuk commented Mar 12, 2025

You have to remember - this is an open-source project that you have full visibilty into the sources and if the maintainers choose not to backport something, doing that yourself is a perfectly possible thing to do .

@shalberd
Copy link

shalberd commented Mar 13, 2025

and if the maintainers choose not to backport something, doing that yourself ...

@potiuk Yes, I understand what you mean, am involved in other open source projects as well.
Just to clarify: By backporting this feature / commit myself to v2.10.x .. do you mean I build it and a custom image myself, or, that you want me to cherry pick the commit against main and apply it as a Merge Request / PR against e.g. v2-10-test branch, adding a now-missing unit test, similar to how you handled it with another contributor in feature 42934 in the past: #42934 (comment)

That way, I can place the burden on me ;-)

Apache Airflow rocks, thank you and all the other maintainers, whether from the community or Astronomer, for a great project / product.

@potiuk
Copy link
Member

potiuk commented Mar 13, 2025

@potiuk Yes, I understand what you mean, am involved in other open source projects as well.
Just to clarify: By backporting this feature / commit myself to v2.10.x .. do you mean I build it and a custom image myself, or, that you want me to cherry pick the commit against main and apply it as a Merge Request / PR against e.g. v2-10-test branch, adding a now-missing unit test, similar to how you handled it with another contributor in feature 42934 in the past: #42934 (comment)

I think it really depends on @ephraimbuddy and @utkarsharma2 - if he will be willing to accept such PR. I think the best way is to do it, see what it involves, get it to a green status and then ask them if that's ok to be accepted. You never know - before you do it, if it's really easy and straightforward - or maybe there are some hidden problems that needs to be solved.

Part of the reluctance to accept any change is the risk involved, and part of it that "accepting" the backport idea of the PR is more or less "committing to merging it" even if it will turn out that the problem is much more complex than we initially thought (happens more often than you thought). And the fact that we do not accept any more "new features" to v2 branches is not some "artifficial no new features" limitations is that we prefer to focus on 3.0 and do not want to spend any time on things that might distract us from it. This is the cost of "lost opportunity" for us.

So if you will to spend all your time to try to cherry-pick and test it and show us a green PR with potentially tests added to show that it works, and when it will turn out that it "just works" - with minimal set of changes, there is a chance - just a chance - that we will accept it as backport. But you should take the risk of doing the job before knowing that it will get accepted. It's on you - including the risk.

Other than that - you can try it yourself in your own environment. And actually trying it on your own would be even better path because if you try on your own and it will "just work" that's even bigger proof for the RM that they can safely merge the PR - because the last thing we want to have now is more problems in v2. caused by trying to add stuff there.

@shalberd
Copy link

shalberd commented Mar 13, 2025

Full ACK, will try myself in custom build, running in own environment on top of Airflow 2.10.5 codebase.
Then, will submit a PR, agreed and OK no guarantees of it ever making it into official 2.10.x codebase. I take that risk, have taken it before in Open Data Hub / Kubeflow notebooks as well, no problem. I "get" / understand your focus on Airflow 3.

shalberd pushed a commit to shalberd/airflow that referenced this pull request Mar 13, 2025
---------

Signed-off-by: kalyanr <kalyan.ben10@live.com>
(cherry picked from commit c05dae7)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Scheduler including HA (high availability) scheduler

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add heartbeat metric for DAG processor

5 participants