opendev/gerrit
Code Issues Proposed changes

Bugfix: Changing Task state breaks comparator in ShowQueue

Browse Source 
This can happens if you have a long queue and the state of
a task (DONE, CANCELLED, RUNNING, READY, SLEEPING, OTHER)
changes while the sorting is ongoing. The reason this
generates an error is because the Task State defines
the tasks’ place in the queue.

If Task state changes while the sorting of the queue is
ongoing the Comparator violates its contract of:
X<Y, Y<Z => X<Z
and throws:
IllegalArgumentException: Comparison mehtod violates its
general contract!

Fixed this bug by saving a snapshot of the state and delay
of the tasks in a wrapper.
* Introduced interface TaskInfo that is implemented by
  QueueTaskInfo
* Added getTaskInfos method in WorkQueue decoupling it from
  ShowQueue implementation by Interface and factory.

Signed-off-by: Gustaf Lundh <gustaf.lundh@sonymobile.com>
Change-Id: Iea17046aea1b8c6119cfc663438e17f663e05b22
This commit is contained in:
Sven Selberg
2013-12-02 16:54:31 +01:00
committed by Shawn Pearce
parent ef30ea4a4b
commit ae4349db63
3 changed files with 123 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files

128
gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ShowQueue.java
View File

@@ -16,6 +16,7 @@ package com.google.gerrit.sshd.commands;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.git.TaskInfoFactory;
import com.google.gerrit.server.git.WorkQueue;
import com.google.gerrit.server.git.WorkQueue.ProjectTask;
import com.google.gerrit.server.git.WorkQueue.Task;
@@ -72,31 +73,10 @@ final class ShowQueue extends SshCommand {
@Override
protected void run() {
final List<Task<?>> pending = workQueue.getTasks();
Collections.sort(pending, new Comparator<Task<?>>() {
public int compare(Task<?> a, Task<?> b) {
final Task.State aState = a.getState();
final Task.State bState = b.getState();
if (aState != bState) {
return aState.ordinal() - bState.ordinal();
}
final long aDelay = a.getDelay(TimeUnit.MILLISECONDS);
final long bDelay = b.getDelay(TimeUnit.MILLISECONDS);
if (aDelay < bDelay) {
return -1;
} else if (aDelay > bDelay) {
return 1;
}
return format(a).compareTo(format(b));
}
});
taskNameWidth = wide ? Integer.MAX_VALUE : columns - 8 - 12 - 8 - 4;
final List<QueueTaskInfo> pending = getSortedTaskInfoList();
stdout.print(String.format("%-8s %-12s %-8s %s\n", //
stdout.print(String.format("%-8s %-12s %-8s %s\n",
"Task", "State", "", "Command"));
stdout.print("----------------------------------------------"
+ "--------------------------------\n");
@@ -105,9 +85,9 @@ final class ShowQueue extends SshCommand {
final long now = System.currentTimeMillis();
final boolean viewAll = currentUser.getCapabilities().canViewQueue();
for (final Task<?> task : pending) {
final long delay = task.getDelay(TimeUnit.MILLISECONDS);
final Task.State state = task.getState();
for (final QueueTaskInfo taskInfo : pending) {
final long delay = taskInfo.delayMillis;
final Task.State state = taskInfo.state;
final String start;
switch (state) {
@@ -131,11 +111,9 @@ final class ShowQueue extends SshCommand {
String remoteName = null;
if (!viewAll) {
if (task instanceof ProjectTask<?>) {
projectName = ((ProjectTask<?>)task).getProjectNameKey();
remoteName = ((ProjectTask<?>)task).getRemoteName();
hasCustomizedPrint = ((ProjectTask<?>)task).hasCustomizedPrint();
}
projectName = taskInfo.getProjectNameKey();
remoteName = taskInfo.getRemoteName();
hasCustomizedPrint = taskInfo.hasCustomizedPrint();
ProjectState e = null;
if (projectName != null) {
@@ -151,8 +129,10 @@ final class ShowQueue extends SshCommand {
// Shows information about tasks depending on the user rights
if (viewAll || (!hasCustomizedPrint && regularUserCanSee)) {
stdout.print(String.format("%8s %-12s %-8s %s\n", //
id(task.getTaskId()), start, "", format(task)));
stdout.print(String.format(
"%8s %-12s %-8s %s\n",
id(taskInfo.getTaskId()), start, "",
taskInfo.getTaskString(taskNameWidth)));
} else if (regularUserCanSee) {
if (remoteName == null) {
remoteName = projectName.get();
@@ -160,8 +140,8 @@ final class ShowQueue extends SshCommand {
remoteName = remoteName + "/" + projectName;
}
stdout.print(String.format("%8s %-12s %-8s %s\n", //
id(task.getTaskId()), start, "", remoteName));
stdout.print(String.format("%8s %-12s %-8s %s\n",
id(taskInfo.getTaskId()), start, "", remoteName));
}
}
stdout.print("----------------------------------------------"
@@ -174,6 +154,33 @@ final class ShowQueue extends SshCommand {
stdout.print(" " + numberOfPendingTasks + " tasks\n");
}
private List<QueueTaskInfo> getSortedTaskInfoList() {
final List<QueueTaskInfo> taskInfos =
workQueue.getTaskInfos(new TaskInfoFactory<QueueTaskInfo>() {
@Override
public QueueTaskInfo getTaskInfo(Task<?> task) {
return new QueueTaskInfo(task);
}
});
Collections.sort(taskInfos, new Comparator<QueueTaskInfo>() {
@Override
public int compare(QueueTaskInfo a, QueueTaskInfo b) {
if (a.state != b.state) {
return a.state.ordinal() - b.state.ordinal();
}
int cmp = Long.signum(a.delayMillis - b.delayMillis);
if (cmp != 0) {
return cmp;
}
return a.getTaskString(taskNameWidth)
.compareTo(b.getTaskString(taskNameWidth));
}
});
return taskInfos;
}
private static String id(final int id) {
return IdGenerator.format(id);
}
@@ -186,15 +193,6 @@ final class ShowQueue extends SshCommand {
return new SimpleDateFormat("MMM-dd HH:mm").format(when);
}
private String format(final Task<?> task) {
String s = task.toString();
if (s.length() < taskNameWidth) {
return s;
} else {
return s.substring(0, taskNameWidth);
}
}
private static String format(final Task.State state) {
switch (state) {
case DONE:
@@ -211,4 +209,46 @@ final class ShowQueue extends SshCommand {
return state.toString();
}
}
private static class QueueTaskInfo {
private final long delayMillis;
private final Task.State state;
private final Task<?> task;
QueueTaskInfo(Task<?> task) {
this.task = task;
this.delayMillis = task.getDelay(TimeUnit.MILLISECONDS);
this.state = task.getState();
}
String getRemoteName() {
if (task instanceof ProjectTask) {
return ((ProjectTask<?>) task).getRemoteName();
}
return null;
}
Project.NameKey getProjectNameKey() {
if (task instanceof ProjectTask<?>) {
return ((ProjectTask<?>) task).getProjectNameKey();
}
return null;
}
boolean hasCustomizedPrint() {
if (task instanceof ProjectTask<?>) {
return ((ProjectTask<?>) task).hasCustomizedPrint();
}
return false;
}
int getTaskId() {
return task.getTaskId();
}
String getTaskString(int maxLength) {
String s = task.toString();
return s.length() < maxLength ? s : s.substring(0, maxLength);
}
}
}