From 8d964b9955a4af15f7a4640a25c4b276f34bdb9a Mon Sep 17 00:00:00 2001 From: Niko Ehrenfeuchter <nikolaus.ehrenfeuchter@unibas.ch> Date: Fri, 26 Apr 2019 15:05:08 +0200 Subject: [PATCH] Separate updating free space information from grace location checks The latter one is an action that potentially takes a long time as all directories have to be searched recursively there, so rather do not do it unless really necessary. As free space checks are done by the service during its main loop but grace location checks only at specific events forcefully combining them could have a strong negative impact on service performance. Refers to #20 --- ATxCommon/StorageStatus.cs | 74 ++++++++++++++++++++++++++------------ 1 file changed, 52 insertions(+), 22 deletions(-) diff --git a/ATxCommon/StorageStatus.cs b/ATxCommon/StorageStatus.cs index 5d56423..715d74f 100644 --- a/ATxCommon/StorageStatus.cs +++ b/ATxCommon/StorageStatus.cs @@ -9,12 +9,13 @@ namespace ATxCommon public class StorageStatus { /// <summary> - /// By default the status will only be updated if more than UpdateDelta seconds have + /// By default the statuses will only be updated if more than UpdateDelta seconds have /// elapsed since the last update to prevent too many updates causing high system load. /// </summary> public int UpdateDelta = 20; - public DateTime LastStatusUpdate = DateTime.MinValue; + private DateTime _lastUpdateFreeSpace = DateTime.MinValue; + private DateTime _lastUpdateGraceLocation = DateTime.MinValue; private static readonly Logger Log = LogManager.GetCurrentClassLogger(); @@ -43,7 +44,7 @@ namespace ATxCommon /// </summary> public int ExpiredDirsCount { get { - Update(); + UpdateGraceLocation(); return _expiredDirs.Count; } } @@ -68,7 +69,7 @@ namespace ATxCommon /// </summary> public Dictionary<string, List<DirectoryDetails>> ExpiredDirs { get { - Update(); + UpdateGraceLocation(); return _expiredDirs; } } @@ -78,8 +79,8 @@ namespace ATxCommon /// </summary> /// <returns>A human-readable (i.e. formatted) string with details on the grace location /// and all expired directories, grouped by the topmost level (i.e. user dirs).</returns> - Update(); public string GraceLocationSummary() { + UpdateGraceLocation(); var summary = "------ Grace location status, " + $"threshold: {_gracePeriod} days ({_gracePeriodHuman}) ------\n\n" + $" - location: [{_graceLocation}]\n"; @@ -107,7 +108,7 @@ namespace ATxCommon /// configured drives. If <paramref name="onlyLowSpace"/> is set to "true", space will only /// be reported for drives that are below their corresponding threshold.</returns> public string SpaceSummary(bool onlyLowSpace = false) { - Update(); + UpdateFreeSpace(); var summary = "------ Storage space status ------\n\n"; foreach (var drive in _drives) { var msg = $" - drive [{drive.DriveName}] " + @@ -126,17 +127,43 @@ namespace ATxCommon } /// <summary> - /// Update the current storage status in case the last update is already older than the - /// configured threshold <see cref="LastStatusUpdate"/>. + /// Update the storage status of free drive space if it's older than its threshold. /// </summary> /// <param name="force">Update, independently of the last update timestamp.</param> - public void Update(bool force = false) { + public void UpdateFreeSpace(bool force = false) { + if (force) + _lastUpdateFreeSpace = DateTime.MinValue; + + if (TimeUtils.SecondsSince(_lastUpdateFreeSpace) < UpdateDelta) + return; + + Log.Debug("Updating storage status: checking free disk space..."); + foreach (var drive in _drives) { + try { + drive.FreeSpace = new DriveInfo(drive.DriveName).TotalFreeSpace; + } + catch (Exception ex) { + // log this as an error which then also gets sent via email (if configured) and + // let the rate-limiter take care of not flooding the admin with mails: + Log.Error("Error in GetFreeDriveSpace({0}): {1}", drive.DriveName, ex.Message); + } + } + + _lastUpdateFreeSpace = DateTime.Now; + } + + /// <summary> + /// Update the storage status of the grace location if it's older than its threshold. + /// </summary> + /// <param name="force">Update, independently of the last update timestamp.</param> + public void UpdateGraceLocation(bool force = false) { if (force) - LastStatusUpdate = DateTime.MinValue; + _lastUpdateGraceLocation = DateTime.MinValue; - if (TimeUtils.SecondsSince(LastStatusUpdate) < UpdateDelta) + if (TimeUtils.SecondsSince(_lastUpdateGraceLocation) < UpdateDelta) return; + Log.Debug("Updating storage status: checking grace location..."); foreach (var userdir in _graceLocation.GetDirectories()) { var expired = new List<DirectoryDetails>(); foreach (var subdir in userdir.GetDirectories()) { @@ -147,22 +174,25 @@ namespace ATxCommon expired.Add(dirDetails); } if (expired.Count > 0) - _expiredDirs.Add(userdir.Name, expired); } - foreach (var drive in _drives) { - try { - drive.FreeSpace = new DriveInfo(drive.DriveName).TotalFreeSpace; - } - catch (Exception ex) { - // log this as an error which then also gets sent via email (if configured) and - // let the rate-limiter take care of not flooding the admin with mails: - Log.Error("Error in GetFreeDriveSpace({0}): {1}", drive.DriveName, ex.Message); - } + if (ExpiredDirsCount > 0) { + Log.Debug("Updated storage status: {0} expired directories in grace location.", + ExpiredDirsCount); } - LastStatusUpdate = DateTime.Now; + _lastUpdateGraceLocation = DateTime.Now; + } + + /// <summary> + /// Update the current storage status in case the last update is already older than the + /// configured threshold <see cref="_lastUpdateFreeSpace"/>. + /// </summary> + /// <param name="force">Update, independently of the last update timestamp.</param> + public void Update(bool force = false) { + UpdateFreeSpace(force); + UpdateGraceLocation(force); } /// <summary> -- GitLab