From 0c30e54bcaa0464826903cc77c98fd9b7c4fb8ce Mon Sep 17 00:00:00 2001 From: Nick Farrell Date: Mon, 3 Apr 2023 12:01:09 +1000 Subject: [PATCH] Prevent myhoard starting more than one backup stream Multiple backup streams add complexity and instability, increasing load on the server for negligible benefit. It is better to complete the current backup before devoting resources to beginning a second stream. This also has the side-effect of better handling situations where one stream appears to fail, then recovers, and interferes with a new backup stream: until the first one is finished, a second one will not start, after which the first one is gone. --- myhoard/backup_stream.py | 4 ++++ myhoard/controller.py | 11 ++++++----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/myhoard/backup_stream.py b/myhoard/backup_stream.py index 9d522aa..4947e31 100644 --- a/myhoard/backup_stream.py +++ b/myhoard/backup_stream.py @@ -252,6 +252,10 @@ def running(self): yield self.stop() + def is_in_terminal_state(self) -> bool: + """Returns if the stream has finished operating, successfully or not""" + return bool(self.state.get("broken_info") or self.state.get("closed_info") or self.state.get("completed_info")) + def activate(self) -> None: with self.lock: if self.mode != self.Mode.promoted: diff --git a/myhoard/controller.py b/myhoard/controller.py index 37cb53d..bb37dc3 100644 --- a/myhoard/controller.py +++ b/myhoard/controller.py @@ -802,11 +802,12 @@ def _check_binlog_apply_status(self) -> None: self.state_manager.update_state(promote_details=promote_details) def _create_new_backup_stream_if_requested_and_max_streams_not_exceeded(self): - # Only ever have two open backup streams. Uploading binlogs to more streams than that is - # unlikely to improve the system behavior. We'll create new backup stream once the latter - # one catches up with the first, the first is marked as closed, and removed from our list. - if len(self.backup_streams) >= 2: - return + # Only ever have one "active" backup stream. + # Having multiple backup streams adds complexity, increases node resource usage, and + # more easily triggers various latent bugs. + for existing_backup_stream in self.backup_streams: + if not existing_backup_stream.is_in_terminal_state(): + return with self.lock: if self.state["backup_request"]: request: BackupRequest = self.state["backup_request"]