Create persistent task files safely Use a temporary directory to create the files in first to avoid partially written files being exposed in the waiting directory. Change-Id: I82222e16b52926ced304cb24eadd1c8d56cf9a7a
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java index 5dc8f73..e9a60e4 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java
@@ -141,7 +141,7 @@ for (Destination cfg : destinations.get().getAll(FilterType.ALL)) { if (cfg.wouldPushProject(project) && cfg.wouldPushRef(refName)) { for (URIish uri : cfg.getURIs(project, urlMatch)) { - replicationTasksStorage.persist( + replicationTasksStorage.create( new ReplicateRefUpdate(project.get(), refName, uri, cfg.getRemoteConfigName())); cfg.schedule(project, refName, uri, state, now); }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationTasksStorage.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationTasksStorage.java index 950ef26..b069508 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationTasksStorage.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationTasksStorage.java
@@ -46,8 +46,9 @@ * task: * * <p><code> - * .../running/<sha1> running replication tasks - * .../waiting/<sha1> outstanding replication tasks + * .../building/<tmp_name> new replication tasks under construction + * .../running/<sha1> running replication tasks + * .../waiting/<sha1> outstanding replication tasks * </code> * * <p>Tasks are moved atomically via a rename between those directories to indicate the current @@ -84,18 +85,20 @@ private static Gson GSON = new Gson(); + private final Path buildingUpdates; private final Path runningUpdates; private final Path waitingUpdates; @Inject ReplicationTasksStorage(ReplicationConfig config) { Path refUpdates = config.getEventsDirectory().resolve("ref-updates"); + buildingUpdates = refUpdates.resolve("building"); runningUpdates = refUpdates.resolve("running"); waitingUpdates = refUpdates.resolve("waiting"); } - public String persist(ReplicateRefUpdate r) { - return new Task(r).persist(); + public String create(ReplicateRefUpdate r) { + return new Task(r).create(); } @VisibleForTesting @@ -175,16 +178,19 @@ waiting = createDir(waitingUpdates).resolve(taskKey); } - public String persist() { + public String create() { if (Files.exists(waiting)) { return taskKey; } try { - logger.atFine().log("CREATE %s %s", waiting, updateLog()); - Files.write(waiting, json.getBytes(UTF_8)); + Path tmp = Files.createTempFile(createDir(buildingUpdates), taskKey, null); + logger.atFine().log("CREATE %s %s", tmp, updateLog()); + Files.write(tmp, json.getBytes(UTF_8)); + logger.atFine().log("RENAME %s %s %s", tmp, waiting, updateLog()); + rename(tmp, waiting); } catch (IOException e) { - logger.atWarning().withCause(e).log("Couldn't persist task %s", json); + logger.atWarning().withCause(e).log("Couldn't create task %s", json); } return taskKey; }