potiuk commented on code in PR #37541:
URL: https://github.com/apache/airflow/pull/37541#discussion_r1498195225
##########
airflow/jobs/job.py:
##########
@@ -183,30 +184,32 @@ def heartbeat(
previous_heartbeat = self.latest_heartbeat
try:
- # This will cause it to load from the db
- self._merge_from(Job._fetch_from_db(self, session))
- previous_heartbeat = self.latest_heartbeat
-
- if self.state == JobState.RESTARTING:
- self.kill()
-
- # Figure out how long to sleep for
- sleep_for = 0
- if self.latest_heartbeat:
- seconds_remaining = (
- self.heartrate - (timezone.utcnow() -
self.latest_heartbeat).total_seconds()
- )
- sleep_for = max(0, seconds_remaining)
- sleep(sleep_for)
-
- job = Job._update_heartbeat(job=self, session=session)
- self._merge_from(job)
-
- # At this point, the DB has updated.
- previous_heartbeat = self.latest_heartbeat
-
- heartbeat_callback(session)
- self.log.debug("[heartbeat]")
+ for attempt in run_with_db_retries(logger=self.log):
Review Comment:
> These retries are required for every DB call. Instead of every where, may
be its good idea to implement a wrapper that does implicitly.
BTW. If I understood what you propose, that's a bad idea to do it for every
db call. This is what people get wrongly. You SHOULD NOT retry every single DB
call when it fails, because in many cases it might lead to inconsistencies -
because your in-memory state might get wrong. This would only be a good idea,
if you have pure DB state, but if your state is shared between the in-memory
objects and the DB, retrying every single DB call on operational error is
wrong. By default it should fail the whole proces and restart it, to make sure
that the error did not cause any in-memory vs. db inclisnsistencies. Only in
cases where you are sure that you can safely retry an operation you should do
it.
--
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]
