Add background job to abandon inactive changes automatically

Add a change cleanup job that runs periodically in the background.
This cleanup job can automatically abandon open changes that have been
inactive for a defined time.

Abandoning old inactive changes has a few advantages:
- it reduces the load for recomputing the mergeability flag when a
  change is merged
- it keeps dashboards clean
- it signals change authors that changes are considered outdated

Change-Id: Ia798667901fe8d734ca29bcf81c7b9b4a1eb4c50
Signed-off-by: Edwin Kempin <edwin.kempin@sap.com>
This commit is contained in:
Edwin Kempin 2015-06-03 10:27:39 +02:00
parent 64d33627b4
commit e326a1dada
12 changed files with 450 additions and 13 deletions

View File

@ -910,6 +910,76 @@ keyboard shortcut is appended.
Default is "Reply and score". In the user interface it becomes "Reply
and score (Shortcut: a)".
[[changeCleanup]]
=== Section changeCleanup
This section allows to configure change cleanups and schedules them to
run periodically.
[[changeCleanup.abandonAfter]]changeCleanup.abandonAfter::
+
Period of inactivity after which open changes should be abandoned
automatically.
+
By default `0`, never abandon open changes.
+
[WARNING] Auto-Abandoning changes may confuse/annoy users. When
enabling this, make sure to choose a reasonably large grace period and
inform users in advance.
+
The following suffixes are supported to define the time unit:
+
* `d, day, days`
* `w, week, weeks` (`1 week` is treated as `7 days`)
* `mon, month, months` (`1 month` is treated as `30 days`)
* `y, year, years` (`1 year` is treated as `365 days`)
[[changeCleanup.abandonMessage]]changeCleanup.abandonMessage::
+
Change message that should be posted when a change is abandoned.
+
'${URL}' can be used as a placeholder for the Gerrit web URL.
+
By default "Auto-Abandoned due to inactivity, see
${URL}Documentation/user-change-cleanup.html#auto-abandon\n\n
If this change is still wanted it should be restored.".
[[changeCleanup.startTime]]changeCleanup.startTime::
+
Start time to define the first execution of the change cleanups.
If the configured `'changeCleanup.interval'` is shorter than
`'changeCleanup.startTime - now'` the start time will be preponed by
the maximum integral multiple of `'changeCleanup.interval'` so that the
start time is still in the future.
+
----
<day of week> <hours>:<minutes>
or
<hours>:<minutes>
<day of week> : Mon, Tue, Wed, Thu, Fri, Sat, Sun
<hours> : 00-23
<minutes> : 0-59
----
[[changeCleanup.interval]]changeCleanup.interval::
+
Interval for periodic repetition of triggering the change cleanups.
The interval must be larger than zero. The following suffixes are supported
to define the time unit for the interval:
+
* `s, sec, second, seconds`
* `m, min, minute, minutes`
* `h, hr, hour, hours`
* `d, day, days`
* `w, week, weeks` (`1 week` is treated as `7 days`)
* `mon, month, months` (`1 month` is treated as `30 days`)
* `y, year, years` (`1 year` is treated as `365 days`)
link:#schedule-examples[Schedule examples] can be found in the
link:#gc[gc] section.
[[changeMerge]]
=== Section changeMerge
@ -1519,6 +1589,7 @@ to define the time unit for the interval:
* `mon, month, months` (`1 month` is treated as `30 days`)
* `y, year, years` (`1 year` is treated as `365 days`)
[[schedule-examples]]
Examples::
+
----

View File

@ -21,6 +21,7 @@
.. Changes
... link:user-changeid.html[Change-Id Lines]
... link:user-signedoffby.html[Signed-off-by Lines]
... link:user-change-cleanup.html[Change Cleanup]
== Project Management
. link:project-configuration.html[Project Configuration]

View File

@ -0,0 +1,28 @@
= Gerrit Code Review - Change Cleanup
Gerrit administrators may configure
link:config-gerrit.html#changeCleanup[change cleanups] that are
executed periodically.
[[auto-abandon]]
== Auto-Abandon
This cleanup job automatically abandons open changes that have been
inactive for a defined time.
Abandoning old inactive changes has the following advantages:
* it signals change authors that changes are considered outdated
* it keeps dashboards clean
* it reduces the load on the server (for open changes the mergeability
flag is recomputed whenever a change is merged)
If a change is still wanted it can be restored by clicking on the
`Restore` button.
GERRIT
------
Part of link:index.html[Gerrit Code Review]
SEARCHBOX
---------

View File

