You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2020/10/09 17:30:52 UTC
[tomcat] branch 9.0.x updated: Partial fix for BZ 63362 provide h2
request statistics
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push:
new 708b169 Partial fix for BZ 63362 provide h2 request statistics
708b169 is described below
commit 708b1694de60e8c1fd0c3635da2f5a1aa701970f
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Jun 24 19:24:17 2020 +0100
Partial fix for BZ 63362 provide h2 request statistics
---
java/org/apache/coyote/AbstractProtocol.java | 35 +++++++++++++----
java/org/apache/coyote/LocalStrings.properties | 1 +
java/org/apache/coyote/UpgradeProtocol.java | 46 ++++++++++++++++++++++
java/org/apache/coyote/http2/Http2Protocol.java | 48 +++++++++++++++++++++--
java/org/apache/coyote/http2/StreamProcessor.java | 6 +++
webapps/docs/changelog.xml | 4 ++
6 files changed, 130 insertions(+), 10 deletions(-)
diff --git a/java/org/apache/coyote/AbstractProtocol.java b/java/org/apache/coyote/AbstractProtocol.java
index d9bab30..ab9dd38 100644
--- a/java/org/apache/coyote/AbstractProtocol.java
+++ b/java/org/apache/coyote/AbstractProtocol.java
@@ -69,12 +69,6 @@ public abstract class AbstractProtocol<S> implements ProtocolHandler,
/**
- * Name of MBean for the Global Request Processor.
- */
- protected ObjectName rgOname = null;
-
-
- /**
* Unique ID for this connector. Only used if the connector is configured
* to use a random port as the port will change if stop(), start() is
* called.
@@ -145,6 +139,14 @@ public abstract class AbstractProtocol<S> implements ProtocolHandler,
// ------------------------------- Properties managed by the ProtocolHandler
/**
+ * Name of MBean for the Global Request Processor.
+ */
+ protected ObjectName rgOname = null;
+ public ObjectName getGlobalRequestProcessorMBeanName() {
+ return rgOname;
+ }
+
+ /**
* The adapter provides the link between the ProtocolHandler and the
* connector.
*/
@@ -569,7 +571,8 @@ public abstract class AbstractProtocol<S> implements ProtocolHandler,
}
if (this.domain != null) {
- rgOname = new ObjectName(domain + ":type=GlobalRequestProcessor,name=" + getName());
+ ObjectName rgOname = new ObjectName(domain + ":type=GlobalRequestProcessor,name=" + getName());
+ this.rgOname = rgOname;
Registry.getRegistry(null, null).registerComponent(
getHandler().getGlobal(), rgOname, null);
}
@@ -579,6 +582,13 @@ public abstract class AbstractProtocol<S> implements ProtocolHandler,
endpoint.setDomain(domain);
endpoint.init();
+
+ UpgradeProtocol[] upgradeProtocols = findUpgradeProtocols();
+ for (UpgradeProtocol upgradeProtocol : upgradeProtocols) {
+ // Implementation note: Failure of one upgrade protocol fails the
+ // whole Connector
+ upgradeProtocol.init();
+ }
}
@@ -692,6 +702,16 @@ public abstract class AbstractProtocol<S> implements ProtocolHandler,
logPortOffset();
}
+ UpgradeProtocol[] upgradeProtocols = findUpgradeProtocols();
+ for (UpgradeProtocol upgradeProtocol : upgradeProtocols) {
+ try {
+ upgradeProtocol.destroy();
+ } catch (Throwable t) {
+ ExceptionUtils.handleThrowable(t);
+ getLog().error(sm.getString("abstractProtocol.upgradeProtocolDestroyError"), t);
+ }
+ }
+
try {
endpoint.destroy();
} finally {
@@ -709,6 +729,7 @@ public abstract class AbstractProtocol<S> implements ProtocolHandler,
}
}
+ ObjectName rgOname = getGlobalRequestProcessorMBeanName();
if (rgOname != null) {
Registry.getRegistry(null, null).unregisterComponent(rgOname);
}
diff --git a/java/org/apache/coyote/LocalStrings.properties b/java/org/apache/coyote/LocalStrings.properties
index 83960cb..43c8d64 100644
--- a/java/org/apache/coyote/LocalStrings.properties
+++ b/java/org/apache/coyote/LocalStrings.properties
@@ -36,6 +36,7 @@ abstractProcessor.socket.ssl=Exception getting SSL attributes
abstractProtocol.mbeanDeregistrationFailed=Failed to deregister MBean named [{0}] from MBean server [{1}]
abstractProtocol.processorRegisterError=Error registering request processor
abstractProtocol.processorUnregisterError=Error unregistering request processor
+abstractProtocol.upgradeProtocolDestroyError=Error destroying upgrade protocol
abstractProtocol.waitingProcessor.add=Added processor [{0}] to waiting processors
abstractProtocol.waitingProcessor.remove=Removed processor [{0}] from waiting processors
diff --git a/java/org/apache/coyote/UpgradeProtocol.java b/java/org/apache/coyote/UpgradeProtocol.java
index dc840df..50fe6c3 100644
--- a/java/org/apache/coyote/UpgradeProtocol.java
+++ b/java/org/apache/coyote/UpgradeProtocol.java
@@ -16,6 +16,7 @@
*/
package org.apache.coyote;
+import org.apache.coyote.http11.AbstractHttp11Protocol;
import org.apache.coyote.http11.upgrade.InternalHttpUpgradeHandler;
import org.apache.tomcat.util.net.SocketWrapperBase;
@@ -104,7 +105,52 @@ public interface UpgradeProtocol {
* handle any connections passed to this UpgradeProtocol via
* the HTTP upgrade mechanism
*/
+ public default void setHttp11Protocol(AbstractHttp11Protocol<?> protocol) {
+ // NO-OP
+ }
+
+ /**
+ * Configure the HTTP/1.1 protocol that this UpgradeProcotol is nested
+ * under. Connections passed to this UpgradeProtocol via HTTP upgrade will
+ * have been initially handled by this HTTP/1.1 protocol implementation.
+ * <p>
+ * The default implementation is to call
+ * {@link #setHttp11Protocol(AbstractHttp11Protocol)} if protocol is an
+ * instance of {@link AbstractHttp11Protocol} else this is a NO-OP.
+ *
+ * @param protocol The HTTP/1.1 protocol implementation that will initially
+ * handle any connections passed to this UpgradeProtocol via
+ * the HTTP upgrade mechanism
+ *
+ * @deprecated This will be removed in Tomcat 10. Use
+ * {@link #setHttp11Protocol(AbstractHttp11Protocol)} instead
+ */
+ @Deprecated
public default void setHttp11Protocol(AbstractProtocol<?> protocol) {
+ if (protocol instanceof AbstractHttp11Protocol) {
+ setHttp11Protocol((AbstractHttp11Protocol<?>) protocol);
+ }
+ }
+
+
+ /**
+ * Initialise the upgrade protocol. Called once the parent HTTP/1.1 protocol
+ * has initialised.
+ *
+ * @throws Exception If initialisation fails
+ */
+ public default void init() throws Exception {
+ // NO-OP
+ }
+
+
+ /**
+ * Destroy the upgrade protocol. Called before the parent HTTP/1.1 protocol
+ * is destroyed.
+ *
+ * @throws Exception If the upgrade protocol is not destroyed cleanly
+ */
+ public default void destroy() throws Exception {
// NO-OP
}
}
diff --git a/java/org/apache/coyote/http2/Http2Protocol.java b/java/org/apache/coyote/http2/Http2Protocol.java
index 78c0395..b61fe55 100644
--- a/java/org/apache/coyote/http2/Http2Protocol.java
+++ b/java/org/apache/coyote/http2/Http2Protocol.java
@@ -27,12 +27,15 @@ import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.regex.Pattern;
+import javax.management.ObjectName;
+
import org.apache.coyote.AbstractProtocol;
import org.apache.coyote.Adapter;
import org.apache.coyote.CompressionConfig;
import org.apache.coyote.ContinueResponseTiming;
import org.apache.coyote.Processor;
import org.apache.coyote.Request;
+import org.apache.coyote.RequestGroupInfo;
import org.apache.coyote.Response;
import org.apache.coyote.UpgradeProtocol;
import org.apache.coyote.UpgradeToken;
@@ -40,6 +43,7 @@ import org.apache.coyote.http11.AbstractHttp11Protocol;
import org.apache.coyote.http11.upgrade.InternalHttpUpgradeHandler;
import org.apache.coyote.http11.upgrade.UpgradeProcessorInternal;
import org.apache.tomcat.util.buf.StringUtils;
+import org.apache.tomcat.util.modeler.Registry;
import org.apache.tomcat.util.net.SocketWrapperBase;
public class Http2Protocol implements UpgradeProtocol {
@@ -95,7 +99,10 @@ public class Http2Protocol implements UpgradeProtocol {
// Compression
private final CompressionConfig compressionConfig = new CompressionConfig();
// Reference to HTTP/1.1 protocol that this instance is configured under
- private AbstractProtocol<?> http11Protocol = null;
+ private AbstractHttp11Protocol<?> http11Protocol = null;
+
+ private RequestGroupInfo global = new RequestGroupInfo();
+ private ObjectName rgOname = null;
@Override
public String getHttpUpgradeName(boolean isSSLEnabled) {
@@ -426,15 +433,50 @@ public class Http2Protocol implements UpgradeProtocol {
public ContinueResponseTiming getContinueResponseTimingInternal() {
- return ((AbstractHttp11Protocol<?>) http11Protocol).getContinueResponseTimingInternal();
+ return http11Protocol.getContinueResponseTimingInternal();
}
public AbstractProtocol<?> getHttp11Protocol() {
return this.http11Protocol;
}
+
+
@Override
- public void setHttp11Protocol(AbstractProtocol<?> http11Protocol) {
+ public void setHttp11Protocol(AbstractHttp11Protocol<?> http11Protocol) {
this.http11Protocol = http11Protocol;
}
+
+
+ public RequestGroupInfo getGlobal() {
+ return global;
+ }
+
+
+ @Override
+ public void init() throws Exception {
+ ObjectName parentRgOname = http11Protocol.getGlobalRequestProcessorMBeanName();
+ if (parentRgOname != null) {
+ StringBuilder name = new StringBuilder(parentRgOname.getCanonicalName());
+ name.append(",Upgrade=");
+ // Neither of these names need quoting
+ if (http11Protocol.isSSLEnabled()) {
+ name.append(ALPN_NAME);
+ } else {
+ name.append(HTTP_UPGRADE_NAME);
+ }
+ ObjectName rgOname = new ObjectName(name.toString());
+ this.rgOname = rgOname;
+ Registry.getRegistry(null, null).registerComponent(global, rgOname, null);
+ }
+ }
+
+
+ @Override
+ public void destroy() throws Exception {
+ ObjectName rgOname = this.rgOname;
+ if (rgOname != null) {
+ Registry.getRegistry(null, null).unregisterComponent(rgOname);
+ }
+ }
}
diff --git a/java/org/apache/coyote/http2/StreamProcessor.java b/java/org/apache/coyote/http2/StreamProcessor.java
index 94904c0..98c86cb 100644
--- a/java/org/apache/coyote/http2/StreamProcessor.java
+++ b/java/org/apache/coyote/http2/StreamProcessor.java
@@ -371,6 +371,12 @@ class StreamProcessor extends AbstractProcessor {
@Override
public final void recycle() {
// StreamProcessor instances are not re-used.
+
+ // Calling removeRequestProcessor even though the RequestProcesser was
+ // never added will add the values from the RequestProcessor to the
+ // running total for the GlobalRequestProcessor
+ handler.getProtocol().getGlobal().removeRequestProcessor(request.getRequestProcessor());
+
// Clear fields that can be cleared to aid GC and trigger NPEs if this
// is reused
setSocketWrapper(null);
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 2cc3d22..2d5388c 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -69,6 +69,10 @@
and the encoded solidus was preceeded by other %nn encoded characters.
Based on a pull request by willmeck. (markt)
</fix>
+ <fix>
+ Implement a partial fix for <bug>63362</bug> that adds collection of
+ request statistics for HTTP/2 requests. (markt)
+ </fix>
</changelog>
</subseciton>
<subsection name="Jasper">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org