Merge pull request #1351

Python Plugins: Avoid pop(0) performance impact

CHANGELOG.md

@@ -10,7 +10,9 @@ and since Bareos version 20 this project adheres to [Semantic Versioning](https:
 
 ### Changed
 - cats: fix issue where `startfile` field gets wrongly updated [PR #1346]
+- Python Plugins: Avoid pop(0) performance impact [PR #1351]
 
 [PR #1335]: https://github.com/bareos/bareos/pull/1335
 [PR #1346]: https://github.com/bareos/bareos/pull/1346
+[PR #1351]: https://github.com/bareos/bareos/pull/1351
 [unreleased]: https://github.com/bareos/bareos/tree/master

contrib/fd-plugins/openvz7/BareosFdPluginVz7CtFs.py

@@ -9,6 +9,7 @@
 import bareosfd
 import BareosFdPluginBaseclass
 from lockfile import LockFile
+import collections
 
 
 class BareosFdPluginVz7CtFs(BareosFdPluginBaseclass.BareosFdPluginBaseclass):
@@ -28,7 +29,7 @@ def __init__(self, plugindef):
         # using mandatory options in python constructor is not working cause options are merged inside plugin_option_parser
         # and not on directzor side
         super(BareosFdPluginVz7CtFs, self).__init__(plugindef)
-        self.files = []
+        self.files = collections.deque()
         # Filled during start_backup_job
         self.cnf_default_excludes = []
         self.excluded_backup_paths = []
@@ -123,9 +124,9 @@ def get_cts(self, name=None, uuid=None):
                                                universal_newlines=True)
         except subprocess.CalledProcessError:
             return []
-        cts = ct_list.split("\n")
-        cts.pop(-1)
-        cts.pop(0)
+        cts = collections.deque(ct_list.split("\n"))
+        cts.pop()
+        cts.popleft()
         ct_list = []
         for record in cts:
             cname, cuuid, status = record.split()
@@ -147,11 +148,11 @@ def list_snapshots(self, ):
                 universal_newlines=True)
         except subprocess.CalledProcessError:
             return []
-        snapshot_list = snapshot_list.split("\n")
+        snapshot_list = collections.deque(snapshot_list.split("\n"))
         snapshots = []
         while snapshot_list:
             parentuuid, status, snapshot_uuid, fname = [None, None, None, None]
-            line = snapshot_list.pop(0)
+            line = snapshot_list.popleft()
             if line == "":
                 continue
             tokens = line.split()
@@ -379,7 +380,7 @@ def start_backup_file(self, savepkt):
                 "No files to backup\n")
             return bareosfd.bRCs['bareosfd.bRC_Skip']
         # reading file list from beginning to ensure dirs are created before files
-        path_to_backup = self.files.pop(0)
+        path_to_backup = self.files.popleft()
         possible_link_to_dir = path_to_backup.rstrip('/')
         try:
             osstat = os.lstat(path_to_backup)

core/src/plugins/filed/python/ldap/BareosFdPluginLDAP.py

@@ -3,7 +3,7 @@
 # BAREOS - Backup Archiving REcovery Open Sourced
 #
 # Copyright (C) 2015-2015 Planets Communications B.V.
-# Copyright (C) 2015-2020 Bareos GmbH & Co. KG
+# Copyright (C) 2015-2023 Bareos GmbH & Co. KG
 #
 # This program is Free Software; you can redistribute it and/or
 # modify it under the terms of version three of the GNU Affero General Public
@@ -37,6 +37,7 @@
 import ldap.modlist
 import time
 from calendar import timegm
+import collections
 
 
 def _safe_encode(data):
@@ -256,7 +257,8 @@ def connect_and_bind(self, options, bulk=False):
         except ldap.INVALID_CREDENTIALS:
             bareosfd.JobMessage(
                 bareosfd.bJobMessageType["M_FATAL"],
-                "Failed to bind to LDAP uri due to invalid credentials %s\n" % (options["uri"]),
+                "Failed to bind to LDAP uri due to invalid credentials %s\n"
+                % (options["uri"]),
             )
 
             return bareosfd.bRC_Error
@@ -397,15 +399,15 @@ def get_next_file_to_backup(self, savepkt):
                 # if there is nothing return an error.
                 try:
                     res_type, res_data, res_msgid, res_controls = self.resultset.next()
-                    self.ldap_entries = res_data
+                    self.ldap_entries = collections.deque(res_data)
                 except ldap.NO_SUCH_OBJECT:
                     return bareosfd.bRC_Error
                 except StopIteration:
                     return bareosfd.bRC_Error
 
             # Get the next entry from the result set.
             if self.ldap_entries:
-                self.dn, self.entry = self.ldap_entries.pop(0)
+                self.dn, self.entry = self.ldap_entries.popleft()
 
                 if self.dn:
                     # Extract the createTimestamp and modifyTimestamp and
@@ -483,7 +485,7 @@ def has_next_file(self):
                 try:
                     # Get the next result set
                     res_type, res_data, res_msgid, res_controls = self.resultset.next()
-                    self.ldap_entries = res_data
+                    self.ldap_entries = collections.deque(res_data)
 
                     # We expect something to be in the result set but better check
                     if self.ldap_entries:

docs/manuals/source/DeveloperGuide/PythonPluginAPI.rst

@@ -200,6 +200,13 @@ To let the plugin do the I/O, just set `IOP.status` to
                 #  do io in plugin
                 IOP.status = bareosfd.iostat_do_in_plugin
 
+Using large lists may cause performance issues
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Using large lists in Python can have a significant performance impact as it incurs O(n) memory movement costs for ``pop(0)`` and ``insert(0, v)`` operations. Plugins which use large lists should use ``collections.deque`` instead as its ``popleft()`` and ``appendleft()`` functions perform well. If possible it is even better to use a generator to also decrease the memory footprint.
+
+For more details see `Python collections.deque documentation <https://docs.python.org/3/library/collections.html#collections.deque>`
+
 
 Description of the Bareos Python plugin API for Bareos < 20
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~