modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/DriftClient.java
| 3
modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/DriftFilesSender.java
| 107 ++++++++--
modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/DriftManager.java
| 82 +------
modules/core/plugin-container/src/test/java/org/rhq/core/pc/drift/DriftFilesSenderTest.java
| 85 ++++++-
modules/core/plugin-container/src/test/java/org/rhq/core/pc/drift/DriftManagerTest.java
| 53 ----
5 files changed, 179 insertions(+), 151 deletions(-)
New commits:
commit 9f18aee38ed44f992ce26e7f8141a057af716533
Author: John Sanda <jsanda(a)redhat.com>
Date: Wed Dec 14 20:59:56 2011 -0500
[BZ 767205] Only read snapshot file once when looking for content to send to server
With this commit DriftFilesSender now builds an index of the file
entries key offed of the SHA so that is can quickly find files without
having to repeatedly scan the snapshot file on disk.
diff --git
a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/DriftClient.java
b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/DriftClient.java
index 1649819..761b3df 100644
--- a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/DriftClient.java
+++ b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/DriftClient.java
@@ -24,7 +24,8 @@ public interface DriftClient {
* @param driftDefinitionName
* @param contentDir
*/
- void sendChangeSetContentToServer(int resourceId, String driftDefinitionName, File
contentDir);
+ //void sendChangeSetContentToServer(int resourceId, String driftDefinitionName, File
contentDir);
+ void sendChangeSetContentToServer(int resourceId, String driftDefName, File
contentZipFile);
void repeatChangeSet(int resourceId, String driftDefName, int version);
diff --git
a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/DriftFilesSender.java
b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/DriftFilesSender.java
index 4181472..59d3054 100644
---
a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/DriftFilesSender.java
+++
b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/DriftFilesSender.java
@@ -10,6 +10,10 @@ import org.rhq.core.util.stream.StreamUtil;
import java.io.*;
import java.util.List;
+import java.util.Map;
+import java.util.TreeMap;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipOutputStream;
public class DriftFilesSender implements Runnable {
@@ -47,41 +51,95 @@ public class DriftFilesSender implements Runnable {
@Override
public void run() {
+ ZipOutputStream stream = null;
try {
if (log.isInfoEnabled()) {
log.info("Preparing to send content to server for " +
defToString());
}
-
+ long startTime = System.currentTimeMillis();
File changeSet = changeSetMgr.findChangeSet(resourceId,
headers.getDriftDefinitionName());
- File contentDir = new File(changeSet.getParentFile(), "content");
- contentDir.mkdir();
-
- for (DriftFile driftFile : driftFiles) {
+ File changeSetDir = changeSet.getParentFile();
+
+ // Note that the content file has a specific format that the server
+ // expects. The file name is of the form content_<token>.zip where
+ // token is a unique string that the agent can use to identify the
+ // content zip file when the server sends an acknowledgement. The
+ // token is necessary because it is possible, albeit not likely,
+ // for a a particular definition to wind up with multiple content
+ // zip files and there is no guarantee that they will be acked in
+ // the order in which they were sent. The name of each file in the
+ // zip file should be the SHA for that file. The server uses that
+ // SHA to look up the DriftFile object it has already created for
+ // the content.
+ //
+ // jsanda
+
+ String timestamp = Long.toString(System.currentTimeMillis());
+ String contentFileName = "content_" + timestamp +
".zip";
+ final File zipFile = new File(changeSetDir, contentFileName);
+ stream = new ZipOutputStream(new BufferedOutputStream(new
FileOutputStream(zipFile)));
+
+ if (driftFiles.size() == 1) {
+ DriftFile driftFile = driftFiles.get(0);
File file = find(driftFile);
if (file == null || !file.exists()) {
log.warn("Unable to find file for " + driftFile);
} else {
if (log.isDebugEnabled()) {
- log.debug("Copying " + file.getPath() + " to
" + contentDir.getPath());
+ log.debug("Adding " + file.getPath() + " to "
+ contentFileName);
}
- StreamUtil.copy(new BufferedInputStream(new FileInputStream(file)),
new BufferedOutputStream(
- new FileOutputStream(new File(contentDir,
driftFile.getHashId()))), true);
- }
- }
-
- File[] content = contentDir.listFiles();
- if (content == null || content.length == 0) {
- if (log.isDebugEnabled()) {
- log.debug("Could not find any of the requested content for
" + defToString());
+ addFileToContentZipFile(stream, driftFile, file);
}
} else {
- if (log.isDebugEnabled()) {
- log.debug("Sending " + content.length + " files to the
server for " + defToString());
+ Map<String, FileEntry> fileEntries = createSnapshotIndex();
+
+ for (DriftFile driftFile : driftFiles) {
+ FileEntry entry = fileEntries.get(driftFile.getHashId());
+ if (entry == null) {
+ continue;
+ }
+ File file = new File(headers.getBasedir(), entry.getFile());
+ if (file == null || !file.exists()) {
+ log.warn("Unable to find file for " + driftFile);
+ } else {
+ if (log.isDebugEnabled()) {
+ log.debug("Adding " + file.getPath() + " to
" + contentFileName);
+ }
+ addFileToContentZipFile(stream, driftFile, file);
+ }
}
- driftClient.sendChangeSetContentToServer(resourceId,
headers.getDriftDefinitionName(), contentDir);
}
+
+ stream.close();
+ stream = null;
+
+ driftClient.sendChangeSetContentToServer(resourceId,
headers.getDriftDefinitionName(), zipFile);
+
+ if (log.isInfoEnabled()) {
+ long endTime = System.currentTimeMillis();
+ log.info("Finished submitting request to send content to server in
" + (endTime - startTime) +
+ " ms");
+ }
+
} catch (IOException e) {
log.error("Failed to send drift files.", e);
+ } finally {
+ if (stream != null) {
+ try {
+ stream.close();
+ } catch (IOException e) {
+ }
+ }
+ }
+ }
+
+ private void addFileToContentZipFile(ZipOutputStream stream, DriftFile driftFile,
File file) throws IOException {
+ FileInputStream fis = new FileInputStream(file);
+ try {
+ stream.putNextEntry(new ZipEntry(driftFile.getHashId()));
+ StreamUtil.copy(fis, stream, false);
+ } finally {
+ fis.close();
}
}
@@ -100,6 +158,19 @@ public class DriftFilesSender implements Runnable {
}
}
+ private Map<String, FileEntry> createSnapshotIndex() throws IOException {
+ ChangeSetReader reader = changeSetMgr.getChangeSetReader(resourceId,
headers.getDriftDefinitionName());
+ try {
+ Map<String, FileEntry> map = new TreeMap<String, FileEntry>();
+ for (FileEntry entry : reader) {
+ map.put(entry.getNewSHA(), entry);
+ }
+ return map;
+ } finally {
+ reader.close();
+ }
+ }
+
private String defToString() {
return "[resourceId: " + resourceId + ", driftDefinitionId: "
+ headers.getDriftDefinitionId() +
", driftDefinitionName: " + headers.getDriftDefinitionName() +
"]";
diff --git
a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/DriftManager.java
b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/DriftManager.java
index 2790f13..3602267 100644
--- a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/DriftManager.java
+++ b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/DriftManager.java
@@ -19,30 +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.domain.drift.DriftComplianceStatus.OUT_OF_COMPLIANCE_NO_BASEDIR;
-
-import java.io.BufferedInputStream;
-import java.io.BufferedOutputStream;
-import java.io.File;
-import java.io.FileInputStream;
-import java.io.FileNotFoundException;
-import java.io.FileOutputStream;
-import java.io.FilenameFilter;
-import java.io.IOException;
-import java.util.List;
-import java.util.concurrent.ScheduledThreadPoolExecutor;
-import java.util.concurrent.TimeUnit;
-import java.util.zip.ZipEntry;
-import java.util.zip.ZipOutputStream;
-
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
-
import org.rhq.common.drift.ChangeSetWriter;
import org.rhq.common.drift.Headers;
import org.rhq.core.clientapi.agent.drift.DriftAgentService;
@@ -63,6 +41,18 @@ import org.rhq.core.pc.measurement.MeasurementManager;
import org.rhq.core.util.file.FileUtil;
import org.rhq.core.util.stream.StreamUtil;
+import java.io.*;
+import java.util.List;
+import java.util.concurrent.ScheduledThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipOutputStream;
+
+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.domain.drift.DriftComplianceStatus.OUT_OF_COMPLIANCE_NO_BASEDIR;
+
public class DriftManager extends AgentService implements DriftAgentService, DriftClient,
ContainerService {
private final Log log = LogFactory.getLog(DriftManager.class);
@@ -252,7 +242,8 @@ public class DriftManager extends AgentService implements
DriftAgentService, Dri
if (log.isDebugEnabled()) {
log.debug("Resending " + contentZipFile.getPath());
}
- sendContentZipFile(Integer.parseInt(resourceDir.getName()),
defDir.getName(), contentZipFile);
+ sendChangeSetContentToServer(Integer.parseInt(resourceDir.getName()),
defDir.getName(),
+ contentZipFile);
}
}
}
@@ -343,46 +334,7 @@ public class DriftManager extends AgentService implements
DriftAgentService, Dri
}
@Override
- public void sendChangeSetContentToServer(int resourceId, String driftDefinitionName,
final File contentDir) {
- ZipOutputStream stream = null;
- try {
- String timestamp = Long.toString(System.currentTimeMillis());
- String contentFileName = "content_" + timestamp +
".zip";
- final File zipFile = new File(contentDir.getParentFile(), contentFileName);
- stream = new ZipOutputStream(new BufferedOutputStream(new
FileOutputStream(zipFile)));
-
- for (File file : contentDir.listFiles()) {
- FileInputStream fis = new FileInputStream(file);
- try {
- stream.putNextEntry(new ZipEntry(file.getName()));
- StreamUtil.copy(fis, stream, false);
- } finally {
- fis.close();
- }
- }
- stream.close();
- stream = null;
- sendContentZipFile(resourceId, driftDefinitionName, zipFile);
- } catch (IOException e) {
- log.error("An error occurred while trying to send content for
changeset[resourceId: " + resourceId
- + ", driftDefinition: " + driftDefinitionName + "]",
e);
- } finally {
- if (stream != null) {
- try {
- stream.close();
- } catch (IOException e) {
- }
- }
- }
-
- for (File file : contentDir.listFiles()) {
- if (!file.delete()) {
- log.warn("Unable to clean up content directory. Failed to delete
" + file.getPath());
- }
- }
- }
-
- private void sendContentZipFile(int resourceId, String driftDefName, File
contentZipFile) {
+ public void sendChangeSetContentToServer(int resourceId, String driftDefName, File
contentZipFile) {
try {
int startIndex = "content_".length();
int endIndex = contentZipFile.getName().indexOf(".");
@@ -390,10 +342,10 @@ public class DriftManager extends AgentService implements
DriftAgentService, Dri
DriftServerService driftServer =
pluginContainerConfiguration.getServerServices().getDriftServerService();
driftServer.sendFilesZip(resourceId, driftDefName, token,
contentZipFile.length(),
- remoteInputStream(new BufferedInputStream(new
FileInputStream(contentZipFile))));
+ remoteInputStream(new BufferedInputStream(new
FileInputStream(contentZipFile))));
} catch (FileNotFoundException e) {
log.error("An error occurred while trying to send change set content zip
file " + contentZipFile.getPath()
- + " to server.", e);
+ + " to server.", e);
}
}
diff --git
a/modules/core/plugin-container/src/test/java/org/rhq/core/pc/drift/DriftFilesSenderTest.java
b/modules/core/plugin-container/src/test/java/org/rhq/core/pc/drift/DriftFilesSenderTest.java
index 77db6b2..d427229 100644
---
a/modules/core/plugin-container/src/test/java/org/rhq/core/pc/drift/DriftFilesSenderTest.java
+++
b/modules/core/plugin-container/src/test/java/org/rhq/core/pc/drift/DriftFilesSenderTest.java
@@ -24,12 +24,19 @@ import org.rhq.common.drift.Headers;
import org.rhq.core.domain.drift.DriftChangeSetCategory;
import org.rhq.core.domain.drift.DriftFile;
import org.rhq.core.domain.drift.JPADriftFile;
+import org.rhq.core.util.ZipUtil;
+import org.rhq.test.AssertUtils;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
import java.io.File;
+import java.io.FilenameFilter;
import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.LinkedList;
import java.util.List;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipInputStream;
import static org.rhq.common.drift.FileEntry.addedFileEntry;
import static org.rhq.core.domain.drift.DriftChangeSetCategory.COVERAGE;
@@ -53,6 +60,27 @@ public class DriftFilesSenderTest extends DriftTest {
}
@Test
+ public void sendOneFile() throws Exception {
+ String driftDefName = "send-one-file-test";
+ File serverConf = createRandomFile(resourceDir, "server.conf");
+ String serverConfHash = sha256(serverConf);
+
+ File changeSetDir = changeSetDir(driftDefName);
+ Headers headers = createHeaders(driftDefName, COVERAGE);
+
+ ChangeSetWriter writer = changeSetMgr.getChangeSetWriter(resourceId(), headers);
+ writer.write(addedFileEntry("server.conf", serverConfHash));
+ writer.close();
+
+ sender.setDriftFiles(driftFiles(serverConfHash));
+ sender.setHeaders(headers);
+ sender.run();
+
+ assertContentFileExists(changeSetDir);
+ assertContentFileMatches(changeSetDir, serverConfHash);
+ }
+
+ @Test
public void sendFilesInOneDirectory() throws Exception {
String driftDefName = "single-directory-test";
@@ -73,14 +101,11 @@ public class DriftFilesSenderTest extends DriftTest {
writer.close();
sender.setDriftFiles(driftFiles(server1ConfHash, server2ConfHash));
- sender.setHeaders(createHeaders(driftDefName, COVERAGE));
+ sender.setHeaders(headers);
sender.run();
- File contentDir = mkdir(changeSetDir, "content");
-
- assertEquals(contentDir.list().length, 2, "Expected to find two files in
" + contentDir.getAbsolutePath());
- assertFileCopiedToContentDir(contentDir, server1ConfHash);
- assertFileCopiedToContentDir(contentDir, server2ConfHash);
+ assertContentFileExists(changeSetDir);
+ assertContentFileMatches(changeSetDir, server1ConfHash, server2ConfHash);
}
@Test
@@ -113,16 +138,11 @@ public class DriftFilesSenderTest extends DriftTest {
// Note that the order of the drift files is random. When the server sends a
request
// for files we cannot assume that the files will be in any particular order.
sender.setDriftFiles(driftFiles(server1JarHash, server2ConfHash, server2JarHash,
server1ConfHash));
- sender.setHeaders(createHeaders(driftDefName, COVERAGE));
+ sender.setHeaders(headers);
sender.run();
- File contentDir = mkdir(changeSetDir, "content");
-
- assertEquals(contentDir.list().length, 4, "Expected to find four files in
" + contentDir.getAbsolutePath());
- assertFileCopiedToContentDir(contentDir, server1ConfHash);
- assertFileCopiedToContentDir(contentDir, server2ConfHash);
- assertFileCopiedToContentDir(contentDir, server1JarHash);
- assertFileCopiedToContentDir(contentDir, server2JarHash);
+ assertContentFileExists(changeSetDir);
+ assertContentFileMatches(changeSetDir, server1ConfHash, server2ConfHash,
server1JarHash, server2JarHash);
}
@Test
@@ -154,10 +174,8 @@ public class DriftFilesSenderTest extends DriftTest {
sender.setHeaders(headers);
sender.run();
- File contentDir = mkdir(changeSetDir, "content");
-
- assertEquals(contentDir.list().length, 1, "Expected to find one file in
" + contentDir.getAbsolutePath());
- assertFileCopiedToContentDir(contentDir, server2ConfHash);
+ assertContentFileExists(changeSetDir);
+ assertContentFileMatches(changeSetDir, server2ConfHash);
}
@Test
@@ -183,6 +201,37 @@ public class DriftFilesSenderTest extends DriftTest {
assertEquals(driftClient.getSendChangeSetContentInvocationCount(), 0,
"Do not call DriftClient to send content to server when there is no
content to send.");
+ assertContentFileExists(changeSetDir(driftDefName));
+ }
+
+ void assertContentFileExists(File changeSetDir) {
+ File[] files = getContentFiles(changeSetDir);
+ assertEquals(files.length, 1, "Expected to find a single content zip file
but found " + Arrays.toString(files));
+ }
+
+ void assertContentFileMatches(File changeSetDir, String... expectedFileNames) throws
Exception {
+ File contentFile = getContentFiles(changeSetDir)[0];
+ final List<String> actualFileNames = new LinkedList<String>();
+
+ ZipUtil.walkZipFile(contentFile, new ZipUtil.ZipEntryVisitor() {
+ @Override
+ public boolean visit(ZipEntry entry, ZipInputStream stream) throws Exception
{
+ actualFileNames.add(entry.getName());
+ return true;
+ }
+ });
+
+ AssertUtils.assertCollectionEqualsNoOrder(Arrays.asList(expectedFileNames),
actualFileNames,
+ "The content file " + contentFile.getPath() + " does not
contain the correct entries");
+ }
+
+ private File[] getContentFiles(File changeSetDir) {
+ return changeSetDir.listFiles(new FilenameFilter() {
+ @Override
+ public boolean accept(File dir, String name) {
+ return name.startsWith("content_") &&
name.endsWith(".zip");
+ }
+ });
}
/**
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 c3e28f5..e1ec00d 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
@@ -133,7 +133,8 @@ public class DriftManagerTest extends DriftTest {
File changeSetDir = changeSetDir(config.getName());
File snapshotFile = createRandomFile(changeSetDir, "changeset.txt");
File previousSnapshotFile = createRandomFile(changeSetDir,
"changeset.txt.previous");
- File changeSetZipFile = createRandomFile(changeSetDir, "changeset_" +
System.currentTimeMillis() + ".zip");
+
+ createRandomFile(changeSetDir, "changeset_" +
System.currentTimeMillis() + ".zip");
driftMgr.ackChangeSet(resourceId(), config.getName());
@@ -145,53 +146,11 @@ public class DriftManagerTest extends DriftTest {
}
@Test
- public void writeContentZipFileToChangeSetContentDirectory() throws Exception {
- String configName = "send-content-in-zip";
- final File changeSetDir = changeSetDir(configName);
- final File contentDir = mkdir(changeSetDir, "content");
-
- createRandomFile(contentDir, "content-1");
- createRandomFile(contentDir, "content-2");
-
- setDriftServiceCallback(new DriftServiceCallback() {
- @Override
- public void execute() {
- assertThatZipFileExists(changeSetDir, "content_",
"Expected to find content zip file in "
- + changeSetDir.getPath() + ". The file name should follow the
pattern "
- + "content_<integer_timestamp>.zip");
- }
- });
-
- driftMgr.sendChangeSetContentToServer(resourceId(), configName, contentDir);
- }
-
- @Test
- public void sendContentToServerInZipFile() throws Exception {
- String configName = "send-content-in-zip";
- File changeSetDir = changeSetDir(configName);
- File contentDir = mkdir(changeSetDir, "content");
-
- final File content1 = createRandomFile(contentDir, "content-1");
- final File content2 = createRandomFile(contentDir, "content-2");
-
- setDriftServiceCallback(new DriftServiceCallback() {
- @Override
- public void execute() {
- assertZipFileMatches(driftServerService.inputStream, content1,
content2);
- }
- });
-
- driftMgr.sendChangeSetContentToServer(resourceId(), configName, contentDir);
-
- // verify that the content directory is purged
- assertThatDirectoryIsEmpty(contentDir);
- }
-
- @Test
public void cleanUpWhenServerAcksChangeSetContent() throws Exception {
String configName = "cleanup-when-server-acks-content";
File changeSetDir = changeSetDir(configName);
- File contentDir = mkdir(changeSetDir, "content");
+
+ mkdir(changeSetDir, "content");
String token = Long.toString(System.currentTimeMillis());
File contentZipFile = createRandomFile(changeSetDir, "content_" + token
+ ".zip");
@@ -309,10 +268,6 @@ public class DriftManagerTest extends DriftTest {
}
}
- private void assertThatDirectoryIsEmpty(File dir) {
- assertEquals(dir.listFiles().length, 0, "Expected " + dir.getPath() +
" to be empty");
- }
-
private File findFile(File dir, File file) {
for (File f : dir.listFiles()) {
if (f.getName().equals(file.getName())) {