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(a)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.");
}