From 564117b3ebf457c3d33343e5262e4d6b0b13eafe Mon Sep 17 00:00:00 2001 From: bohlski Date: Wed, 12 Oct 2022 15:29:07 +0200 Subject: [PATCH 1/3] Fix audit trail GUI wrong first preservation start The underlying bug has now been fixed. Included in this AuditTrailCollector and LocalAuditTrailPreserver now inherit from the new AuditTrailTaskStarter class, which allows similarities between the two to be abstracted away. --- .../audittrails/AuditTrailTaskStarter.java | 44 +++++++++++++++++++ .../collector/AuditTrailCollector.java | 41 +++++------------ .../preserver/LocalAuditTrailPreserver.java | 29 ++++++------ 3 files changed, 67 insertions(+), 47 deletions(-) create mode 100644 bitrepository-audit-trail-service/src/main/java/org/bitrepository/audittrails/AuditTrailTaskStarter.java diff --git a/bitrepository-audit-trail-service/src/main/java/org/bitrepository/audittrails/AuditTrailTaskStarter.java b/bitrepository-audit-trail-service/src/main/java/org/bitrepository/audittrails/AuditTrailTaskStarter.java new file mode 100644 index 000000000..d0403c245 --- /dev/null +++ b/bitrepository-audit-trail-service/src/main/java/org/bitrepository/audittrails/AuditTrailTaskStarter.java @@ -0,0 +1,44 @@ +package org.bitrepository.audittrails; + +import org.bitrepository.audittrails.store.AuditTrailStore; +import org.bitrepository.common.ArgumentValidator; +import org.bitrepository.common.settings.Settings; +import org.bitrepository.common.utils.XmlUtils; + +import java.time.Duration; + +/** + * Parent class for classes starting TimerTasks related to audit-trail operations. + */ +public abstract class AuditTrailTaskStarter { + /** Default duration to pass after system-startup before starting audit trail tasks. */ + protected final Duration DEFAULT_GRACE_PERIOD = Duration.ZERO; + protected final Settings settings; + protected final AuditTrailStore store; + + public AuditTrailTaskStarter(Settings settings, AuditTrailStore store) { + ArgumentValidator.checkNotNull(settings, "settings"); + ArgumentValidator.checkNotNull(store, "AuditTrailStore store"); + this.settings = settings; + this.store = store; + } + + /** + * Get the grace period/delay duration for audit trail tasks from settings/default duration. + * + * Changing the grace period allows time for the system to finish startup before it has to start + * delivering/processing audit trails. Zero duration = start tasks immediately on startup. + * @return The time to wait before starting audit trail tasks (collection/preservation). + */ + protected Duration getGracePeriod() { + if (settings.getReferenceSettings().getAuditTrailServiceSettings().isSetGracePeriod()) { + javax.xml.datatype.Duration gracePeriod = + settings.getReferenceSettings().getAuditTrailServiceSettings().getGracePeriod(); + return XmlUtils.xmlDurationToDuration(gracePeriod); + } else { + return DEFAULT_GRACE_PERIOD; + } + } + + public abstract void close(); +} diff --git a/bitrepository-audit-trail-service/src/main/java/org/bitrepository/audittrails/collector/AuditTrailCollector.java b/bitrepository-audit-trail-service/src/main/java/org/bitrepository/audittrails/collector/AuditTrailCollector.java index 0d2b1ade2..d3b398dce 100644 --- a/bitrepository-audit-trail-service/src/main/java/org/bitrepository/audittrails/collector/AuditTrailCollector.java +++ b/bitrepository-audit-trail-service/src/main/java/org/bitrepository/audittrails/collector/AuditTrailCollector.java @@ -25,6 +25,7 @@ package org.bitrepository.audittrails.collector; import org.bitrepository.access.getaudittrails.AuditTrailClient; +import org.bitrepository.audittrails.AuditTrailTaskStarter; import org.bitrepository.audittrails.store.AuditTrailStore; import org.bitrepository.audittrails.webservice.CollectorInfo; import org.bitrepository.common.ArgumentValidator; @@ -46,36 +47,29 @@ /** * Manages the retrieval of AuditTrails from contributors. */ -public class AuditTrailCollector { +public class AuditTrailCollector extends AuditTrailTaskStarter { private final Logger log = LoggerFactory.getLogger(getClass()); private final Map collectorTasks = new HashMap<>(); private final Timer timer; - private final Settings settings; - /** - * Initial grace period in milliseconds after startup to allow the system to finish startup. - */ - private static final Duration DEFAULT_GRACE_PERIOD = Duration.ZERO; /** * @param settings The settings for this collector. * @param client The client for handling the conversation for collecting the audit trails. * @param store The storage of the audit trails data. - * @param alarmDispatcher the alarm dispatcher. Can be null. + * @param alarmDispatcher The alarm dispatcher. Can be null. */ public AuditTrailCollector(Settings settings, AuditTrailClient client, AuditTrailStore store, AlarmDispatcher alarmDispatcher) { - ArgumentValidator.checkNotNull(settings, "settings"); + super(settings, store); ArgumentValidator.checkNotNull(client, "AuditTrailClient client"); - ArgumentValidator.checkNotNull(store, "AuditTrailStore store"); ArgumentValidator.checkNotNull(alarmDispatcher, "AlarmDispatcher alarmDispatcher"); - - this.settings = settings; this.timer = new Timer(true); javax.xml.datatype.Duration collectAuditInterval = settings.getReferenceSettings().getAuditTrailServiceSettings().getCollectAuditInterval(); Duration collectionInterval = XmlUtils.xmlDurationToDuration(collectAuditInterval); long collectionIntervalMillis = collectionInterval.toMillis(); + Duration collectionGracePeriod = getGracePeriod(); for (Collection c : settings.getRepositorySettings().getCollections().getCollection()) { IncrementalCollector collector = new IncrementalCollector(c.getID(), @@ -84,11 +78,10 @@ public AuditTrailCollector(Settings settings, AuditTrailClient client, AuditTrai SettingsUtils.getMaxClientPageSize(), alarmDispatcher); AuditTrailCollectionTimerTask collectorTask = new AuditTrailCollectionTimerTask( - collector, collectionIntervalMillis, Math.toIntExact(getGracePeriod().toMillis())); - log.info("Will start collection of audit trail every " + - TimeUtils.durationToHuman(collectionInterval) + " " + - "after a grace period of " + TimeUtils.durationToHuman(getGracePeriod())); - timer.scheduleAtFixedRate(collectorTask, getGracePeriod().toMillis(), collectionIntervalMillis / 10); + collector, collectionIntervalMillis, Math.toIntExact(collectionGracePeriod.toMillis())); + log.info("Will start collection of audit trail every " + TimeUtils.durationToHuman(collectionInterval) + + "after a grace period of " + TimeUtils.durationToHuman(collectionGracePeriod)); + timer.scheduleAtFixedRate(collectorTask, collectionGracePeriod.toMillis(), collectionIntervalMillis / 10); collectorTasks.put(c.getID(), collectorTask); } } @@ -108,7 +101,7 @@ public CollectorInfo getCollectorInfo(String collectionID) { info.setLastDuration("Collection has not finished yet"); } } else { - info.setLastStart("Audit trail collection has not started"); + info.setLastStart("Audit trail collection has not started yet"); info.setLastDuration("Not available"); } info.setNextStart(TimeUtils.shortDate(nextRun)); @@ -125,20 +118,6 @@ public void collectNewestAudits(String collectionID) { collectorTasks.get(collectionID).runCollection(); } - /** - * @return The time to wait before starting collection of audit trails. This enables the system to have time to - * finish startup before they have to start delivering/process audit trails. - */ - private Duration getGracePeriod() { - if (settings.getReferenceSettings().getAuditTrailServiceSettings().isSetGracePeriod()) { - javax.xml.datatype.Duration gracePeriod = - settings.getReferenceSettings().getAuditTrailServiceSettings().getGracePeriod(); - return XmlUtils.xmlDurationToDuration(gracePeriod); - } else { - return DEFAULT_GRACE_PERIOD; - } - } - /** * Closes the AuditTrailCollector. */ diff --git a/bitrepository-audit-trail-service/src/main/java/org/bitrepository/audittrails/preserver/LocalAuditTrailPreserver.java b/bitrepository-audit-trail-service/src/main/java/org/bitrepository/audittrails/preserver/LocalAuditTrailPreserver.java index 86e83fecc..12b16e855 100644 --- a/bitrepository-audit-trail-service/src/main/java/org/bitrepository/audittrails/preserver/LocalAuditTrailPreserver.java +++ b/bitrepository-audit-trail-service/src/main/java/org/bitrepository/audittrails/preserver/LocalAuditTrailPreserver.java @@ -22,13 +22,14 @@ package org.bitrepository.audittrails.preserver; import org.apache.commons.codec.DecoderException; -import org.bitrepository.common.TimerTaskSchedule; +import org.bitrepository.audittrails.AuditTrailTaskStarter; import org.bitrepository.audittrails.store.AuditTrailStore; import org.bitrepository.audittrails.webservice.PreservationInfo; import org.bitrepository.bitrepositoryelements.ChecksumDataForFileTYPE; import org.bitrepository.bitrepositoryelements.ChecksumSpecTYPE; import org.bitrepository.client.eventhandler.EventHandler; import org.bitrepository.common.ArgumentValidator; +import org.bitrepository.common.TimerTaskSchedule; import org.bitrepository.common.exceptions.OperationFailedException; import org.bitrepository.common.settings.Settings; import org.bitrepository.common.utils.Base16Utils; @@ -52,12 +53,10 @@ import java.net.URISyntaxException; import java.net.URL; import java.time.Duration; -import java.time.Instant; import java.util.Date; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.Timer; import java.util.TimerTask; @@ -65,13 +64,11 @@ * Handles the preservation of audit trails to a collection defined for the local repository. * This means, that each set of audit trails will be preserved within its own collection. */ -public class LocalAuditTrailPreserver implements AuditTrailPreserver { +public class LocalAuditTrailPreserver extends AuditTrailTaskStarter implements AuditTrailPreserver { private final Logger log = LoggerFactory.getLogger(getClass()); - private final AuditTrailStore store; private final BlockingPutFileClient client; private final Map auditPackers = new HashMap<>(); private final AuditTrailPreservation preservationSettings; - private final Settings settings; private final FileExchange exchange; private Timer timer; private AuditPreservationTimerTask preservationTask = null; @@ -83,13 +80,10 @@ public class LocalAuditTrailPreserver implements AuditTrailPreserver { * @param client The PutFileClient for putting the audit trail packages to the collection. */ public LocalAuditTrailPreserver(Settings settings, AuditTrailStore store, PutFileClient client, FileExchange exchange) { - ArgumentValidator.checkNotNull(settings, "Settings preservationSettings"); - ArgumentValidator.checkNotNull(store, "AuditTrailStore store"); + super(settings, store); ArgumentValidator.checkNotNull(client, "PutFileClient client"); - this.settings = settings; this.preservationSettings = settings.getReferenceSettings().getAuditTrailServiceSettings().getAuditTrailPreservation(); - this.store = store; this.client = new BlockingPutFileClient(client); this.exchange = exchange; for (String collectionID : SettingsUtils.getAllCollectionsIDs()) { @@ -126,16 +120,19 @@ public void start() { javax.xml.datatype.Duration preservationIntervalXmlDur = preservationSettings.getAuditTrailPreservationInterval(); Duration preservationInterval = XmlUtils.xmlDurationToDuration(preservationIntervalXmlDur); long timerCheckIntervalMillis = preservationInterval.dividedBy(10).toMillis(); - log.info("Instantiating the preservation of audit trails every {}", - TimeUtils.durationToHuman(preservationInterval)); + Duration preservationGracePeriod = getGracePeriod(); + log.info("Starting the preservation of audit trails every {} after grace period of {}.", + TimeUtils.durationToHuman(preservationInterval), TimeUtils.durationToHuman(preservationGracePeriod)); timer = new Timer(true); - preservationTask = new AuditPreservationTimerTask(preservationInterval.toMillis()); - timer.scheduleAtFixedRate(preservationTask, timerCheckIntervalMillis, timerCheckIntervalMillis); + preservationTask = new AuditPreservationTimerTask(preservationInterval.toMillis(), + Math.toIntExact(preservationGracePeriod.toMillis())); + timer.scheduleAtFixedRate(preservationTask, preservationGracePeriod.toMillis(), timerCheckIntervalMillis); } @Override public void close() { if (timer != null) { + preservationTask.cancel(); timer.cancel(); } } @@ -270,8 +267,8 @@ private class AuditPreservationTimerTask extends TimerTask { * @param interval The interval between running this timer task. */ // TODO: Replace old time representation (https://sbforge.org/jira/browse/BITMAG-1180) - private AuditPreservationTimerTask(long interval) { - this.schedule = new TimerTaskSchedule(interval, 0); + private AuditPreservationTimerTask(long interval, int gracePeriod) { + this.schedule = new TimerTaskSchedule(interval, gracePeriod); } public Date getNextScheduledRun() { From a842dcf70b748b0f03571a5ff00e7533c8d6b5a5 Mon Sep 17 00:00:00 2001 From: bohlski Date: Wed, 12 Oct 2022 15:33:18 +0200 Subject: [PATCH 2/3] Fix AuditTrailCollector log message --- .../audittrails/collector/AuditTrailCollector.java | 4 ++-- .../audittrails/preserver/LocalAuditTrailPreserver.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/bitrepository-audit-trail-service/src/main/java/org/bitrepository/audittrails/collector/AuditTrailCollector.java b/bitrepository-audit-trail-service/src/main/java/org/bitrepository/audittrails/collector/AuditTrailCollector.java index d3b398dce..8c125d7cb 100644 --- a/bitrepository-audit-trail-service/src/main/java/org/bitrepository/audittrails/collector/AuditTrailCollector.java +++ b/bitrepository-audit-trail-service/src/main/java/org/bitrepository/audittrails/collector/AuditTrailCollector.java @@ -79,8 +79,8 @@ public AuditTrailCollector(Settings settings, AuditTrailClient client, AuditTrai alarmDispatcher); AuditTrailCollectionTimerTask collectorTask = new AuditTrailCollectionTimerTask( collector, collectionIntervalMillis, Math.toIntExact(collectionGracePeriod.toMillis())); - log.info("Will start collection of audit trail every " + TimeUtils.durationToHuman(collectionInterval) + - "after a grace period of " + TimeUtils.durationToHuman(collectionGracePeriod)); + log.info("Starting collection of audit trails every {} after grace period of {}.", + TimeUtils.durationToHuman(collectionInterval), TimeUtils.durationToHuman(collectionGracePeriod)); timer.scheduleAtFixedRate(collectorTask, collectionGracePeriod.toMillis(), collectionIntervalMillis / 10); collectorTasks.put(c.getID(), collectorTask); } diff --git a/bitrepository-audit-trail-service/src/main/java/org/bitrepository/audittrails/preserver/LocalAuditTrailPreserver.java b/bitrepository-audit-trail-service/src/main/java/org/bitrepository/audittrails/preserver/LocalAuditTrailPreserver.java index 12b16e855..ab7546040 100644 --- a/bitrepository-audit-trail-service/src/main/java/org/bitrepository/audittrails/preserver/LocalAuditTrailPreserver.java +++ b/bitrepository-audit-trail-service/src/main/java/org/bitrepository/audittrails/preserver/LocalAuditTrailPreserver.java @@ -121,7 +121,7 @@ public void start() { Duration preservationInterval = XmlUtils.xmlDurationToDuration(preservationIntervalXmlDur); long timerCheckIntervalMillis = preservationInterval.dividedBy(10).toMillis(); Duration preservationGracePeriod = getGracePeriod(); - log.info("Starting the preservation of audit trails every {} after grace period of {}.", + log.info("Starting preservation of audit trails every {} after grace period of {}.", TimeUtils.durationToHuman(preservationInterval), TimeUtils.durationToHuman(preservationGracePeriod)); timer = new Timer(true); preservationTask = new AuditPreservationTimerTask(preservationInterval.toMillis(), From 1da75a21d92d917c041c64e909eebaff1b2d11d6 Mon Sep 17 00:00:00 2001 From: bohlski Date: Fri, 14 Oct 2022 10:46:05 +0200 Subject: [PATCH 3/3] Stop audit trail TimerTasks waking between intervals Both the TimerTasks for collection and preservation were set to wake 10 times within their intervals checking if it was time to do stuff and otherwise doing nothing. So waking 9 times with no purpose. No explaining comment as to why. Timer#scheduleAtFixedRate only schedules tasks on one thread, so scheduling more tasks than needed does not seem to have any benefit even if it was in case tasks were taking long to complete. The TimerTasks now only wake on their intervals. --- .../collector/AuditTrailCollectionTimerTask.java | 4 +--- .../collector/AuditTrailCollector.java | 5 ++--- .../preserver/LocalAuditTrailPreserver.java | 16 +++++++--------- 3 files changed, 10 insertions(+), 15 deletions(-) diff --git a/bitrepository-audit-trail-service/src/main/java/org/bitrepository/audittrails/collector/AuditTrailCollectionTimerTask.java b/bitrepository-audit-trail-service/src/main/java/org/bitrepository/audittrails/collector/AuditTrailCollectionTimerTask.java index 8800e3aad..5b8af171c 100644 --- a/bitrepository-audit-trail-service/src/main/java/org/bitrepository/audittrails/collector/AuditTrailCollectionTimerTask.java +++ b/bitrepository-audit-trail-service/src/main/java/org/bitrepository/audittrails/collector/AuditTrailCollectionTimerTask.java @@ -75,8 +75,6 @@ public synchronized void runCollection() { @Override public void run() { - if (schedule.getNextRun().getTime() < System.currentTimeMillis()) { - runCollection(); - } + runCollection(); } } diff --git a/bitrepository-audit-trail-service/src/main/java/org/bitrepository/audittrails/collector/AuditTrailCollector.java b/bitrepository-audit-trail-service/src/main/java/org/bitrepository/audittrails/collector/AuditTrailCollector.java index 8c125d7cb..c5c09e635 100644 --- a/bitrepository-audit-trail-service/src/main/java/org/bitrepository/audittrails/collector/AuditTrailCollector.java +++ b/bitrepository-audit-trail-service/src/main/java/org/bitrepository/audittrails/collector/AuditTrailCollector.java @@ -68,7 +68,6 @@ public AuditTrailCollector(Settings settings, AuditTrailClient client, AuditTrai javax.xml.datatype.Duration collectAuditInterval = settings.getReferenceSettings().getAuditTrailServiceSettings().getCollectAuditInterval(); Duration collectionInterval = XmlUtils.xmlDurationToDuration(collectAuditInterval); - long collectionIntervalMillis = collectionInterval.toMillis(); Duration collectionGracePeriod = getGracePeriod(); for (Collection c : settings.getRepositorySettings().getCollections().getCollection()) { @@ -78,10 +77,10 @@ public AuditTrailCollector(Settings settings, AuditTrailClient client, AuditTrai SettingsUtils.getMaxClientPageSize(), alarmDispatcher); AuditTrailCollectionTimerTask collectorTask = new AuditTrailCollectionTimerTask( - collector, collectionIntervalMillis, Math.toIntExact(collectionGracePeriod.toMillis())); + collector, collectionInterval.toMillis(), Math.toIntExact(collectionGracePeriod.toMillis())); log.info("Starting collection of audit trails every {} after grace period of {}.", TimeUtils.durationToHuman(collectionInterval), TimeUtils.durationToHuman(collectionGracePeriod)); - timer.scheduleAtFixedRate(collectorTask, collectionGracePeriod.toMillis(), collectionIntervalMillis / 10); + timer.scheduleAtFixedRate(collectorTask, collectionGracePeriod.toMillis(), collectionInterval.toMillis()); collectorTasks.put(c.getID(), collectorTask); } } diff --git a/bitrepository-audit-trail-service/src/main/java/org/bitrepository/audittrails/preserver/LocalAuditTrailPreserver.java b/bitrepository-audit-trail-service/src/main/java/org/bitrepository/audittrails/preserver/LocalAuditTrailPreserver.java index ab7546040..0d6e28c00 100644 --- a/bitrepository-audit-trail-service/src/main/java/org/bitrepository/audittrails/preserver/LocalAuditTrailPreserver.java +++ b/bitrepository-audit-trail-service/src/main/java/org/bitrepository/audittrails/preserver/LocalAuditTrailPreserver.java @@ -117,16 +117,16 @@ public void start() { log.debug("Cancelling old timer."); timer.cancel(); } + javax.xml.datatype.Duration preservationIntervalXmlDur = preservationSettings.getAuditTrailPreservationInterval(); Duration preservationInterval = XmlUtils.xmlDurationToDuration(preservationIntervalXmlDur); - long timerCheckIntervalMillis = preservationInterval.dividedBy(10).toMillis(); Duration preservationGracePeriod = getGracePeriod(); log.info("Starting preservation of audit trails every {} after grace period of {}.", TimeUtils.durationToHuman(preservationInterval), TimeUtils.durationToHuman(preservationGracePeriod)); timer = new Timer(true); preservationTask = new AuditPreservationTimerTask(preservationInterval.toMillis(), Math.toIntExact(preservationGracePeriod.toMillis())); - timer.scheduleAtFixedRate(preservationTask, preservationGracePeriod.toMillis(), timerCheckIntervalMillis); + timer.scheduleAtFixedRate(preservationTask, preservationGracePeriod.toMillis(), preservationInterval.toMillis()); } @Override @@ -285,13 +285,11 @@ public Date getLastPreservationFinish() { @Override public void run() { - if (getNextScheduledRun().getTime() < System.currentTimeMillis()) { - log.info("Starting preservation of audit trails."); - schedule.start(); - preserveRepositoryAuditTrails(); - schedule.finish(); - log.info("Finished preservation. Scheduled new preservation task to start {}", getNextScheduledRun()); - } + log.info("Starting preservation of audit trails."); + schedule.start(); + preserveRepositoryAuditTrails(); + schedule.finish(); + log.info("Finished preservation. Scheduled new preservation task to start {}", getNextScheduledRun()); } } }