Add support to skip evaluation of expensive tasks
This change introduces a new task config key i.e. `evaluation-threshold`
to allow skipping evaluation of expensive tasks when the number of
changes in the gerrit query output (or createPluginDefinedInfos() input)
is more than the value defined for this key.
Since task plugin UI only supports passing a single change to
`createPluginDefinedInfos()`, it'll never produce `SKIPPED` tasks and
thus doesn't need to be updated.
Change-Id: I333058472ca75b27a6bc64549b5a35a45ca0389f
diff --git a/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfig.java b/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfig.java
index 1859b0d..74c164a 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfig.java
@@ -57,6 +57,7 @@
public class TaskBase extends SubSection {
public String applicable;
public String duplicateKey;
+ public int evaluationThreshold;
public Map<String, String> exported;
public String fail;
public String failHint;
@@ -79,6 +80,7 @@
this.isMasqueraded = isMasqueraded;
applicable = getString(s, KEY_APPLICABLE, null);
duplicateKey = getString(s, KEY_DUPLICATE_KEY, null);
+ evaluationThreshold = getInt(s, KEY_EVALUATION_THRESHOLD, DEFAULT_EVALUATION_THRESHOLD);
exported = getProperties(s, KEY_EXPORT_PREFIX);
fail = getString(s, KEY_FAIL, null);
failHint = getString(s, KEY_FAIL_HINT, null);
@@ -205,6 +207,7 @@
public static final String KEY_ARG = "arg";
public static final String KEY_CHANGES = "changes";
public static final String KEY_DUPLICATE_KEY = "duplicate-key";
+ public static final String KEY_EVALUATION_THRESHOLD = "evaluation-threshold";
public static final String KEY_EXPORT_PREFIX = "export-";
public static final String KEY_FAIL = "fail";
public static final String KEY_FAIL_HINT = "fail-hint";
@@ -224,6 +227,7 @@
public static final String KEY_SUBTASKS_FILE = "subtasks-file";
public static final String KEY_TYPE = "type";
public static final String KEY_USER = "user";
+ public static final int DEFAULT_EVALUATION_THRESHOLD = Integer.MAX_VALUE;
protected final FileKey file;
public boolean isVisible;
@@ -323,6 +327,10 @@
return cfg.getString(s.section(), s.subSection(), key);
}
+ protected int getInt(SubSectionKey s, String key, int def) {
+ return cfg.getInt(s.section(), s.subSection(), key, def);
+ }
+
protected List<String> getStringList(SubSectionKey s, String key) {
List<String> stringList = Arrays.asList(cfg.getStringList(s.section(), s.subSection(), key));
stringList.replaceAll(str -> str == null ? "" : str);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/task/TaskPluginDefinedInfoFactory.java b/src/main/java/com/googlesource/gerrit/plugins/task/TaskPluginDefinedInfoFactory.java
index 7cb083f..0791bda 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskPluginDefinedInfoFactory.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskPluginDefinedInfoFactory.java
@@ -48,6 +48,7 @@
INVALID,
UNKNOWN,
DUPLICATE,
+ SKIPPED,
WAITING,
READY,
PASS,
@@ -59,6 +60,7 @@
public long numberOfChangeNodes;
public long numberOfDuplicates;
public long numberOfNodes;
+ public long numberOfSkippedTasks;
public long numberOfTaskPluginAttributes;
public Object predicateCache;
public Object matchCache;
@@ -109,6 +111,7 @@
protected Modules.MyOptions options;
protected TaskPluginAttribute lastTaskPluginAttribute;
protected Statistics statistics;
+ protected int numberOfChanges;
@Inject
public TaskPluginDefinedInfoFactory(
@@ -132,6 +135,7 @@
@Override
public Map<Change.Id, PluginDefinedInfo> createPluginDefinedInfos(
Collection<ChangeData> cds, BeanProvider beanProvider, String plugin) {
+ numberOfChanges = cds.size();
Map<Change.Id, PluginDefinedInfo> pluginInfosByChange = new HashMap<>();
options = (Modules.MyOptions) beanProvider.getDynamicBean(plugin);
if (options.all || options.onlyApplicable || options.onlyInvalid) {
@@ -176,16 +180,15 @@
protected Task task;
protected TaskAttribute attribute;
protected boolean isDuplicate;
+ protected boolean isExpensive;
protected AttributeFactory(Node node) {
this.node = node;
this.task = node.task;
attribute = new TaskAttribute(task.name());
- isDuplicate =
- node.isDuplicate
- || (node.isChange()
- && node.getNodeSetByBaseTasksFactory().get(task.subSection).contains(node.key()));
+ isExpensive = numberOfChanges > node.task.evaluationThreshold;
+ isDuplicate = !isExpensive && isDuplicate();
if (options.includeStatistics) {
statistics.numberOfNodes++;
@@ -195,6 +198,9 @@
if (isDuplicate) {
statistics.numberOfDuplicates++;
}
+ if (isExpensive) {
+ statistics.numberOfSkippedTasks++;
+ }
attribute.statistics = new TaskAttribute.Statistics();
attribute.statistics.properties = node.propertiesStatistics;
}
@@ -222,8 +228,8 @@
if (node.isChange()) {
attribute.change = node.getChangeData().getId().get();
}
- attribute.hasPass = !(isDuplicate || isAllNull(task.pass, task.fail));
- if (!isDuplicate) {
+ attribute.hasPass = !(isDuplicate || isExpensive || isAllNull(task.pass, task.fail));
+ if (!(isDuplicate || isExpensive)) {
attribute.subTasks = getSubTasks();
}
attribute.status = getStatus();
@@ -246,7 +252,7 @@
if (!options.onlyApplicable) {
attribute.applicable = applicable;
}
- if (!isDuplicate) {
+ if (!(isDuplicate || isExpensive)) {
if (task.inProgress != null) {
attribute.inProgress = node.matchOrNull(task.inProgress);
}
@@ -297,6 +303,9 @@
if (isDuplicate) {
return Status.DUPLICATE;
}
+ if (isExpensive) {
+ return Status.SKIPPED;
+ }
if (isAllNull(task.pass, task.fail, attribute.subTasks)) {
// A leaf def has no defined subdefs.
boolean hasDefinedSubtasks =
@@ -333,7 +342,8 @@
}
if (attribute.subTasks != null
- && !isAll(attribute.subTasks, EnumSet.of(Status.PASS, Status.DUPLICATE))) {
+ && !isAll(
+ attribute.subTasks, EnumSet.of(Status.PASS, Status.DUPLICATE, Status.SKIPPED))) {
// It is possible for a subtask's PASS criteria to change while
// a parent task is executing, or even after the parent task
// completes. This can result in the parent PASS criteria being
@@ -393,6 +403,12 @@
return false;
}
}
+
+ protected boolean isDuplicate() {
+ return node.isDuplicate
+ || (node.isChange()
+ && node.getNodeSetByBaseTasksFactory().get(task.subSection).contains(node.key()));
+ }
}
protected long millis() {
@@ -445,6 +461,8 @@
return def.failHint;
case DUPLICATE:
return "Duplicate task is non blocking and empty to break the loop";
+ case SKIPPED:
+ return "Expensive task evaluation-threshold breached, skipped evaluation";
default:
}
}
diff --git a/src/main/resources/Documentation/task.md b/src/main/resources/Documentation/task.md
index b37f52a..4644649 100644
--- a/src/main/resources/Documentation/task.md
+++ b/src/main/resources/Documentation/task.md
@@ -77,6 +77,9 @@
A task with a `INVALID` status has an invalid/missing definition or an
invalid query.
+A task with a `SKIPPED` status has been skipped while evaluating the task tree
+based on the [evaluation-threshold](#evaluation-threshold) defined for it.
+
Tasks
-----
Tasks can either be root tasks, or subtasks. Tasks are expected to be
@@ -289,6 +292,37 @@
changes = -status:merged parentof:${_change_number} project:${_change_project} branch:${_change_branch}
```
+<a id="evaluation-threshold"></a>
+
+`evaluation-threshold`
+
+: This key defines a threshold based on which a task can be determined to
+be expensive for evaluation and consequently, should be skipped. If the number
+of changes in a single `createPluginDefinedInfos()` invocation (such as from a
+`gerrit query` command) are more than the `evaluation-threshold` of the task,
+it will be skipped i.e. the keys below are not evaluated for the task.
+
+- pass
+- fail
+- in-progress
+- subtask
+- subtasks-factory
+- subtasks-external
+- subtasks-file
+- preload-task
+- duplicate-key
+
+Example:
+
+```
+[root "Root with expensive task"]
+ subtask = expensive task
+
+[task "expensive task"]
+ evaluation-threshold = 1
+ pass = True
+```
+
Root Tasks
----------
Root tasks typically define the "final verification" tasks for changes. Each
diff --git a/src/main/resources/Documentation/test/task_states.md b/src/main/resources/Documentation/test/task_states.md
index aea074e..49c0703 100644
--- a/src/main/resources/Documentation/test/task_states.md
+++ b/src/main/resources/Documentation/test/task_states.md
@@ -2137,6 +2137,115 @@
]
}
+[root "Root (subtask SKIPPED)"]
+ subtask = Subtask SKIPPED
+
+[task "Subtask SKIPPED"]
+ evaluation-threshold = 1
+ pass = True
+
+{
+ "applicable" : true,
+ "hasPass" : false,
+ "name" : "Root (subtask SKIPPED)",
+ "status" : "PASS",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "hasPass" : false,
+ "hint" : "Expensive task evaluation-threshold breached, skipped evaluation",
+ "name" : "Subtask SKIPPED",
+ "status" : "SKIPPED"
+ }
+ ]
+}
+
+[root "Root (subtask NOT SKIPPED)"]
+ subtask = Subtask NOT SKIPPED
+
+[task "Subtask NOT SKIPPED"]
+ evaluation-threshold = 2
+ pass = True
+
+{
+ "applicable" : true,
+ "hasPass" : false,
+ "name" : "Root (subtask NOT SKIPPED)",
+ "status" : "PASS",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "hasPass" : true,
+ "name" : "Subtask NOT SKIPPED",
+ "status" : "PASS"
+ }
+ ]
+}
+
+[root "Root tasks-factory CHANGE SKIPPED"]
+ subtasks-factory = tasks-factory change SKIPPED
+
+[tasks-factory "tasks-factory change SKIPPED"]
+ names-factory = names-factory change list
+ evaluation-threshold = 1
+ preload-task = Subtask APPLICABLE
+
+{
+ "applicable" : true,
+ "hasPass" : false,
+ "name" : "Root tasks-factory CHANGE SKIPPED",
+ "status" : "PASS",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "change" : _change1_number,
+ "hasPass" : false,
+ "hint": "Expensive task evaluation-threshold breached, skipped evaluation",
+ "name" : "_change1_number",
+ "status" : "SKIPPED"
+ },
+ {
+ "applicable" : true,
+ "change" : _change2_number,
+ "hasPass" : false,
+ "hint": "Expensive task evaluation-threshold breached, skipped evaluation",
+ "name" : "_change2_number",
+ "status" : "SKIPPED"
+ }
+ ]
+}
+
+[root "Root tasks-factory CHANGE NOT SKIPPED"]
+ subtasks-factory = tasks-factory change NOT SKIPPED
+
+[tasks-factory "tasks-factory change NOT SKIPPED"]
+ names-factory = names-factory change list
+ evaluation-threshold = 2
+ preload-task = Subtask APPLICABLE
+
+{
+ "applicable" : true,
+ "hasPass" : false,
+ "name" : "Root tasks-factory CHANGE NOT SKIPPED",
+ "status" : "PASS",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "change" : _change1_number,
+ "hasPass" : true,
+ "name" : "_change1_number",
+ "status" : "PASS"
+ },
+ {
+ "applicable" : true,
+ "change" : _change2_number,
+ "hasPass" : true,
+ "name" : "_change2_number",
+ "status" : "PASS"
+ }
+ ]
+}
+
[root "Root Looping"]
subtask = Looping
diff --git a/test/strip_non_applicable.py b/test/strip_non_applicable.py
index a0ce4fa..6cec219 100755
--- a/test/strip_non_applicable.py
+++ b/test/strip_non_applicable.py
@@ -43,7 +43,7 @@
status=''
if STATUS in list(task.keys()):
status = task[STATUS]
- if status != 'INVALID' and status != 'DUPLICATE':
+ if status != 'INVALID' and status != 'DUPLICATE' and status != 'SKIPPED':
del tasks[i]
nexti = i
