Skip to content

Commit

Permalink
Merge pull request #187 from jglick/lastLocation
Browse files Browse the repository at this point in the history 
More efficient `lastLocation` handling in watch mode
  • Loading branch information
@jglick
jglick authored Aug 18, 2023
2 parents 989bc64 + 9445f80 commit aa22cf1
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 12 deletions.
18 changes: 18 additions & 0 deletions pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,4 +73,22 @@
</dependency>
</dependencies>
</dependencyManagement>
<build>
<pluginManagement>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<dependencies>
<!-- https://github.com/jenkinsci/plugin-pom/pull/560#issuecomment-1656232490 -->
<dependency>
<groupId>org.apache.maven.surefire</groupId>
<artifactId>surefire-junit47</artifactId>
<version>${maven-surefire-plugin.version}</version>
</dependency>
</dependencies>
</plugin>
</plugins>
</pluginManagement>
</build>
</project>
20 changes: 15 additions & 5 deletions src/main/java/org/jenkinsci/plugins/durabletask/FileMonitoringTask.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -602,6 +602,7 @@ private static class Watcher implements Runnable {
private final Handler handler;
private final TaskListener listener;
private final @CheckForNull Charset cs;
private long lastLocation = -1;

Watcher(FileMonitoringController controller, FilePath workspace, Handler handler, TaskListener listener) {
LOGGER.log(Level.FINE, "starting " + this, new Throwable());
Expand All @@ -616,10 +617,17 @@ private static class Watcher implements Runnable {
@Override public void run() {
try {
Integer exitStatus = controller.exitStatus(workspace, listener); // check before collecting output, in case the process is just now finishing
long lastLocation = 0;
FilePath lastLocationFile = controller.getLastLocationFile(workspace);
if (lastLocationFile.exists()) {
lastLocation = Long.parseLong(lastLocationFile.readToString());
if (lastLocation == -1) {
FilePath lastLocationFile = controller.getLastLocationFile(workspace);
if (lastLocationFile.exists()) {
lastLocation = Long.parseLong(lastLocationFile.readToString());
LOGGER.finest(() -> "Loaded lastLocation=" + lastLocation);
} else {
lastLocation = 0;
LOGGER.finest("New watch, lastLocation=0");
}
} else {
LOGGER.finest(() -> "Using cached lastLocation=" + lastLocation);
}
FilePath logFile = controller.getLogFile(workspace);
long len = logFile.length();
Expand All @@ -630,9 +638,11 @@ private static class Watcher implements Runnable {
InputStream utf8EncodedStream = cs == null ? locallyEncodedStream : new ReaderInputStream(new InputStreamReader(locallyEncodedStream, cs), StandardCharsets.UTF_8);
handler.output(utf8EncodedStream);
long newLocation = ch.position();
lastLocationFile.write(Long.toString(newLocation), null);
// TODO use AtomicFileWriter or equivalent?

Check warning on line 641 in src/main/java/org/jenkinsci/plugins/durabletask/FileMonitoringTask.java

View check run for this annotation

ci.jenkins.io / Open Tasks Scanner

TODO 

NORMAL:
use AtomicFileWriter or equivalent?
controller.getLastLocationFile(workspace).write(Long.toString(newLocation), null);
long delta = newLocation - lastLocation;
LOGGER.finer(() -> this + " copied " + delta + " bytes from " + logFile);
lastLocation = newLocation;
}
}
if (exitStatus != null) {
Expand Down
25 changes: 18 additions & 7 deletions src/test/java/org/jenkinsci/plugins/durabletask/BourneShellScriptTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import hudson.Launcher;
import hudson.Platform;
import hudson.Proc;
import hudson.model.Node;
import hudson.model.Slave;
import hudson.plugins.sshslaves.SSHLauncher;
import hudson.remoting.Channel;
Expand Down Expand Up @@ -86,7 +87,7 @@
import org.jvnet.hudson.test.SimpleCommandLauncher;

enum TestPlatform {
NATIVE, ALPINE, CENTOS, UBUNTU, NO_INIT, UBUNTU_NO_BINARY, SLIM
ON_CONTROLLER, NATIVE, ALPINE, CENTOS, UBUNTU, NO_INIT, UBUNTU_NO_BINARY, SLIM
}

@RunWith(Parameterized.class)
Expand All @@ -113,11 +114,11 @@ static void assumeDocker() throws Exception {
assumeThat("Docker must be at least 1.13.0 for this test (uses --init)", new VersionNumber(baos.toString().trim()), greaterThanOrEqualTo(new VersionNumber("1.13.0")));
}

@Rule public LoggerRule logging = new LoggerRule().recordPackage(BourneShellScript.class, Level.FINE);
@Rule public LoggerRule logging = new LoggerRule().recordPackage(BourneShellScript.class, Level.FINEST);

private TestPlatform platform;
private StreamTaskListener listener;
private Slave s;
private Node s;
private FilePath ws;
private Launcher launcher;

Expand All @@ -128,6 +129,10 @@ public BourneShellScriptTest(TestPlatform platform) throws Exception {

@Before public void prepareAgentForPlatform() throws Exception {
switch (platform) {
case ON_CONTROLLER:
BourneShellScript.USE_BINARY_WRAPPER = true;
s = j.jenkins;
break;
case NATIVE:
BourneShellScript.USE_BINARY_WRAPPER = true;
s = j.createOnlineSlave();
Expand All @@ -141,13 +146,15 @@ public BourneShellScriptTest(TestPlatform platform) throws Exception {
case UBUNTU_NO_BINARY:
assumeDocker();
s = prepareAgentDocker();
j.jenkins.addNode(s);
j.waitOnline(s);
if (s instanceof Slave) {
j.jenkins.addNode(s);
j.waitOnline((Slave) s);
}
break;
default:
throw new AssertionError(platform);
}
ws = s.getWorkspaceRoot().child("ws");
ws = (s instanceof Slave ? ((Slave) s).getWorkspaceRoot() : j.jenkins.getRootPath()).child("ws");
launcher = s.createLauncher(listener);
}

Expand Down Expand Up @@ -206,6 +213,7 @@ private Slave prepareDockerPlatforms() throws Exception {
@Test public void smokeTest() throws Exception {
int sleepSeconds;
switch (platform) {
case ON_CONTROLLER:
case NATIVE:
sleepSeconds = 0;
break;
Expand Down Expand Up @@ -432,6 +440,7 @@ static class MockHandler extends Handler {
@Test public void backgroundLaunch() throws IOException, InterruptedException {
int sleepSeconds;
switch (platform) {
case ON_CONTROLLER:
case NATIVE:
case CENTOS:
case UBUNTU:
Expand Down Expand Up @@ -477,6 +486,7 @@ static class MockHandler extends Handler {
String os;
String architecture;
switch (platform) {
case ON_CONTROLLER:
case NATIVE:
if (Platform.isDarwin()) {
os = "darwin";
Expand Down Expand Up @@ -507,7 +517,7 @@ static class MockHandler extends Handler {
String version = j.getPluginManager().getPlugin("durable-task").getVersion();
version = StringUtils.substringBefore(version, "-");
String binaryName = "durable_task_monitor_" + version + "_" + os + "_" + architecture;
FilePath binaryPath = ws.getParent().getParent().child("caches/durable-task/" + binaryName);
FilePath binaryPath = s.getRootPath().child("caches/durable-task/" + binaryName);
assertFalse(binaryPath.exists());

BourneShellScript script = new BourneShellScript("echo hello");
Expand Down Expand Up @@ -602,6 +612,7 @@ private String psOut(String psFormat) throws InterruptedException, IOException {
private String setPsFormat() {
String cmdCol = null;
switch (platform) {
case ON_CONTROLLER:
case NATIVE:
cmdCol = Platform.isDarwin() ? "comm" : "cmd";
break;
Expand Down

0 comments on commit aa22cf1

Please sign in to comment.