Terminate sortkey with prejudice

It has not been used by the search system since 2.8 and all code using
it in a meaningful way in Gerrit has been removed. Now we can remove
it from the database and API classes.

This requires a schema upgrade to delete the column and rebuild some
indexes to use last_updated_on instead of sort_key.

Change-Id: I87660f8e0e44bf11bafd9fa24f990351759df3fb
This commit is contained in:
Dave Borowitz 2014-12-19 11:27:14 -08:00
parent 52403ce985
commit 4241459b3f
25 changed files with 62 additions and 121 deletions

View File

@ -116,7 +116,7 @@ Find the 2 most recent open changes in the tools/gerrit project:
====
$ ssh -p 29418 review.example.com gerrit query --format=JSON status:open project:tools/gerrit limit:2
{"project":"tools/gerrit", ...}
{"project":"tools/gerrit", ..., sortKey:"000e6aee00003e26", ...}
{"project":"tools/gerrit", ...}
{"type":"stats","rowCount":2,"runningTimeMilliseconds:15}
====

View File

@ -35,8 +35,6 @@ was created.
lastUpdated:: Time in seconds since the UNIX epoch when this change
was last updated.
sortKey:: Internal key used to sort changes, based on lastUpdated.
open:: Boolean indicating if the change is still open for review.
status:: Current state of this change.

View File

@ -1123,7 +1123,6 @@ link:rest-api-changes.html#change-info[ChangeInfo] entities.
"created": "2013-02-01 09:59:32.126000000",
"updated": "2013-02-21 11:16:36.775000000",
"mergeable": true,
"_sortkey": "0023412400000f7d",
"_number": 3965,
"owner": {
"name": "John Doe"

View File

@ -55,7 +55,6 @@ the resulting change.
"mergeable": true,
"insertions": 0,
"deletions": 0,
"_sortkey": "002cbc25000004e5",
"_number": 4711,
"owner": {
"name": "John Doe"
@ -105,7 +104,6 @@ Query for open changes of watched projects:
"mergeable": true,
"insertions": 26,
"deletions": 10,
"_sortkey": "001e7057000006dc",
"_number": 1756,
"owner": {
"name": "John Doe"
@ -123,7 +121,6 @@ Query for open changes of watched projects:
"mergeable": true,
"insertions": 12,
"deletions": 18,
"_sortkey": "001e7056000006dd",
"_number": 1757,
"owner": {
"name": "John Doe"
@ -177,7 +174,6 @@ Query that retrieves changes for a user's dashboard:
"mergeable": true,
"insertions": 4,
"deletions": 7,
"_sortkey": "001e7057000006dc",
"_number": 1756,
"owner": {
"name": "John Doe"
@ -330,7 +326,6 @@ default. Optional fields are:
"mergeable": true,
"insertions": 16,
"deletions": 7,
"_sortkey": "001c9bf400000061",
"_number": 97,
"owner": {
"name": "Shawn Pearce"
@ -467,7 +462,6 @@ describes the change.
"mergeable": true,
"insertions": 34,
"deletions": 101,
"_sortkey": "0023412400000f7d",
"_number": 3965,
"owner": {
"name": "John Doe"
@ -520,7 +514,6 @@ REJECTED > APPROVED > DISLIKED > RECOMMENDED.
"mergeable": true,
"insertions": 126,
"deletions": 11,
"_sortkey": "0023412400000f7d",
"_number": 3965,
"owner": {
"_account_id": 1000096,
@ -764,7 +757,6 @@ describes the abandoned change.
"mergeable": true,
"insertions": 3,
"deletions": 310,
"_sortkey": "0023412400000f7d",
"_number": 3965,
"owner": {
"name": "John Doe"
@ -823,7 +815,6 @@ describes the restored change.
"mergeable": true,
"insertions": 2,
"deletions": 13,
"_sortkey": "0023412400000f7d",
"_number": 3965,
"owner": {
"name": "John Doe"
@ -880,7 +871,6 @@ is included.
"mergeable": false,
"insertions": 33,
"deletions": 9,
"_sortkey": "0024cf9a000012bf",
"_number": 4799,
"owner": {
"name": "John Doe"
@ -973,7 +963,6 @@ describes the reverting change.
"mergeable": true,
"insertions": 6,
"deletions": 4,
"_sortkey": "0023412400000f7d",
"_number": 3965,
"owner": {
"name": "John Doe"
@ -1035,7 +1024,6 @@ describes the submitted/merged change.
"status": "MERGED",
"created": "2013-02-01 09:59:32.126000000",
"updated": "2013-02-21 11:16:36.775000000",
"_sortkey": "0023412400000f7d",
"_number": 3965,
"owner": {
"name": "John Doe"
@ -1179,7 +1167,6 @@ missing from the result. At least `id`, `project`, `branch`, and
"mergeable": true,
"insertions": 34,
"deletions": 101,
"_sortkey": "0023412400000f7d",
"_number": 3965,
"owner": {
"name": "John Doe"
@ -1228,7 +1215,6 @@ Only the change owner, a project owner, or an administrator may fix changes.
"mergeable": true,
"insertions": 34,
"deletions": 101,
"_sortkey": "0023412400000f7d",
"_number": 3965,
"owner": {
"name": "John Doe"
@ -1863,7 +1849,6 @@ for the current patch set.
"mergeable": true,
"insertions": 34,
"deletions": 45,
"_sortkey": "0023412400000f7d",
"_number": 3965,
"owner": {
"_account_id": 1000096,
@ -2128,7 +2113,6 @@ is included.
"mergeable": false,
"insertions": 21,
"deletions": 21,
"_sortkey": "0024cf9a000012bf",
"_number": 4799,
"owner": {
"name": "John Doe"
@ -3096,7 +3080,6 @@ describes the resulting cherry picked change.
"mergeable": true,
"insertions": 12,
"deletions": 11,
"_sortkey": "0023412400000f7d",
"_number": 3965,
"owner": {
"name": "John Doe"
@ -3150,7 +3133,6 @@ describes the change.
"mergeable": true,
"insertions": 261,
"deletions": 101,
"_sortkey": "0023412400000f7d",
"_number": 3965,
"owner": {
"name": "John Doe"
@ -3335,7 +3317,6 @@ Not set for merged changes, or if the change has not yet been tested.
Number of inserted lines.
|`deletions` ||
Number of deleted lines.
|`_sortkey` ||The sortkey of the change.
|`_number` ||The legacy numeric ID of the change.
|`owner` ||
The owner of the change as an link:rest-api-accounts.html#account-info[
@ -3373,7 +3354,7 @@ Only set if link:#current-revision[the current revision] is requested
if link:#all-revisions[all revisions] are requested.
|`_more_changes` |optional, not set if `false`|
Whether the query would deliver more results if not limited. +
Only set on either the last or the first change that is returned.
Only set on the last change that is returned.
|`problems` |optional|
A list of link:#problem-info[ProblemInfo] entities describing potential
problems with this change. Only set if link:#check[CHECK] is set.

View File

@ -31,7 +31,6 @@ public class ChangeInfo {
protected String topic;
protected boolean starred;
protected Timestamp lastUpdatedOn;
protected String sortKey;
protected PatchSet.Id patchSetId;
protected boolean latest;
@ -52,7 +51,6 @@ public class ChangeInfo {
branch = c.getDest().getShortName();
topic = c.getTopic();
lastUpdatedOn = c.getLastUpdatedOn();
sortKey = c.getSortKey();
patchSetId = patchId;
latest = patchSetId == null || patchSetId.equals(c.currentPatchSetId());
}
@ -112,8 +110,4 @@ public class ChangeInfo {
public java.sql.Timestamp getLastUpdatedOn() {
return lastUpdatedOn;
}
public String getSortKey() {
return sortKey;
}
}

View File

@ -36,7 +36,6 @@ public class ChangeInfo {
public Integer insertions;
public Integer deletions;
public String _sortkey;
public String baseChange;
public int _number;

View File

@ -96,7 +96,6 @@ public class ChangeInfo extends JavaScriptObject {
private final native String updatedRaw() /*-{ return this.updated; }-*/;
public final native boolean starred() /*-{ return this.starred ? true : false; }-*/;
public final native boolean reviewed() /*-{ return this.reviewed ? true : false; }-*/;
public final native String _sortkey() /*-{ return this._sortkey; }-*/;
public final native NativeMap<LabelInfo> all_labels() /*-{ return this.labels; }-*/;
public final native LabelInfo label(String n) /*-{ return this.labels[n]; }-*/;
public final native String current_revision() /*-{ return this.current_revision; }-*/;

View File

@ -433,9 +433,7 @@ public final class Change {
@Column(id = 5)
protected Timestamp lastUpdatedOn;
/** A {@link #lastUpdatedOn} ASC,{@link #changeId} ASC for sorting. */
@Column(id = 6, length = 16)
protected String sortKey;
// DELETED: id = 6 (sortkey)
@Column(id = 7, name = "owner_account_id")
protected Account.Id owner;
@ -489,7 +487,6 @@ public final class Change {
rowVersion = other.rowVersion;
createdOn = other.createdOn;
lastUpdatedOn = other.lastUpdatedOn;
sortKey = other.sortKey;
owner = other.owner;
dest = other.dest;
open = other.open;
@ -534,14 +531,6 @@ public final class Change {
return rowVersion;
}
public String getSortKey() {
return sortKey;
}
public void setSortKey(final String newSortKey) {
sortKey = newSortKey;
}
public Account.Id getOwner() {
return owner;
}

View File

@ -55,11 +55,6 @@ public interface ChangeAccess extends Access<Change, Change.Id> {
@Query("WHERE open = true AND dest = ?")
ResultSet<Change> byBranchOpenAll(Branch.NameKey p) throws OrmException;
@Query("WHERE open = true AND dest.projectName = ? AND sortKey < ?"
+ " ORDER BY sortKey DESC LIMIT ?")
ResultSet<Change> byProjectOpenNext(Project.NameKey p, String sortKey,
int limit) throws OrmException;
@Query
ResultSet<Change> all() throws OrmException;
}

View File

@ -75,7 +75,7 @@ ON changes (status, dest_project_name, dest_branch_name, last_updated_on);
-- covers: byProjectOpenAll
CREATE INDEX changes_byProjectOpen
ON changes (open, dest_project_name, sort_key);
ON changes (open, dest_project_name, last_updated_on);
-- covers: byProject
CREATE INDEX changes_byProject

View File

@ -83,7 +83,7 @@ ON changes (status, dest_project_name, dest_branch_name, last_updated_on)
-- covers: byProjectOpenPrev, byProjectOpenNext
CREATE INDEX changes_byProjectOpen
ON changes (open, dest_project_name, sort_key)
ON changes (open, dest_project_name, last_updated_on);
#
-- covers: byProject

View File

@ -124,7 +124,7 @@ WHERE status = 's';
-- covers: byProjectOpenAll
CREATE INDEX changes_byProjectOpen
ON changes (dest_project_name, sort_key)
ON changes (dest_project_name, last_updated_on)
WHERE open = 'Y';
-- covers: byProject

View File

@ -15,15 +15,10 @@
package com.google.gerrit.server;
import static com.google.gerrit.server.change.PatchSetInserter.ValidatePolicy.RECEIVE_COMMITS;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static java.util.concurrent.TimeUnit.MINUTES;
import static java.util.concurrent.TimeUnit.SECONDS;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Optional;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.primitives.Ints;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.reviewdb.client.Change;
@ -85,15 +80,6 @@ import java.util.Map;
@Singleton
public class ChangeUtil {
/**
* Epoch for sort key calculations, Tue Sep 30 2008 17:00:00.
* <p>
* We overrun approximately 4,083 years later, so ~6092.
*/
@VisibleForTesting
private static final long SORT_KEY_EPOCH_MINS =
MINUTES.convert(1222819200L, SECONDS);
private static final Object uuidLock = new Object();
private static final int SEED = 0x2418e6f9;
private static int uuidPrefix;
@ -150,7 +136,6 @@ public class ChangeUtil {
public static void updated(Change c) {
c.setLastUpdatedOn(TimeUtil.nowTs());
computeSortKey(c);
}
public static void insertAncestors(ReviewDb db, PatchSet.Id id, RevCommit src)
@ -166,29 +151,6 @@ public class ChangeUtil {
db.patchSetAncestors().insert(toInsert);
}
public static String sortKey(long lastUpdatedMs, int id) {
long lastUpdatedMins = MINUTES.convert(lastUpdatedMs, MILLISECONDS);
long minsSinceEpoch = lastUpdatedMins - SORT_KEY_EPOCH_MINS;
StringBuilder r = new StringBuilder(16);
r.setLength(16);
formatHexInt(r, 0, Ints.checkedCast(minsSinceEpoch));
formatHexInt(r, 8, id);
return r.toString();
}
public static long parseSortKey(String sortKey) {
if ("z".equals(sortKey)) {
return Long.MAX_VALUE;
}
return Long.parseLong(sortKey, 16);
}
public static void computeSortKey(Change c) {
long lastUpdatedMs = c.getLastUpdatedOn().getTime();
int id = c.getId().get();
c.setSortKey(sortKey(lastUpdatedMs, id));
}
public static PatchSet.Id nextPatchSetId(Map<String, Ref> allRefs,
PatchSet.Id id) {
PatchSet.Id next = nextPatchSetId(id);
@ -585,19 +547,4 @@ public class ChangeUtil {
public static PatchSet.Id nextPatchSetId(PatchSet.Id id) {
return new PatchSet.Id(id.getParentKey(), id.get() + 1);
}
private static final char[] hexchar =
{'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', //
'a', 'b', 'c', 'd', 'e', 'f'};
private static void formatHexInt(final StringBuilder dst, final int p, int w) {
int o = p + 7;
while (o >= p && w != 0) {
dst.setCharAt(o--, hexchar[w & 0xf]);
w >>>= 4;
}
while (o >= p) {
dst.setCharAt(o--, '0');
}
}
}

View File

@ -136,7 +136,6 @@ public class ChangeInserter {
patchSet.setRevision(new RevId(commit.name()));
patchSetInfo = patchSetInfoFactory.get(commit, patchSet.getId());
change.setCurrentPatchSet(patchSetInfo);
ChangeUtil.computeSortKey(change);
}
public Change getChange() {

View File

@ -379,7 +379,6 @@ public class ChangeJson {
out.created = in.getCreatedOn();
out.updated = in.getLastUpdatedOn();
out._number = in.getId().get();
out._sortkey = in.getSortKey();
out.starred = userProvider.get().getStarredChanges().contains(in.getId())
? true
: null;

View File

@ -47,7 +47,6 @@ import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.ProjectUtil;
@ -271,7 +270,6 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
if (change.getStatus().isOpen()) {
change.setStatus(Change.Status.SUBMITTED);
change.setLastUpdatedOn(timestamp);
ChangeUtil.computeSortKey(change);
return change;
}
return null;

View File

@ -31,7 +31,6 @@ public class ChangeAttribute {
public Long createdOn;
public Long lastUpdated;
public String sortKey;
public Boolean open;
public Change.Status status;
public List<MessageAttribute> comments;

View File

@ -18,5 +18,5 @@ public class QueryStatsAttribute {
public final String type = "stats";
public int rowCount;
public long runTimeMilliseconds;
public String resumeSortKey;
public boolean moreChanges;
}

View File

@ -164,7 +164,6 @@ public class EventFactory {
public void extend(ChangeAttribute a, Change change) {
a.createdOn = change.getCreatedOn().getTime() / 1000L;
a.lastUpdated = change.getLastUpdatedOn().getTime() / 1000L;
a.sortKey = change.getSortKey();
a.open = change.getStatus().isOpen();
}

View File

@ -44,6 +44,7 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.util.List;
import java.util.Map;
import java.util.Set;
@ -173,7 +174,7 @@ public class ReceiveCommitsAdvertiseRefsHook implements AdvertiseRefsHook {
return toInclude;
}
private Iterable<ChangeData> queryRecentChanges(int limit)
private List<ChangeData> queryRecentChanges(int limit)
throws OrmException {
QueryProcessor qp = queryProcessor.get();
qp.setLimit(limit);

View File

@ -246,9 +246,7 @@ public class OutputStreamQuery {
}
stats.rowCount = results.changes().size();
if (results.moreChanges()) {
stats.resumeSortKey = c.sortKey;
}
stats.moreChanges = results.moreChanges();
stats.runTimeMilliseconds =
TimeUtil.nowMs() - stats.runTimeMilliseconds;
show(stats);

View File

@ -32,7 +32,7 @@ import java.util.List;
/** A version of the database schema. */
public abstract class SchemaVersion {
/** The current schema version. */
public static final Class<Schema_101> C = Schema_101.class;
public static final Class<Schema_102> C = Schema_102.class;
public static class Module extends AbstractModule {
@Override

View File

@ -0,0 +1,51 @@
// Copyright (C) 2014 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.schema;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gwtorm.jdbc.JdbcSchema;
import com.google.gwtorm.schema.sql.DialectPostgreSQL;
import com.google.gwtorm.schema.sql.SqlDialect;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import java.sql.SQLException;
import java.sql.Statement;
public class Schema_102 extends SchemaVersion {
@Inject
Schema_102(Provider<Schema_100> prior) {
super(prior);
}
@Override
protected void migrateData(ReviewDb db, UpdateUI ui)
throws OrmException, SQLException {
JdbcSchema schema = (JdbcSchema) db;
SqlDialect dialect = schema.getDialect();
try (Statement stmt = schema.getConnection().createStatement()) {
stmt.executeUpdate("DROP INDEX changes_byProjectOpen");
if (dialect instanceof DialectPostgreSQL) {
stmt.executeUpdate("CREATE INDEX changes_byProjectOpen"
+ " ON changes (dest_project_name, last_updated_on)"
+ " WHERE open = 'Y'");
} else {
stmt.executeUpdate("CREATE INDEX changes_byProjectOpen"
+ " ON changes (open, dest_project_name, last_updated_on)");
}
}
}
}

View File

@ -34,7 +34,6 @@ import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.PatchSetApproval.LabelId;
import com.google.gerrit.reviewdb.client.PatchSetInfo;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountManager;
import com.google.gerrit.server.account.AuthRequest;
@ -112,7 +111,6 @@ public class LabelNormalizerTest {
new Change.Id(1), userId,
new Branch.NameKey(allProjects, "refs/heads/master"),
TimeUtil.nowTs());
ChangeUtil.computeSortKey(change);
PatchSetInfo ps = new PatchSetInfo(new PatchSet.Id(change.getId(), 1));
ps.setSubject("Test change");
change.setCurrentPatchSet(ps);

View File

@ -25,7 +25,6 @@ import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetInfo;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.AllUsersNameProvider;
@ -114,6 +113,5 @@ public class TestChanges {
change.getId(), curr != null ? curr.get() + 1 : 1));
ps.setSubject("Change subject");
change.setCurrentPatchSet(ps);
ChangeUtil.computeSortKey(change);
}
}