opendev/gerrit
Code Issues Proposed changes

Drop the lastUsedOn fields from AccountSshKeys, AccountExternalIds

Browse Source 
By dropping this column we avoid updating the database during the
authentication phase for a new web session, or a new SSH session.

Avoiding this update on read-only slaves can improve performance if
they have very slow connectivity to the master database.  On the
master node, avoiding the update saves a few relatively useless
disk IOs on the database.

This change also removes one of the issues we had with converting
the user account schema to GiMD.  Updating the account database
in Git every time a user authenticates would create thousands of
objects per hour on busy servers, all of them almost completely
useless to the system administrator.

This change makes a trade-off of faster session authentication, in
return for a bit less information about account usage.  Instead of
pulling account usage from the database, we'll have to start writing
to local log files instead.

Change-Id: Ie335f5e6e0f5ff16b987d4e2698bc25ff14bd989
Signed-off-by: Shawn O. Pearce <sop@google.com>
This commit is contained in:
Shawn O. Pearce
2009-12-28 18:11:22 -08:00
parent 4719339fde
commit 5f95a93f0f
12 changed files with 40 additions and 111 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
gerrit-gwtui/src/main/java/com/google/gerrit/client/account/AccountConstants.java
View File

@@ -53,8 +53,6 @@ public interface AccountConstants extends Constants {
String sshKeyAlgorithm();
String sshKeyKey();
String sshKeyComment();
String sshKeyLastUsed();
String sshKeyStored();
String addSshKeyPanelHeader();
String addSshKeyHelp();
@@ -65,7 +63,6 @@ public interface AccountConstants extends Constants {
String sshHostKeyFingerprint();
String sshHostKeyKnownHostEntry();
String webIdLastUsed();
String webIdStatus();
String webIdEmail();
String webIdIdentity();

3
gerrit-gwtui/src/main/java/com/google/gerrit/client/account/AccountConstants.properties
View File

@@ -33,14 +33,11 @@ sshKeyInvalid = Invalid Key
sshKeyAlgorithm = Algorithm
sshKeyKey = Key
sshKeyComment = Comment
sshKeyLastUsed = Last Used
sshKeyStored = Stored
sshHostKeyTitle = Server Host Key
sshHostKeyFingerprint = Fingerprint:
sshHostKeyKnownHostEntry = Entry for <code>~/.ssh/known_hosts</code>:
webIdLastUsed = Last Login
webIdStatus = Status
webIdEmail = Email Address
webIdIdentity = Identity

28
gerrit-gwtui/src/main/java/com/google/gerrit/client/account/ExternalIdPanel.java
View File

@@ -14,7 +14,6 @@
package com.google.gerrit.client.account;
import com.google.gerrit.client.FormatUtil;
import com.google.gerrit.client.Gerrit;
import com.google.gerrit.client.auth.openid.OpenIdSignInDialog;
import com.google.gerrit.client.auth.openid.OpenIdUtil;
@@ -91,17 +90,15 @@ class ExternalIdPanel extends Composite {
private class IdTable extends FancyFlexTable<AccountExternalId> {
IdTable() {
table.setWidth("");
table.setText(0, 2, Util.C.webIdLastUsed());
table.setText(0, 3, Util.C.webIdStatus());
table.setText(0, 4, Util.C.webIdEmail());
table.setText(0, 5, Util.C.webIdIdentity());
table.setText(0, 2, Util.C.webIdStatus());
table.setText(0, 3, Util.C.webIdEmail());
table.setText(0, 4, Util.C.webIdIdentity());
final FlexCellFormatter fmt = table.getFlexCellFormatter();
fmt.addStyleName(0, 1, Gerrit.RESOURCES.css().iconHeader());
fmt.addStyleName(0, 2, Gerrit.RESOURCES.css().dataHeader());
fmt.addStyleName(0, 3, Gerrit.RESOURCES.css().dataHeader());
fmt.addStyleName(0, 4, Gerrit.RESOURCES.css().dataHeader());
fmt.addStyleName(0, 5, Gerrit.RESOURCES.css().dataHeader());
}
void deleteChecked() {
@@ -176,31 +173,24 @@ class ExternalIdPanel extends Composite {
} else {
table.setText(row, 1, "");
}
if (k.getLastUsedOn() != null) {
table.setText(row, 2, FormatUtil.mediumFormat(k.getLastUsedOn()));
} else {
table.setText(row, 2, "");
}
if (k.isTrusted()) {
table.setText(row, 3, "");
table.setText(row, 2, "");
} else {
table.setText(row, 3, Util.C.untrustedProvider());
fmt.addStyleName(row, 3, Gerrit.RESOURCES.css()
table.setText(row, 2, Util.C.untrustedProvider());
fmt.addStyleName(row, 2, Gerrit.RESOURCES.css()
.identityUntrustedExternalId());
}
if (k.getEmailAddress() != null && k.getEmailAddress().length() > 0) {
table.setText(row, 4, k.getEmailAddress());
table.setText(row, 3, k.getEmailAddress());
} else {
table.setText(row, 4, "");
table.setText(row, 3, "");
}
table.setText(row, 5, describe(k));
table.setText(row, 4, describe(k));
fmt.addStyleName(row, 1, Gerrit.RESOURCES.css().iconCell());
fmt.addStyleName(row, 2, Gerrit.RESOURCES.css().dataCell());
fmt.addStyleName(row, 3, Gerrit.RESOURCES.css().dataCell());
fmt.addStyleName(row, 3, Gerrit.RESOURCES.css().cLastUpdate());
fmt.addStyleName(row, 4, Gerrit.RESOURCES.css().dataCell());
fmt.addStyleName(row, 5, Gerrit.RESOURCES.css().dataCell());
setRowItem(row, k);
}

11
gerrit-gwtui/src/main/java/com/google/gerrit/client/account/SshPanel.java
View File

@@ -15,7 +15,6 @@
package com.google.gerrit.client.account;
import com.google.gerrit.client.ErrorDialog;
import com.google.gerrit.client.FormatUtil;
import com.google.gerrit.client.Gerrit;
import com.google.gerrit.client.rpc.GerritCallback;
import com.google.gerrit.client.ui.FancyFlexTable;
@@ -495,8 +494,6 @@ class SshPanel extends Composite {
table.setText(0, 3, Util.C.sshKeyAlgorithm());
table.setText(0, 4, Util.C.sshKeyKey());
table.setText(0, 5, Util.C.sshKeyComment());
table.setText(0, 6, Util.C.sshKeyLastUsed());
table.setText(0, 7, Util.C.sshKeyStored());
final FlexCellFormatter fmt = table.getFlexCellFormatter();
fmt.addStyleName(0, 1, Gerrit.RESOURCES.css().iconHeader());
@@ -504,8 +501,6 @@ class SshPanel extends Composite {
fmt.addStyleName(0, 3, Gerrit.RESOURCES.css().dataHeader());
fmt.addStyleName(0, 4, Gerrit.RESOURCES.css().dataHeader());
fmt.addStyleName(0, 5, Gerrit.RESOURCES.css().dataHeader());
fmt.addStyleName(0, 6, Gerrit.RESOURCES.css().dataHeader());
fmt.addStyleName(0, 7, Gerrit.RESOURCES.css().dataHeader());
}
void deleteChecked() {
@@ -561,17 +556,13 @@ class SshPanel extends Composite {
table.setText(row, 3, k.getAlgorithm());
table.setText(row, 4, elide(k.getEncodedKey(), 40));
table.setText(row, 5, k.getComment());
table.setText(row, 6, FormatUtil.mediumFormat(k.getLastUsedOn()));
table.setText(row, 7, FormatUtil.mediumFormat(k.getStoredOn()));
fmt.addStyleName(row, 1, Gerrit.RESOURCES.css().iconCell());
fmt.addStyleName(row, 2, Gerrit.RESOURCES.css().iconCell());
fmt.addStyleName(row, 4, Gerrit.RESOURCES.css().sshKeyPanelEncodedKey());
for (int c = 3; c <= 7; c++) {
for (int c = 3; c <= 5; c++) {
fmt.addStyleName(row, c, Gerrit.RESOURCES.css().dataCell());
}
fmt.addStyleName(row, 6, Gerrit.RESOURCES.css().cLastUpdate());
fmt.addStyleName(row, 7, Gerrit.RESOURCES.css().cLastUpdate());
setRowItem(row, k);
}

13
gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountExternalId.java
View File

@@ -17,8 +17,6 @@ package com.google.gerrit.reviewdb;
import com.google.gwtorm.client.Column;
import com.google.gwtorm.client.StringKey;
import java.sql.Timestamp;
/** Association of an external account identifier to a local {@link Account}. */
public final class AccountExternalId {
public static final String SCHEME_GERRIT = "gerrit:";
@@ -59,9 +57,6 @@ public final class AccountExternalId {
@Column(notNull = false)
protected String emailAddress;
@Column(notNull = false)
protected Timestamp lastUsedOn;
/** <i>computed value</i> is this identity trusted by the site administrator? */
protected boolean trusted;
@@ -103,14 +98,6 @@ public final class AccountExternalId {
emailAddress = e;
}
public Timestamp getLastUsedOn() {
return lastUsedOn;
}
public void setLastUsedOn() {
lastUsedOn = new Timestamp(System.currentTimeMillis());
}
public boolean isScheme(final String scheme) {
final String id = getExternalId();
return id != null && id.startsWith(scheme);

21
gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountSshKey.java
View File

@@ -17,8 +17,6 @@ package com.google.gerrit.reviewdb;
import com.google.gwtorm.client.Column;
import com.google.gwtorm.client.IntKey;
import java.sql.Timestamp;
/** An SSH key approved for use by an {@link Account}. */
public final class AccountSshKey {
public static class Id extends IntKey<Account.Id> {
@@ -61,22 +59,15 @@ public final class AccountSshKey {
@Column(length = Integer.MAX_VALUE)
protected String sshPublicKey;
@Column
protected Timestamp storedOn;
@Column
protected boolean valid;
@Column(notNull = false)
protected Timestamp lastUsedOn;
protected AccountSshKey() {
}
public AccountSshKey(final AccountSshKey.Id i, final String pub) {
id = i;
sshPublicKey = pub;
storedOn = new Timestamp(System.currentTimeMillis());
valid = true; // We can assume it is fine.
}
@@ -131,10 +122,6 @@ public final class AccountSshKey {
return parts[2];
}
public Timestamp getStoredOn() {
return storedOn;
}
public boolean isValid() {
return valid;
}
@@ -142,12 +129,4 @@ public final class AccountSshKey {
public void setInvalid() {
valid = false;
}
public Timestamp getLastUsedOn() {
return lastUsedOn;
}
public void setLastUsedOn() {
lastUsedOn = new Timestamp(System.currentTimeMillis());
}
}

4
gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountSshKeyAccess.java
View File

@@ -25,9 +25,9 @@ public interface AccountSshKeyAccess extends
@PrimaryKey("id")
AccountSshKey get(AccountSshKey.Id id) throws OrmException;
@Query("WHERE id.accountId = ? ORDER BY storedOn DESC")
@Query("WHERE id.accountId = ?")
ResultSet<AccountSshKey> byAccount(Account.Id id) throws OrmException;
@Query("WHERE id.accountId = ? AND valid = true ORDER BY storedOn DESC")
@Query("WHERE id.accountId = ? AND valid = true")
ResultSet<AccountSshKey> valid(Account.Id id) throws OrmException;
}

7
gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java
View File

@@ -21,7 +21,6 @@ import com.google.gerrit.reviewdb.AccountGroup;
import com.google.gerrit.reviewdb.AccountGroupMember;
import com.google.gerrit.reviewdb.AccountGroupMemberAudit;
import com.google.gerrit.reviewdb.ReviewDb;
import com.google.gerrit.reviewdb.AccountExternalId.Key;
import com.google.gerrit.server.config.AuthConfig;
import com.google.gwtorm.client.OrmException;
import com.google.gwtorm.client.SchemaFactory;
@@ -151,7 +150,6 @@ public class AccountManager {
account.setSshUserName(who.getSshUserName());
}
extId.setLastUsedOn();
db.accountExternalIds().update(Collections.singleton(extId), txn);
if (updateAccount) {
db.accounts().update(Collections.singleton(account), txn);
@@ -208,7 +206,6 @@ public class AccountManager {
final AccountExternalId newId = createId(accountId, who);
newId.setEmailAddress(who.getEmailAddress());
newId.setLastUsedOn();
if (openId.size() == 1) {
final AccountExternalId oldId = openId.get(0);
@@ -229,7 +226,7 @@ public class AccountManager {
final AccountExternalId oldId = v1.get(0);
final AccountExternalId newId = createId(oldId.getAccountId(), who);
newId.setEmailAddress(who.getEmailAddress());
newId.setLastUsedOn();
final Transaction txn = db.beginTransaction();
db.accountExternalIds().delete(Collections.singleton(oldId), txn);
db.accountExternalIds().insert(Collections.singleton(newId), txn);
@@ -245,7 +242,6 @@ public class AccountManager {
final Account account = new Account(newId);
final AccountExternalId extId = createId(newId, who);
extId.setLastUsedOn();
extId.setEmailAddress(who.getEmailAddress());
account.setFullName(who.getDisplayName());
account.setPreferredEmail(extId.getEmailAddress());
@@ -313,7 +309,6 @@ public class AccountManager {
final Transaction txn = db.beginTransaction();
extId = createId(to, who);
extId.setEmailAddress(who.getEmailAddress());
extId.setLastUsedOn();
db.accountExternalIds().insert(Collections.singleton(extId), txn);
if (who.getEmailAddress() != null) {

2
gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java
View File

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

25
gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_20.java Normal file
View File

@@ -0,0 +1,25 @@
// Copyright (C) 2009 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.inject.Inject;
import com.google.inject.Provider;
class Schema_20 extends SchemaVersion {
@Inject
Schema_20(Provider<Schema_19> prior) {
super(prior);
}
}

7
gerrit-sshd/src/main/java/com/google/gerrit/sshd/DatabasePubKeyAuth.java
View File

@@ -15,8 +15,6 @@
package com.google.gerrit.sshd;
import com.google.gerrit.reviewdb.AccountSshKey;
import com.google.gerrit.reviewdb.ReviewDb;
import com.google.gwtorm.client.SchemaFactory;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -35,12 +33,10 @@ import java.security.PublicKey;
@Singleton
class DatabasePubKeyAuth implements PublickeyAuthenticator {
private final SshKeyCacheImpl sshKeyCache;
private final SchemaFactory<ReviewDb> schema;
@Inject
DatabasePubKeyAuth(final SshKeyCacheImpl skc, final SchemaFactory<ReviewDb> sf) {
DatabasePubKeyAuth(final SshKeyCacheImpl skc) {
sshKeyCache = skc;
schema = sf;
}
public boolean authenticate(final String username,
@@ -63,7 +59,6 @@ class DatabasePubKeyAuth implements PublickeyAuthenticator {
}
}
key.updateLastUsed(schema);
session.setAttribute(SshUtil.CURRENT_ACCOUNT, key.getAccount());
return true;
}

27
gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshKeyCacheEntry.java
View File

@@ -16,20 +16,10 @@ package com.google.gerrit.sshd;
import com.google.gerrit.reviewdb.Account;
import com.google.gerrit.reviewdb.AccountSshKey;
import com.google.gerrit.reviewdb.ReviewDb;
import com.google.gwtorm.client.OrmException;
import com.google.gwtorm.client.SchemaFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.security.PublicKey;
import java.util.Collections;
class SshKeyCacheEntry {
private static final Logger log =
LoggerFactory.getLogger(SshKeyCacheEntry.class);
private final AccountSshKey.Id id;
private final PublicKey publicKey;
@@ -45,21 +35,4 @@ class SshKeyCacheEntry {
boolean match(final PublicKey inkey) {
return publicKey.equals(inkey);
}
void updateLastUsed(final SchemaFactory<ReviewDb> schema) {
try {
final ReviewDb db = schema.open();
try {
final AccountSshKey k = db.accountSshKeys().get(id);
if (k != null) {
k.setLastUsedOn();
db.accountSshKeys().update(Collections.singleton(k));
}
} finally {
db.close();
}
} catch (OrmException e) {
log.warn("Failed to update \"" + id + "\" SSH key used", e);
}
}
}