From a4455eab77e55f21c508cf9877ad40413aefbcc8 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Wed, 20 Nov 2013 23:19:39 -0800 Subject: [PATCH] Hide multi-master cache coherency issues on preferences User preferences are held in memory in the accounts cache, but can be stale in a multi-master configuration due to writes being handled on a different server process. Paper over the lack of cache coherency by reading the preferences from the database anytime they are requested through the RPC or REST APIs. Change-Id: I2a14e1f50e908a1d5ea5628e5a05e392be2e0d42 --- .../httpd/rpc/account/AccountServiceImpl.java | 7 ++++++- .../server/account/GetDiffPreferences.java | 19 +++++++++++++++---- .../gerrit/server/account/GetPreferences.java | 18 ++++++++++++++---- 3 files changed, 35 insertions(+), 9 deletions(-) diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/AccountServiceImpl.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/AccountServiceImpl.java index ebeef670e7..7b02630096 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/AccountServiceImpl.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/AccountServiceImpl.java @@ -69,7 +69,12 @@ class AccountServiceImpl extends BaseServiceImplementation implements } public void myAccount(final AsyncCallback callback) { - callback.onSuccess(currentUser.get().getAccount()); + run(callback, new Action() { + @Override + public Account run(ReviewDb db) throws OrmException { + return db.accounts().get(currentUser.get().getAccountId()); + } + }); } public void changePreferences(final AccountGeneralPreferences pref, diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GetDiffPreferences.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GetDiffPreferences.java index 8e65a23ff0..459864a3b7 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GetDiffPreferences.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GetDiffPreferences.java @@ -16,28 +16,39 @@ package com.google.gerrit.server.account; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.RestReadView; +import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountDiffPreference; import com.google.gerrit.reviewdb.client.AccountDiffPreference.Whitespace; +import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.CurrentUser; +import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Provider; public class GetDiffPreferences implements RestReadView { - private final Provider self; + private final Provider db; @Inject - GetDiffPreferences(Provider self) { + GetDiffPreferences(Provider self, Provider db) { this.self = self; + this.db = db; } @Override - public DiffPreferencesInfo apply(AccountResource rsrc) throws AuthException { + public DiffPreferencesInfo apply(AccountResource rsrc) + throws AuthException, OrmException { if (self.get() != rsrc.getUser() && !self.get().getCapabilities().canAdministrateServer()) { throw new AuthException("restricted to administrator"); } - return DiffPreferencesInfo.parse(rsrc.getUser().getAccountDiffPreference()); + + Account.Id userId = rsrc.getUser().getAccountId(); + AccountDiffPreference a = db.get().accountDiffPreferences().get(userId); + if (a == null) { + a = new AccountDiffPreference(userId); + } + return DiffPreferencesInfo.parse(a); } static class DiffPreferencesInfo { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GetPreferences.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GetPreferences.java index 4d3b913240..6f30b166ca 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GetPreferences.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GetPreferences.java @@ -15,7 +15,9 @@ package com.google.gerrit.server.account; import com.google.gerrit.extensions.restapi.AuthException; +import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.RestReadView; +import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountGeneralPreferences; import com.google.gerrit.reviewdb.client.AccountGeneralPreferences.ChangeScreen; import com.google.gerrit.reviewdb.client.AccountGeneralPreferences.CommentVisibilityStrategy; @@ -24,26 +26,34 @@ import com.google.gerrit.reviewdb.client.AccountGeneralPreferences.DiffView; import com.google.gerrit.reviewdb.client.AccountGeneralPreferences.DownloadCommand; import com.google.gerrit.reviewdb.client.AccountGeneralPreferences.DownloadScheme; import com.google.gerrit.reviewdb.client.AccountGeneralPreferences.TimeFormat; +import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.CurrentUser; +import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Provider; public class GetPreferences implements RestReadView { private final Provider self; + private final Provider db; @Inject - GetPreferences(Provider self) { + GetPreferences(Provider self, Provider db) { this.self = self; + this.db = db; } @Override - public PreferenceInfo apply(AccountResource rsrc) throws AuthException { + public PreferenceInfo apply(AccountResource rsrc) + throws AuthException, ResourceNotFoundException, OrmException { if (self.get() != rsrc.getUser() && !self.get().getCapabilities().canAdministrateServer()) { throw new AuthException("restricted to administrator"); } - return new PreferenceInfo(rsrc.getUser().getAccount() - .getGeneralPreferences()); + Account a = db.get().accounts().get(rsrc.getUser().getAccountId()); + if (a == null) { + throw new ResourceNotFoundException(); + } + return new PreferenceInfo(a.getGeneralPreferences()); } static class PreferenceInfo {