opendev/gerrit
Code Issues Proposed changes

Manage database connections directly in PatchScriptFactory

Browse Source 
This should open/close database connections in the PatchScript factory
itself, making sure that we do not hold on to them for too long, which
was causing a too large number of open connections on the live server.

Change-Id: I1b7749659a6a8afedb745ed8e7aadfeb91adf3ef
This commit is contained in:
Nico Sallembien
2010-02-01 10:45:50 -08:00
parent 1248f6789f
commit 9fbcd80165
1 changed files with 29 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

52
gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchScriptFactory.java
View File

@@ -35,6 +35,7 @@ import com.google.gerrit.server.patch.PatchListKey;
import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gwtorm.client.OrmException; import com.google.gwtorm.client.OrmException;
import com.google.gwtorm.client.SchemaFactory;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
import com.google.inject.assistedinject.Assisted; import com.google.inject.assistedinject.Assisted;
@@ -64,7 +65,7 @@ class PatchScriptFactory extends Handler<PatchScript> {
private final GitRepositoryManager repoManager; private final GitRepositoryManager repoManager;
private final Provider<PatchScriptBuilder> builderFactory; private final Provider<PatchScriptBuilder> builderFactory;
private final PatchListCache patchListCache; private final PatchListCache patchListCache;
private final ReviewDb db; private final SchemaFactory<ReviewDb> schemaFactory;
private final ChangeControl.Factory changeControlFactory; private final ChangeControl.Factory changeControlFactory;
private final Patch.Key patchKey; private final Patch.Key patchKey;
@@ -90,7 +91,7 @@ class PatchScriptFactory extends Handler<PatchScript> {
@Inject @Inject
PatchScriptFactory(final GitRepositoryManager grm, PatchScriptFactory(final GitRepositoryManager grm,
Provider<PatchScriptBuilder> builderFactory, Provider<PatchScriptBuilder> builderFactory,
final PatchListCache patchListCache, final ReviewDb db, final PatchListCache patchListCache, final SchemaFactory<ReviewDb> sf,
final ChangeControl.Factory changeControlFactory, final ChangeControl.Factory changeControlFactory,
@Assisted final Patch.Key patchKey, @Assisted final Patch.Key patchKey,
@Assisted("patchSetA") @Nullable final PatchSet.Id patchSetA, @Assisted("patchSetA") @Nullable final PatchSet.Id patchSetA,
@@ -99,7 +100,7 @@ class PatchScriptFactory extends Handler<PatchScript> {
this.repoManager = grm; this.repoManager = grm;
this.builderFactory = builderFactory; this.builderFactory = builderFactory;
this.patchListCache = patchListCache; this.patchListCache = patchListCache;
this.db = db; this.schemaFactory = sf;
this.changeControlFactory = changeControlFactory; this.changeControlFactory = changeControlFactory;
this.patchKey = patchKey; this.patchKey = patchKey;
@@ -118,15 +119,7 @@ class PatchScriptFactory extends Handler<PatchScript> {
control = changeControlFactory.validateFor(changeId); control = changeControlFactory.validateFor(changeId);
change = control.getChange(); change = control.getChange();
patchSet = db.patchSets().get(patchSetId); final CommentDetail comments = allComments();
if (patchSet == null) {
throw new NoSuchChangeException(changeId);
}
projectKey = change.getProject();
aId = psa != null ? toObjectId(db, psa) : null;
bId = toObjectId(db, psb);
try { try {
git = repoManager.openRepository(projectKey.get()); git = repoManager.openRepository(projectKey.get());
} catch (RepositoryNotFoundException e) { } catch (RepositoryNotFoundException e) {
@@ -139,7 +132,6 @@ class PatchScriptFactory extends Handler<PatchScript> {
final PatchList list = listFor(keyFor(settings.getWhitespace())); final PatchList list = listFor(keyFor(settings.getWhitespace()));
final PatchScriptBuilder b = newBuilder(list); final PatchScriptBuilder b = newBuilder(list);
final PatchListEntry contentWS = list.get(fileName); final PatchListEntry contentWS = list.get(fileName);
final CommentDetail comments = allComments(db);
final PatchListEntry contentActual; final PatchListEntry contentActual;
if (settings.getWhitespace() == Whitespace.IGNORE_NONE) { if (settings.getWhitespace() == Whitespace.IGNORE_NONE) {
@@ -220,19 +212,33 @@ class PatchScriptFactory extends Handler<PatchScript> {
} }
} }
private CommentDetail allComments(final ReviewDb db) throws OrmException { private CommentDetail allComments()
final CommentDetail r = new CommentDetail(psa, psb); throws OrmException, NoSuchChangeException {
final String pn = patchKey.get(); ReviewDb db = schemaFactory.open();
for (PatchLineComment p : db.patchComments().published(changeId, pn)) { try {
r.include(p); final CommentDetail r = new CommentDetail(psa, psb);
} patchSet = db.patchSets().get(patchSetId);
if (patchSet == null) {
throw new NoSuchChangeException(changeId);
}
if (control.getCurrentUser() instanceof IdentifiedUser) { projectKey = change.getProject();
for (PatchLineComment p : db.patchComments().draft(changeId, pn, aId = psa != null ? toObjectId(db, psa) : null;
((IdentifiedUser) control.getCurrentUser()).getAccountId())) { bId = toObjectId(db, psb);
final String pn = patchKey.get();
for (PatchLineComment p : db.patchComments().published(changeId, pn)) {
r.include(p); r.include(p);
} }
if (control.getCurrentUser() instanceof IdentifiedUser) {
for (PatchLineComment p : db.patchComments().draft(changeId, pn,
((IdentifiedUser) control.getCurrentUser()).getAccountId())) {
r.include(p);
}
}
return r;
} finally {
db.close();
} }
return r;
} }
} }