From 73b8fd117c7f461f760d9820b7c395a885e40f5c Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Tue, 11 Feb 2020 16:44:50 +0100 Subject: [PATCH 1/4] Write upload-pack metrics to sshd log In order to enable detailed performance and problem analysis add upload-pack metrics to the sshd log. The log format for upload-pack log records is now enhanced to: - timestamp - session id - thread name - user name - account id - message - command wait time - command execution time - time negotiating - time searching for reuse - time searching for sizes - time counting - time compressing - time writing - total time in UploadPack - bitmap index misses (-1 means no bitmap index available) - total deltas - total objects - total bytes transferred - client agent If statistics aren't available e.g. since an exception occurred they are logged as -1. Change-Id: I4ec08579cedfeb6a30eb41d2e2a110f4f8eee8fa --- java/com/google/gerrit/sshd/BaseCommand.java | 4 +-- .../gerrit/sshd/CommandFactoryProvider.java | 8 ++++- java/com/google/gerrit/sshd/SshLog.java | 9 +++++ java/com/google/gerrit/sshd/SshLogLayout.java | 2 ++ .../google/gerrit/sshd/commands/Upload.java | 36 +++++++++++++++++++ 5 files changed, 56 insertions(+), 3 deletions(-) diff --git a/java/com/google/gerrit/sshd/BaseCommand.java b/java/com/google/gerrit/sshd/BaseCommand.java index 472f1c7218..7c77a2c091 100644 --- a/java/com/google/gerrit/sshd/BaseCommand.java +++ b/java/com/google/gerrit/sshd/BaseCommand.java @@ -79,7 +79,7 @@ public abstract class BaseCommand implements Command { protected OutputStream out; protected OutputStream err; - private ExitCallback exit; + protected ExitCallback exit; @Inject protected CurrentUser user; @@ -87,7 +87,7 @@ public abstract class BaseCommand implements Command { @Inject private CmdLineParser.Factory cmdLineParserFactory; - @Inject private RequestCleanup cleanup; + @Inject protected RequestCleanup cleanup; @Inject @CommandExecutor private ScheduledThreadPoolExecutor executor; diff --git a/java/com/google/gerrit/sshd/CommandFactoryProvider.java b/java/com/google/gerrit/sshd/CommandFactoryProvider.java index 3fb2ed411b..89a1654101 100644 --- a/java/com/google/gerrit/sshd/CommandFactoryProvider.java +++ b/java/com/google/gerrit/sshd/CommandFactoryProvider.java @@ -194,7 +194,7 @@ class CommandFactoryProvider implements Provider, LifecycleListe @Override public void onExit(int rc, String exitMessage) { exit.onExit(translateExit(rc), exitMessage); - log(rc); + log(rc, exitMessage); } @Override @@ -232,6 +232,12 @@ class CommandFactoryProvider implements Provider, LifecycleListe } } + private void log(int rc, String message) { + if (logged.compareAndSet(false, true)) { + log.onExecute(cmd, rc, ctx.getSession(), message); + } + } + @Override public void destroy() { Future future = task.getAndSet(null); diff --git a/java/com/google/gerrit/sshd/SshLog.java b/java/com/google/gerrit/sshd/SshLog.java index 6ea03c6e59..e8df4416c0 100644 --- a/java/com/google/gerrit/sshd/SshLog.java +++ b/java/com/google/gerrit/sshd/SshLog.java @@ -53,6 +53,7 @@ class SshLog implements LifecycleListener, GerritConfigListener { private static final String P_EXEC = "executionTime"; private static final String P_STATUS = "status"; private static final String P_AGENT = "agent"; + private static final String P_MESSAGE = "message"; private final Provider session; private final Provider context; @@ -147,6 +148,10 @@ class SshLog implements LifecycleListener, GerritConfigListener { } void onExecute(DispatchCommand dcmd, int exitValue, SshSession sshSession) { + onExecute(dcmd, exitValue, sshSession, null); + } + + void onExecute(DispatchCommand dcmd, int exitValue, SshSession sshSession, String message) { final Context ctx = context.get(); ctx.finished = TimeUtil.nowMs(); @@ -180,6 +185,10 @@ class SshLog implements LifecycleListener, GerritConfigListener { event.setProperty(P_AGENT, peerAgent); } + if (message != null) { + event.setProperty(P_MESSAGE, message); + } + if (async != null) { async.append(event); } diff --git a/java/com/google/gerrit/sshd/SshLogLayout.java b/java/com/google/gerrit/sshd/SshLogLayout.java index fd7fef18e4..f16dd732e7 100644 --- a/java/com/google/gerrit/sshd/SshLogLayout.java +++ b/java/com/google/gerrit/sshd/SshLogLayout.java @@ -30,6 +30,7 @@ public final class SshLogLayout extends Layout { private static final String P_EXEC = "executionTime"; private static final String P_STATUS = "status"; private static final String P_AGENT = "agent"; + private static final String P_MESSAGE = "message"; private final Calendar calendar; private long lastTimeMillis; @@ -68,6 +69,7 @@ public final class SshLogLayout extends Layout { opt(P_WAIT, buf, event); opt(P_EXEC, buf, event); + opt(P_MESSAGE, buf, event); opt(P_STATUS, buf, event); opt(P_AGENT, buf, event); diff --git a/java/com/google/gerrit/sshd/commands/Upload.java b/java/com/google/gerrit/sshd/commands/Upload.java index 6b5dd7533d..30fd80aca5 100644 --- a/java/com/google/gerrit/sshd/commands/Upload.java +++ b/java/com/google/gerrit/sshd/commands/Upload.java @@ -31,6 +31,7 @@ import com.google.gerrit.sshd.SshSession; import com.google.inject.Inject; import java.io.IOException; import java.util.List; +import org.eclipse.jgit.storage.pack.PackStatistics; import org.eclipse.jgit.transport.PostUploadHook; import org.eclipse.jgit.transport.PostUploadHookChain; import org.eclipse.jgit.transport.PreUploadHook; @@ -47,6 +48,8 @@ final class Upload extends AbstractGitCommand { @Inject private SshSession session; @Inject private PermissionBackend permissionBackend; + private PackStatistics stats; + @Override protected void runImpl() throws IOException, Failure { PermissionBackend.ForProject perm = @@ -76,6 +79,7 @@ final class Upload extends AbstractGitCommand { try { up.upload(in, out, err); session.setPeerAgent(up.getPeerUserAgent()); + stats = up.getStatistics(); } catch (UploadValidationException e) { // UploadValidationException is used by the UploadValidators to // stop the uploadPack. We do not want this exception to go beyond this @@ -86,4 +90,36 @@ final class Upload extends AbstractGitCommand { } } } + + @Override + protected void onExit(int rc) { + exit.onExit( + rc, + stats != null + ? stats.getTimeNegotiating() + + "ms " + + stats.getTimeSearchingForReuse() + + "ms " + + stats.getTimeSearchingForSizes() + + "ms " + + stats.getTimeCounting() + + "ms " + + stats.getTimeCompressing() + + "ms " + + stats.getTimeWriting() + + "ms " + + stats.getTimeTotal() + + "ms " + + stats.getBitmapIndexMisses() + + " " + + stats.getTotalDeltas() + + " " + + stats.getTotalObjects() + + " " + + stats.getTotalBytes() + : "-1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1"); + if (cleanup != null) { + cleanup.run(); + } + } } From 4b25dd3c15c6239cd87b22af09b1a3ec67d4074f Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Fri, 24 May 2019 10:15:40 -0700 Subject: [PATCH 2/4] Error Prone: Enable and fix ClassCanBeStatic Change-Id: Ice8202695c2a525d751682185e1aa94663e0fbe0 --- .../com/google/gerrit/server/account/AccountResolverTest.java | 2 +- tools/BUILD | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/javatests/com/google/gerrit/server/account/AccountResolverTest.java b/javatests/com/google/gerrit/server/account/AccountResolverTest.java index 6f074dde02..c37a9454fc 100644 --- a/javatests/com/google/gerrit/server/account/AccountResolverTest.java +++ b/javatests/com/google/gerrit/server/account/AccountResolverTest.java @@ -37,7 +37,7 @@ import java.util.stream.Stream; import org.junit.Test; public class AccountResolverTest extends GerritBaseTests { - private class TestSearcher extends StringSearcher { + private static class TestSearcher extends StringSearcher { private final String pattern; private final boolean shortCircuit; private final ImmutableList accounts; diff --git a/tools/BUILD b/tools/BUILD index ad639180c5..8ecf2273d4 100644 --- a/tools/BUILD +++ b/tools/BUILD @@ -30,7 +30,7 @@ java_package_configuration( "-Xep:BadComparable:WARN", "-Xep:BoxedPrimitiveConstructor:ERROR", "-Xep:CannotMockFinalClass:WARN", - "-Xep:ClassCanBeStatic:WARN", + "-Xep:ClassCanBeStatic:ERROR", "-Xep:ClassNewInstance:WARN", "-Xep:DateFormatConstant:ERROR", "-Xep:DefaultCharset:ERROR", From f386aa3de2469b476fa69a37167b4494dacb27c7 Mon Sep 17 00:00:00 2001 From: Marco Miller Date: Tue, 11 Feb 2020 16:33:09 -0500 Subject: [PATCH 3/4] Add support for Elasticsearch version 7.6.* - Upgrade elasticsearch-rest-client to 7.6.0. - Add V7_6 to the version manager, and use this in all the V7 tests. - Use the 7.6.0 image from blacktop in the test container; based on [1]. - Triggered by watching releases [2], which links to the release notes. [1] https://github.com/blacktop/docker-elasticsearch-alpine/pull/27 [2] https://github.com/elastic/elasticsearch/releases/tag/v7.6.0 Change-Id: I63e6c396b8c159b4ebdfe9817518f239fd62928c --- java/com/google/gerrit/elasticsearch/ElasticVersion.java | 3 ++- .../com/google/gerrit/acceptance/pgm/ElasticReindexIT.java | 2 +- .../com/google/gerrit/acceptance/ssh/ElasticIndexIT.java | 2 +- .../com/google/gerrit/elasticsearch/ElasticContainer.java | 2 ++ .../gerrit/elasticsearch/ElasticV7QueryAccountsTest.java | 2 +- .../gerrit/elasticsearch/ElasticV7QueryChangesTest.java | 2 +- .../gerrit/elasticsearch/ElasticV7QueryGroupsTest.java | 2 +- .../gerrit/elasticsearch/ElasticV7QueryProjectsTest.java | 2 +- .../com/google/gerrit/elasticsearch/ElasticVersionTest.java | 6 ++++++ tools/nongoogle.bzl | 4 ++-- 10 files changed, 18 insertions(+), 9 deletions(-) diff --git a/java/com/google/gerrit/elasticsearch/ElasticVersion.java b/java/com/google/gerrit/elasticsearch/ElasticVersion.java index 574a22623a..6f4ade2dd6 100644 --- a/java/com/google/gerrit/elasticsearch/ElasticVersion.java +++ b/java/com/google/gerrit/elasticsearch/ElasticVersion.java @@ -31,7 +31,8 @@ public enum ElasticVersion { V7_2("7.2.*"), V7_3("7.3.*"), V7_4("7.4.*"), - V7_5("7.5.*"); + V7_5("7.5.*"), + V7_6("7.6.*"); private final String version; private final Pattern pattern; diff --git a/javatests/com/google/gerrit/acceptance/pgm/ElasticReindexIT.java b/javatests/com/google/gerrit/acceptance/pgm/ElasticReindexIT.java index a5ac3a9163..181f6f064a 100644 --- a/javatests/com/google/gerrit/acceptance/pgm/ElasticReindexIT.java +++ b/javatests/com/google/gerrit/acceptance/pgm/ElasticReindexIT.java @@ -37,7 +37,7 @@ public class ElasticReindexIT extends AbstractReindexTests { @ConfigSuite.Config public static Config elasticsearchV7() { - return getConfig(ElasticVersion.V7_5); + return getConfig(ElasticVersion.V7_6); } @Override diff --git a/javatests/com/google/gerrit/acceptance/ssh/ElasticIndexIT.java b/javatests/com/google/gerrit/acceptance/ssh/ElasticIndexIT.java index 5348c53266..0d55d7a718 100644 --- a/javatests/com/google/gerrit/acceptance/ssh/ElasticIndexIT.java +++ b/javatests/com/google/gerrit/acceptance/ssh/ElasticIndexIT.java @@ -36,7 +36,7 @@ public class ElasticIndexIT extends AbstractIndexTests { @ConfigSuite.Config public static Config elasticsearchV7() { - return getConfig(ElasticVersion.V7_5); + return getConfig(ElasticVersion.V7_6); } @Override diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java index 53acdeb020..77c50efde5 100644 --- a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java +++ b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java @@ -64,6 +64,8 @@ public class ElasticContainer extends ElasticsearchContainer { return "blacktop/elasticsearch:7.4.2"; case V7_5: return "blacktop/elasticsearch:7.5.2"; + case V7_6: + return "blacktop/elasticsearch:7.6.0"; } throw new IllegalStateException("No tests for version: " + version.name()); } diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryAccountsTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryAccountsTest.java index 5e533fb1b8..826473f2c0 100644 --- a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryAccountsTest.java +++ b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryAccountsTest.java @@ -41,7 +41,7 @@ public class ElasticV7QueryAccountsTest extends AbstractQueryAccountsTest { return; } - container = ElasticContainer.createAndStart(ElasticVersion.V7_5); + container = ElasticContainer.createAndStart(ElasticVersion.V7_6); nodeInfo = new ElasticNodeInfo(container.getHttpHost().getPort()); } diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryChangesTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryChangesTest.java index 3c70ed5179..43a2dfc46e 100644 --- a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryChangesTest.java +++ b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryChangesTest.java @@ -47,7 +47,7 @@ public class ElasticV7QueryChangesTest extends AbstractQueryChangesTest { return; } - container = ElasticContainer.createAndStart(ElasticVersion.V7_5); + container = ElasticContainer.createAndStart(ElasticVersion.V7_6); nodeInfo = new ElasticNodeInfo(container.getHttpHost().getPort()); client = HttpAsyncClients.createDefault(); client.start(); diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryGroupsTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryGroupsTest.java index f2dcdaa335..0954e603c2 100644 --- a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryGroupsTest.java +++ b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryGroupsTest.java @@ -41,7 +41,7 @@ public class ElasticV7QueryGroupsTest extends AbstractQueryGroupsTest { return; } - container = ElasticContainer.createAndStart(ElasticVersion.V7_5); + container = ElasticContainer.createAndStart(ElasticVersion.V7_6); nodeInfo = new ElasticNodeInfo(container.getHttpHost().getPort()); } diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryProjectsTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryProjectsTest.java index e2de7fe040..e6d78770dd 100644 --- a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryProjectsTest.java +++ b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryProjectsTest.java @@ -41,7 +41,7 @@ public class ElasticV7QueryProjectsTest extends AbstractQueryProjectsTest { return; } - container = ElasticContainer.createAndStart(ElasticVersion.V7_5); + container = ElasticContainer.createAndStart(ElasticVersion.V7_6); nodeInfo = new ElasticNodeInfo(container.getHttpHost().getPort()); } diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java index b5c455d56e..9786e373c0 100644 --- a/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java +++ b/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java @@ -66,6 +66,9 @@ public class ElasticVersionTest { assertThat(ElasticVersion.forVersion("7.5.0")).isEqualTo(ElasticVersion.V7_5); assertThat(ElasticVersion.forVersion("7.5.1")).isEqualTo(ElasticVersion.V7_5); + + assertThat(ElasticVersion.forVersion("7.6.0")).isEqualTo(ElasticVersion.V7_6); + assertThat(ElasticVersion.forVersion("7.6.1")).isEqualTo(ElasticVersion.V7_6); } @Test @@ -92,6 +95,7 @@ public class ElasticVersionTest { assertThat(ElasticVersion.V7_3.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse(); assertThat(ElasticVersion.V7_4.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse(); assertThat(ElasticVersion.V7_5.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse(); + assertThat(ElasticVersion.V7_6.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse(); } @Test @@ -110,6 +114,7 @@ public class ElasticVersionTest { assertThat(ElasticVersion.V7_3.isV6OrLater()).isTrue(); assertThat(ElasticVersion.V7_4.isV6OrLater()).isTrue(); assertThat(ElasticVersion.V7_5.isV6OrLater()).isTrue(); + assertThat(ElasticVersion.V7_6.isV6OrLater()).isTrue(); } @Test @@ -128,5 +133,6 @@ public class ElasticVersionTest { assertThat(ElasticVersion.V7_3.isV7OrLater()).isTrue(); assertThat(ElasticVersion.V7_4.isV7OrLater()).isTrue(); assertThat(ElasticVersion.V7_5.isV7OrLater()).isTrue(); + assertThat(ElasticVersion.V7_6.isV7OrLater()).isTrue(); } } diff --git a/tools/nongoogle.bzl b/tools/nongoogle.bzl index 63e13192a8..7819502d5e 100644 --- a/tools/nongoogle.bzl +++ b/tools/nongoogle.bzl @@ -94,8 +94,8 @@ def declare_nongoogle_deps(): # and httpasyncclient as necessary. maven_jar( name = "elasticsearch-rest-client", - artifact = "org.elasticsearch.client:elasticsearch-rest-client:7.5.2", - sha1 = "e11393f600a425b7f62e6f653e19a9e53556fd79", + artifact = "org.elasticsearch.client:elasticsearch-rest-client:7.6.0", + sha1 = "3d56c1fca22af1aab5a1b23698ae9ec6f71db1a2", ) maven_jar( From 5a7a364ff28ae3cf80ff21c30fb9e5ed47ac80d3 Mon Sep 17 00:00:00 2001 From: Tao Zhou Date: Wed, 12 Feb 2020 08:42:01 +0100 Subject: [PATCH 4/4] Update web-component-tester to 6.5.1 This will fix the build failure as the sinon.js dependency issue fixed in upstream (wct 6.5.1) Change-Id: I871f08e59dcfc0717888aa9f0ade82fc3edc851d --- WORKSPACE | 4 ++-- lib/js/bower_archives.bzl | 2 +- package.json | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index 0b40765515..760153e20f 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -1185,8 +1185,8 @@ bower_archive( bower_archive( name = "web-component-tester", package = "polymer/web-component-tester", - sha1 = "62739cb633fccfddc5eeed98e9e3f69cd0388b5b", - version = "6.5.0", + sha1 = "d84f6a13bde5f8fd39ee208d43f33925410530d7", + version = "6.5.1", ) # Bower component transitive dependencies. diff --git a/lib/js/bower_archives.bzl b/lib/js/bower_archives.bzl index d84a2b6e7a..a7ce585537 100644 --- a/lib/js/bower_archives.bzl +++ b/lib/js/bower_archives.bzl @@ -149,7 +149,7 @@ def load_bower_archives(): ) bower_archive( name = "sinonjs", - package = "sinonjs", + package = "Polymer/sinon.js", version = "1.17.1", sha1 = "a26a6aab7358807de52ba738770f6ac709afd240", ) diff --git a/package.json b/package.json index 07eb7b384b..740eb4edee 100644 --- a/package.json +++ b/package.json @@ -11,7 +11,7 @@ "fried-twinkie": "^0.2.2", "polylint": "^2.10.4", "typescript": "^2.x.x", - "web-component-tester": "^6.5.0" + "web-component-tester": "^6.5.1" }, "scripts": { "start": "polygerrit-ui/run-server.sh",