@ -44,6 +44,7 @@ import com.google.gerrit.pgm.util.SiteProgram;
import com.google.gerrit.reviewdb.client.AuthType;
import com.google.gerrit.server.account.InternalAccountDirectory;
import com.google.gerrit.server.cache.h2.DefaultCacheFactory;
import com.google.gerrit.server.change.ChangeCleanupRunner;
import com.google.gerrit.server.config.AuthConfig;
import com.google.gerrit.server.config.AuthConfigModule;
import com.google.gerrit.server.config.CanonicalWebUrlModule;
@ -373,6 +374,7 @@ public class Daemon extends SiteProgram {
}
});
modules.add(new GarbageCollectionModule());
modules.add(new ChangeCleanupRunner.Module());
return cfgInjector.createChildInjector(modules);
}

View File

@ -22,6 +22,7 @@ import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.webui.UiAction;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.server.ReviewDb;
@ -88,14 +89,23 @@ public class Abandon implements RestModifyView<ChangeResource, AbandonInput>,
} else if (change.getStatus() == Change.Status.DRAFT) {
throw new ResourceConflictException("draft changes cannot be abandoned");
}
change = abandon(control, input.message, caller.getAccount());
ChangeInfo result = json.format(change);
return result;
}
public Change abandon(ChangeControl control,
String msgTxt, Account acc) throws ResourceConflictException,
OrmException, IOException {
Change change;
ChangeMessage message;
ChangeUpdate update;
Change.Id changeId = control.getChange().getId();
ReviewDb db = dbProvider.get();
db.changes().beginTransaction(change.getId());
db.changes().beginTransaction(changeId);
try {
change = db.changes().atomicUpdate(
change.getId(),
changeId,
new AtomicUpdate<Change>() {
@Override
public Change update(Change change) {
@ -109,12 +119,12 @@ public class Abandon implements RestModifyView<ChangeResource, AbandonInput>,
});
if (change == null) {
throw new ResourceConflictException("change is "
+ status(db.changes().get(req.getChange().getId())));
+ status(db.changes().get(changeId)));
}
//TODO(yyonas): atomic update was not propagated
update = updateFactory.create(control, change.getLastUpdatedOn());
message = newMessage(input, caller, change);
message = newMessage(msgTxt, acc != null ? acc.getId() : null, change);
cmUtil.addChangeMessage(db, update, message);
db.commit();
} finally {
@ -125,19 +135,20 @@ public class Abandon implements RestModifyView<ChangeResource, AbandonInput>,
indexer.index(db, change);
try {
ReplyToChangeSender cm = abandonedSenderFactory.create(change.getId());
cm.setFrom(caller.getAccountId());
if (acc != null) {
cm.setFrom(acc.getId());
}
cm.setChangeMessage(message);
cm.send();
} catch (Exception e) {
log.error("Cannot email update for change " + change.getChangeId(), e);
}
hooks.doChangeAbandonedHook(change,
caller.getAccount(),
acc,
db.patchSets().get(change.currentPatchSetId()),
Strings.emptyToNull(input.message),
Strings.emptyToNull(msgTxt),
db);
ChangeInfo result = json.format(change);
return result;
return change;
}
@Override
@ -150,20 +161,20 @@ public class Abandon implements RestModifyView<ChangeResource, AbandonInput>,
&& resource.getControl().canAbandon());
}
private ChangeMessage newMessage(AbandonInput input, IdentifiedUser caller,
private ChangeMessage newMessage(String msgTxt, Account.Id accId,
Change change) throws OrmException {
StringBuilder msg = new StringBuilder();
msg.append("Abandoned");
if (!Strings.nullToEmpty(input.message).trim().isEmpty()) {
if (!Strings.nullToEmpty(msgTxt).trim().isEmpty()) {
msg.append("\n\n");
msg.append(input.message.trim());
msg.append(msgTxt.trim());
}
ChangeMessage message = new ChangeMessage(
new ChangeMessage.Key(
change.getId(),
ChangeUtil.messageUUID(dbProvider.get())),
caller.getAccountId(),
accId,
change.getLastUpdatedOn(),
change.currentPatchSetId());
message.setMessage(msg.toString());

View File

@ -0,0 +1,97 @@
// Copyright (C) 2015 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.server.change;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.config.ChangeCleanupConfig;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.query.QueryParseException;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.ChangeQueryBuilder;
import com.google.gerrit.server.query.change.QueryProcessor;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.util.List;
import java.util.concurrent.TimeUnit;
@Singleton
public class AbandonUtil {
private static final Logger log = LoggerFactory.getLogger(AbandonUtil.class);
private final ChangeCleanupConfig cfg;
private final IdentifiedUser.GenericFactory identifiedUserFactory;
private final QueryProcessor queryProcessor;
private final ChangeQueryBuilder queryBuilder;
private final ChangeControl.GenericFactory changeControlFactory;
private final Abandon abandon;
@Inject
AbandonUtil(
ChangeCleanupConfig cfg,
IdentifiedUser.GenericFactory identifiedUserFactory,
QueryProcessor queryProcessor,
ChangeQueryBuilder queryBuilder,
ChangeControl.GenericFactory changeControlFactory,
Abandon abandon) {
this.cfg = cfg;
this.identifiedUserFactory = identifiedUserFactory;
this.queryProcessor = queryProcessor;
this.queryBuilder = queryBuilder;
this.changeControlFactory = changeControlFactory;
this.abandon = abandon;
}
public void abandonInactiveOpenChanges() {
if (cfg.getAbandonAfter() <= 0) {
return;
}
try {
String query = "status:new age:"
+ TimeUnit.MILLISECONDS.toMinutes(cfg.getAbandonAfter())
+ "m";
List<ChangeData> changesToAbandon = queryProcessor.enforceVisibility(false)
.queryChanges(queryBuilder.parse(query)).changes();
for (ChangeData cd : changesToAbandon) {
try {
abandon.abandon(changeControl(cd), cfg.getAbandonMessage(), null);
} catch (ResourceConflictException e) {
// Change was already merged or abandoned.
} catch (Throwable e) {
log.error(String.format(
"Failed to auto-abandon inactive open change %d.",
cd.getId().get()), e);
}
}
} catch (QueryParseException | OrmException e) {
log.error("Failed to query inactive open changes for auto-abandoning.", e);
}
}
private ChangeControl changeControl(ChangeData cd)
throws NoSuchChangeException, OrmException {
Change c = cd.change();
return changeControlFactory.controlFor(c,
identifiedUserFactory.create(c.getOwner()));
}
}

View File

@ -0,0 +1,108 @@
// Copyright (C) 2015 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.server.change;
import static com.google.gerrit.server.config.ScheduleConfig.MISSING_CONFIG;
import com.google.gerrit.extensions.events.LifecycleListener;
import com.google.gerrit.lifecycle.LifecycleModule;
import com.google.gerrit.server.config.ChangeCleanupConfig;
import com.google.gerrit.server.config.ScheduleConfig;
import com.google.gerrit.server.git.WorkQueue;
import com.google.gerrit.server.util.ManualRequestContext;
import com.google.gerrit.server.util.OneOffRequestContext;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.util.concurrent.TimeUnit;
/** Runnable to enable scheduling change cleanups to run periodically */
public class ChangeCleanupRunner implements Runnable {
private static final Logger log = LoggerFactory
.getLogger(ChangeCleanupRunner.class);
public static class Module extends LifecycleModule {
@Override
protected void configure() {
listener().to(Lifecycle.class);
}
}
static class Lifecycle implements LifecycleListener {
private final WorkQueue queue;
private final ChangeCleanupRunner runner;
private final ChangeCleanupConfig cfg;
@Inject
Lifecycle(WorkQueue queue,
ChangeCleanupRunner runner,
ChangeCleanupConfig cfg) {
this.queue = queue;
this.runner = runner;
this.cfg = cfg;
}
@Override
public void start() {
ScheduleConfig scheduleConfig = cfg.getScheduleConfig();
long interval = scheduleConfig.getInterval();
long delay = scheduleConfig.getInitialDelay();
if (delay == MISSING_CONFIG && interval == MISSING_CONFIG) {
log.info("Ignoring missing changeCleanup schedule configuration");
} else if (delay < 0 || interval <= 0) {
log.warn(String.format(
"Ignoring invalid changeCleanup schedule configuration: %s",
scheduleConfig));
} else {
queue.getDefaultQueue().scheduleAtFixedRate(runner, delay,
interval, TimeUnit.MILLISECONDS);
}
}
@Override
public void stop() {
// handled by WorkQueue.stop() already
}
}
private final OneOffRequestContext oneOffRequestContext;
private final AbandonUtil abandonUtil;
@Inject
ChangeCleanupRunner(
OneOffRequestContext oneOffRequestContext,
AbandonUtil abandonUtil) {
this.oneOffRequestContext = oneOffRequestContext;
this.abandonUtil = abandonUtil;
}
@Override
public void run() {
log.info("Running change cleanups.");
try (ManualRequestContext ctx = oneOffRequestContext.open()) {
abandonUtil.abandonInactiveOpenChanges();
} catch (OrmException e) {
log.error("Failed to cleanup changes.", e);
}
}
@Override
public String toString() {
return "change cleanup runner";
}
}

View File

@ -0,0 +1,75 @@
// Copyright (C) 2015 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.server.config;
import com.google.common.base.Strings;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import org.eclipse.jgit.lib.Config;
import java.util.concurrent.TimeUnit;
@Singleton
public class ChangeCleanupConfig {
private static String SECTION = "changeCleanup";
private static String KEY_ABANDON_AFTER = "abandonAfter";
private static String KEY_ABANDON_MESSAGE = "abandonMessage";
private static String DEFAULT_ABANDON_MESSAGE =
"Auto-Abandoned due to inactivity, see "
+ "${URL}Documentation/user-change-cleanup.html#auto-abandon\n"
+ "\n"
+ "If this change is still wanted it should be restored.";
private final ScheduleConfig scheduleConfig;
private final long abandonAfter;
private final String abandonMessage;
@Inject
ChangeCleanupConfig(@GerritServerConfig Config cfg,
@CanonicalWebUrl String canonicalWebUrl) {
scheduleConfig = new ScheduleConfig(cfg, SECTION);
abandonAfter = readAbandonAfter(cfg);
abandonMessage = readAbandonMessage(cfg, canonicalWebUrl);
}
private long readAbandonAfter(Config cfg) {
long abandonAfter =
ConfigUtil.getTimeUnit(cfg, SECTION, null, KEY_ABANDON_AFTER, 0,
TimeUnit.MILLISECONDS);
return abandonAfter >= 0 ? abandonAfter : 0;
}
private String readAbandonMessage(Config cfg, String webUrl) {
String abandonMessage = cfg.getString(SECTION, null, KEY_ABANDON_MESSAGE);
if (Strings.isNullOrEmpty(abandonMessage)) {
abandonMessage = DEFAULT_ABANDON_MESSAGE;
}
abandonMessage = abandonMessage.replaceAll("\\$\\{URL\\}", webUrl);
return abandonMessage;
}
public ScheduleConfig getScheduleConfig() {
return scheduleConfig;
}
public long getAbandonAfter() {
return abandonAfter;
}
public String getAbandonMessage() {
return abandonMessage;
}
}

View File

@ -219,6 +219,7 @@ public class GerritGlobalModule extends FactoryModule {
bind(GitWebConfig.class);
bind(GcConfig.class);
bind(ChangeCleanupConfig.class);
bind(ApprovalsUtil.class);
bind(ChangeMergeQueue.class).in(SINGLETON);

View File

@ -38,6 +38,11 @@ public class ScheduleConfig {
private static final String KEY_INTERVAL = "interval";
private static final String KEY_STARTTIME = "startTime";
private final Config rc;
private final String section;
private final String subsection;
private final String keyInterval;
private final String keyStartTime;
private final long initialDelay;
private final long interval;
@ -62,6 +67,11 @@ public class ScheduleConfig {
@VisibleForTesting
ScheduleConfig(Config rc, String section, String subsection,
String keyInterval, String keyStartTime, DateTime now) {
this.rc = rc;
this.section = section;
this.subsection = subsection;
this.keyInterval = keyInterval;
this.keyStartTime = keyStartTime;
this.interval = interval(rc, section, subsection, keyInterval);
if (interval > 0) {
this.initialDelay = initialDelay(rc, section, subsection, keyStartTime, now,
@ -150,4 +160,31 @@ public class ScheduleConfig {
return delay;
}
@Override
public String toString() {
StringBuilder b = new StringBuilder();
b.append(formatValue(keyInterval));
b.append(", ");
b.append(formatValue(keyStartTime));
return b.toString();
}
private String formatValue(String key) {
StringBuilder b = new StringBuilder();
b.append(section);
if (subsection != null) {
b.append(".");
b.append(subsection);
}
b.append(".");
b.append(key);
String value = rc.getString(section, subsection, key);
if (value != null) {
b.append(" = ");
b.append(value);
} else {
b.append(": NA");
}
return b.toString();
}
}

View File

@ -489,6 +489,10 @@ public class EventFactory {
* @return object suitable for serialization to JSON
*/
public AccountAttribute asAccountAttribute(final Account account) {
if (account == null) {
return null;
}
AccountAttribute who = new AccountAttribute();
who.name = account.getFullName();
who.email = account.getPreferredEmail();

View File

@ -28,6 +28,7 @@ import com.google.gerrit.lucene.LuceneIndexModule;
import com.google.gerrit.reviewdb.client.AuthType;
import com.google.gerrit.server.account.InternalAccountDirectory;
import com.google.gerrit.server.cache.h2.DefaultCacheFactory;
import com.google.gerrit.server.change.ChangeCleanupRunner;
import com.google.gerrit.server.config.AuthConfig;
import com.google.gerrit.server.config.AuthConfigModule;
import com.google.gerrit.server.config.CanonicalWebUrlModule;
@ -323,6 +324,7 @@ public class WebAppInitializer extends GuiceServletContextListener
}
});
modules.add(new GarbageCollectionModule());
modules.add(new ChangeCleanupRunner.Module());
return cfgInjector.createChildInjector(modules);
}