modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/DriftDetector.java | 51 ++--- modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/ScheduleQueue.java | 2 modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/ScheduleQueueImpl.java | 31 ++- modules/core/plugin-container/src/test/java/org/rhq/core/pc/drift/DriftDetectorTest.java | 94 ++++++---- modules/core/plugin-container/src/test/java/org/rhq/core/pc/drift/DriftManagerTest.java | 2 5 files changed, 117 insertions(+), 63 deletions(-)
New commits: commit 518ccccdc4ad05353de809068c289c1fc64a5384 Author: John Sanda jsanda@redhat.com Date: Thu Dec 8 13:39:47 2011 -0500
[BZ 761320] Only update the detection schedule if drift detection runs
With this commit we no longer update the detection schedule's next scan time if detection does not run. Drift detection might not run for a few different reasons - the definition could be disabled, it it earlier than the next scheduled run, or the server has not acknowledged the previous change set sent by the agent.
Some additional debug logging has also been added to output when the next scheduled run is supposed to happen.
diff --git a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/DriftDetector.java b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/DriftDetector.java index 71ddf44..21341c4 100644 --- a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/DriftDetector.java +++ b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/DriftDetector.java @@ -19,26 +19,8 @@
package org.rhq.core.pc.drift;
-import static org.rhq.common.drift.FileEntry.addedFileEntry; -import static org.rhq.common.drift.FileEntry.changedFileEntry; -import static org.rhq.common.drift.FileEntry.removedFileEntry; -import static org.rhq.core.domain.drift.DriftChangeSetCategory.COVERAGE; -import static org.rhq.core.domain.drift.DriftChangeSetCategory.DRIFT; -import static org.rhq.core.util.file.FileUtil.copyFile; -import static org.rhq.core.util.file.FileUtil.forEachFile; - -import java.io.File; -import java.io.FilenameFilter; -import java.io.IOException; -import java.util.HashMap; -import java.util.HashSet; -import java.util.LinkedList; -import java.util.List; -import java.util.Set; - import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; - import org.rhq.common.drift.ChangeSetReader; import org.rhq.common.drift.ChangeSetWriter; import org.rhq.common.drift.FileEntry; @@ -49,6 +31,17 @@ import org.rhq.core.domain.drift.Filter; import org.rhq.core.util.MessageDigestGenerator; import org.rhq.core.util.file.FileVisitor;
+import java.io.File; +import java.io.FilenameFilter; +import java.io.IOException; +import java.util.*; + +import static org.rhq.common.drift.FileEntry.*; +import static org.rhq.core.domain.drift.DriftChangeSetCategory.COVERAGE; +import static org.rhq.core.domain.drift.DriftChangeSetCategory.DRIFT; +import static org.rhq.core.util.file.FileUtil.copyFile; +import static org.rhq.core.util.file.FileUtil.forEachFile; + public class DriftDetector implements Runnable { private Log log = LogFactory.getLog(DriftDetector.class);
@@ -76,14 +69,17 @@ public class DriftDetector implements Runnable { public void run() { log.debug("Starting drift detection..."); long startTime = System.currentTimeMillis(); + boolean updateSchedule = true; + DriftDetectionSchedule schedule = null; try { if (log.isDebugEnabled()) { log.debug("Fetching next schedule from " + scheduleQueue); }
- DriftDetectionSchedule schedule = scheduleQueue.getNextSchedule(); + schedule = scheduleQueue.getNextSchedule(); if (schedule == null) { log.debug("No schedules are in the queue."); + updateSchedule = false; return; }
@@ -93,17 +89,26 @@ public class DriftDetector implements Runnable { }
if (schedule.getNextScan() > (System.currentTimeMillis() + 100L)) { - log.debug("Skipping " + schedule + " because it is too early to do the next detection."); + if (log.isDebugEnabled()) { + log.debug("Skipping " + schedule + " because it is too early to do the next detection."); + } + updateSchedule = false; return; }
if (!schedule.getDriftDefinition().isEnabled()) { - log.debug("Skipping " + schedule + " because the drift definition is disabled."); + if (log.isDebugEnabled()) { + log.debug("Skipping " + schedule + " because the drift definition is disabled."); + } + updateSchedule = false; return; }
if (previousSnapshotExists(schedule)) { - log.debug("Skipping " + schedule + " because server has not yet acked previous change set"); + if (log.isDebugEnabled()) { + log.debug("Skipping " + schedule + " because server has not yet acked previous change set"); + } + updateSchedule = false; return; }
@@ -140,7 +145,7 @@ public class DriftDetector implements Runnable {
} finally { try { - scheduleQueue.deactivateSchedule(); + scheduleQueue.deactivateSchedule(updateSchedule); long endTime = System.currentTimeMillis(); log.debug("Finished drift detection in " + (endTime - startTime) + " ms");
diff --git a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/ScheduleQueue.java b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/ScheduleQueue.java index 35960ad..13f078d 100644 --- a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/ScheduleQueue.java +++ b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/ScheduleQueue.java @@ -35,7 +35,7 @@ public interface ScheduleQueue { * at the head of the queue to become active. If there is no active schedule this * method simply does nothing and returns. */ - void deactivateSchedule(); + void deactivateSchedule(boolean updateSchedule);
/** * Adds a schedule to the queue for processing by the drift detector diff --git a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/ScheduleQueueImpl.java b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/ScheduleQueueImpl.java index cbc50b8..d008f36 100644 --- a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/ScheduleQueueImpl.java +++ b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/ScheduleQueueImpl.java @@ -1,15 +1,14 @@ package org.rhq.core.pc.drift;
-import java.util.ArrayList; -import java.util.Arrays; -import java.util.Iterator; -import java.util.List; -import java.util.PriorityQueue; -import java.util.concurrent.locks.ReentrantReadWriteLock; - +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.rhq.core.domain.drift.DriftDefinition; import org.rhq.core.domain.drift.DriftDefinitionComparator;
+import java.text.SimpleDateFormat; +import java.util.*; +import java.util.concurrent.locks.ReentrantReadWriteLock; + public class ScheduleQueueImpl implements ScheduleQueue {
private static final Runnable NO_OP = new Runnable() { @@ -26,6 +25,8 @@ public class ScheduleQueueImpl implements ScheduleQueue {
private Runnable deactivationTask;
+ private Log log = LogFactory.getLog(ScheduleQueueImpl.class); + @Override public DriftDetectionSchedule getNextSchedule() { try { @@ -66,16 +67,28 @@ public class ScheduleQueueImpl implements ScheduleQueue { }
@Override - public void deactivateSchedule() { + public void deactivateSchedule(boolean updateSchedule) { try { lock.writeLock().lock(); if (deactivationTask != null) { deactivationTask.run(); } + if (activeSchedule == null) { return; } - activeSchedule.updateShedule(); + + if (updateSchedule) { + activeSchedule.updateShedule(); + } + + if (log.isDebugEnabled()) { + SimpleDateFormat formatter = new SimpleDateFormat(); + formatter.applyPattern("hh:mm:ss:SSS a"); + log.debug("The next drift detection run for " + activeSchedule + " is set for " + + formatter.format(new Date(activeSchedule.getNextScan()))); + } + queue.offer(activeSchedule); activeSchedule = null; } finally { diff --git a/modules/core/plugin-container/src/test/java/org/rhq/core/pc/drift/DriftDetectorTest.java b/modules/core/plugin-container/src/test/java/org/rhq/core/pc/drift/DriftDetectorTest.java index 4a2b333..08373ec 100644 --- a/modules/core/plugin-container/src/test/java/org/rhq/core/pc/drift/DriftDetectorTest.java +++ b/modules/core/plugin-container/src/test/java/org/rhq/core/pc/drift/DriftDetectorTest.java @@ -19,22 +19,12 @@
package org.rhq.core.pc.drift;
-import static java.util.Arrays.asList; -import static java.util.Collections.emptyList; -import static org.apache.commons.io.FileUtils.copyFile; -import static org.apache.commons.io.FileUtils.deleteDirectory; -import static org.apache.commons.io.FileUtils.touch; -import static org.rhq.common.drift.FileEntry.addedFileEntry; -import static org.rhq.common.drift.FileEntry.changedFileEntry; -import static org.rhq.common.drift.FileEntry.removedFileEntry; -import static org.rhq.core.domain.drift.DriftChangeSetCategory.COVERAGE; -import static org.rhq.core.domain.drift.DriftChangeSetCategory.DRIFT; -import static org.rhq.test.AssertUtils.assertCollectionMatchesNoOrder; -import static org.rhq.test.AssertUtils.assertPropertiesMatch; -import static org.testng.Assert.assertEquals; -import static org.testng.Assert.assertFalse; -import static org.testng.Assert.assertTrue; -import static org.testng.Assert.fail; +import org.rhq.common.drift.*; +import org.rhq.core.domain.drift.DriftDefinition; +import org.rhq.core.system.OperatingSystemType; +import org.rhq.core.system.SystemInfoFactory; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test;
import java.io.BufferedReader; import java.io.File; @@ -43,18 +33,15 @@ import java.util.ArrayList; import java.util.List; import java.util.concurrent.atomic.AtomicBoolean;
-import org.testng.annotations.BeforeMethod; -import org.testng.annotations.Test; - -import org.rhq.common.drift.ChangeSetReader; -import org.rhq.common.drift.ChangeSetReaderImpl; -import org.rhq.common.drift.ChangeSetWriter; -import org.rhq.common.drift.ChangeSetWriterImpl; -import org.rhq.common.drift.FileEntry; -import org.rhq.common.drift.Headers; -import org.rhq.core.domain.drift.DriftDefinition; -import org.rhq.core.system.OperatingSystemType; -import org.rhq.core.system.SystemInfoFactory; +import static java.util.Arrays.asList; +import static java.util.Collections.emptyList; +import static org.apache.commons.io.FileUtils.*; +import static org.rhq.common.drift.FileEntry.*; +import static org.rhq.core.domain.drift.DriftChangeSetCategory.COVERAGE; +import static org.rhq.core.domain.drift.DriftChangeSetCategory.DRIFT; +import static org.rhq.test.AssertUtils.assertCollectionMatchesNoOrder; +import static org.rhq.test.AssertUtils.assertPropertiesMatch; +import static org.testng.Assert.*;
public class DriftDetectorTest extends DriftTest {
@@ -168,6 +155,30 @@ public class DriftDetectorTest extends DriftTest { changeSet); }
+ @Test + public void updateScheduleAfterGeneratingCoverageChangeSet() throws Exception { + DriftDefinition driftDef = driftDefinition("update-schedule-after-coverage-changeset", + resourceDir.getAbsolutePath()); + DriftDetectionSchedule schedule = new DriftDetectionSchedule(resourceId(), driftDef); + + File confDir = mkdir(resourceDir, "conf"); + File serverConf = createRandomFile(confDir, "server.conf"); + + long currentTime = System.currentTimeMillis(); + + scheduleQueue.addSchedule(schedule); + detector.run(); + + assertTrue(schedule.getNextScan() >= (currentTime + driftDef.getInterval()), + "Failed to update schedule. next scan is " + schedule.getNextScan() + " and should be greater than " + + (currentTime + driftDef.getInterval())); + } + + @Test + public void updateScheduleAfterGeneratingDriftChangeSet() throws Exception { + + } + @SuppressWarnings("unchecked") @Test public void doNotUpdateSnapshotOrGenerateDriftChangeSetIfNothingChanges() throws Exception { @@ -219,12 +230,37 @@ public class DriftDetectorTest extends DriftTest { DriftDefinition def = driftDefinition("disabled-config-test", resourceDir.getAbsolutePath()); def.setEnabled(false);
+ DriftDetectionSchedule schedule = new DriftDetectionSchedule(resourceId(), def); + long nextScan = schedule.getNextScan(); + File confDir = mkdir(resourceDir, "conf"); File server1Conf = new File(confDir, "server-1.conf"); touch(server1Conf);
- scheduleQueue.addSchedule(new DriftDetectionSchedule(resourceId(), def)); + scheduleQueue.addSchedule(schedule); + detector.run(); + + // make sure that the next scan time is not updated + assertEquals(schedule.getNextScan(), nextScan, "The next scan time for the drift detection schedule should " + + " not get updated if drift detection does not actually run for the definition."); + } + + @Test + public void doNotUpdateScheduleIfItIsTooEarlyToRunDetection() throws Exception { + DriftDefinition driftDef = driftDefinition("schedule-not-ready-test", resourceDir.getAbsolutePath()); + DriftDetectionSchedule schedule = new DriftDetectionSchedule(resourceId(), driftDef); + + File confDir = mkdir(resourceDir, "conf"); + File server1Conf = new File(confDir, "server.conf"); + + scheduleQueue.addSchedule(schedule); detector.run(); + + long nextScan = schedule.getNextScan(); + detector.run(); + + assertEquals(schedule.getNextScan(), nextScan, "The next scan time for the drift detection schedule should " + + " not get updated if drift detection does not actually run for the definition."); }
@Test diff --git a/modules/core/plugin-container/src/test/java/org/rhq/core/pc/drift/DriftManagerTest.java b/modules/core/plugin-container/src/test/java/org/rhq/core/pc/drift/DriftManagerTest.java index 518e8e4..c3e28f5 100644 --- a/modules/core/plugin-container/src/test/java/org/rhq/core/pc/drift/DriftManagerTest.java +++ b/modules/core/plugin-container/src/test/java/org/rhq/core/pc/drift/DriftManagerTest.java @@ -260,7 +260,7 @@ public class DriftManagerTest extends DriftTest { assertTrue(changeSetDir.exists(), "The change set directory should not be deleted while the schedule is " + "still active.");
- driftMgr.getSchedulesQueue().deactivateSchedule(); + driftMgr.getSchedulesQueue().deactivateSchedule(false); assertFalse(changeSetDir.exists(), "The change set directory should have been deleted after the schedule is " + "deactivated."); }
rhq-commits@lists.fedorahosted.org