heartbeat recovery message

[Bowrna]

related: #31810
closed: #31810

this covers the point C of the issue

---

^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

[potiuk]

this looks good @Bowrna (except the static check failing) - but I have one small proposal. It took me a bit of time to see why grace_multiplier is not passed to to the method in heartbeat - and I found out that even if it is specified in is_alive method, it's never set to anything diffferent than 2.1. And I think it's placed wrrongly. it should not be parameter of is_alive, but it should be - similarly as heartrate for example) added as Job's field in Job's constructor.

self.grace_multiplier = 2.1

And then - it should be taken from job whenever needed (similarly as we take job.job_type).

It would be great to change it here - because we are anyhow touching the same code, but If you think it's too much, I am fine to approve it even without it (and you could do it as a follow-up for example).

[Bowrna]

@potiuk i will add the changes that you have suggested

[Bowrna]

@potiuk i made the changes regarding the grace_multiplier. If I remember right, the grace_multiplier was allowed to be set as a param value in config side and I don't see that part of the code currently. am I missing something here?

[potiuk]

LGTM. But I would love some other committer to take a look - it's quite crucial part of our processing and we had some issues that have been overlooked there in the past - for example #37992

[potiuk]

@potiuk i made the changes regarding the grace_multiplier. If I remember right, the grace_multiplier was allowed to be set as a param value in config side and I don't see that part of the code currently. am I missing something here?

Yes. Grace_multiplier is not set anywhere to a different value than default 2.1 now.

[potiuk]

(or so I saw from reviewing the code myself). It's still good to keep it though in case we have a case we want to override it.

[potiuk]

Looks like real issues.

[Bowrna]

Test is failing in the health api call testcases. let me debug the issue and fix it by EOD

[Bowrna]

All green now :)

[potiuk]

🎉