This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push:
new cc76b83 Partial fix for BZ 63362 provide h2 request statistics
cc76b83 is described below
commit cc76b8368c5f93154b5dc71cfbad464fff21586f
Author: Mark Thomas <[email protected]>
AuthorDate: Wed Jun 24 19:24:17 2020 +0100
Partial fix for BZ 63362 provide h2 request statistics
---
java/org/apache/coyote/AbstractProtocol.java | 44 +++++++++++++++++++----
java/org/apache/coyote/LocalStrings.properties | 1 +
java/org/apache/coyote/http2/Http2Protocol.java | 40 +++++++++++++++++++++
java/org/apache/coyote/http2/StreamProcessor.java | 6 ++++
webapps/docs/changelog.xml | 4 +++
5 files changed, 88 insertions(+), 7 deletions(-)
diff --git a/java/org/apache/coyote/AbstractProtocol.java
b/java/org/apache/coyote/AbstractProtocol.java
index bbb897a..dd1359b 100644
--- a/java/org/apache/coyote/AbstractProtocol.java
+++ b/java/org/apache/coyote/AbstractProtocol.java
@@ -35,6 +35,7 @@ import javax.management.ObjectName;
import javax.servlet.http.HttpUpgradeHandler;
import javax.servlet.http.WebConnection;
+import org.apache.coyote.http2.Http2Protocol;
import org.apache.juli.logging.Log;
import org.apache.tomcat.InstanceManager;
import org.apache.tomcat.util.ExceptionUtils;
@@ -63,12 +64,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.
@@ -140,6 +135,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.
*/
@@ -575,7 +578,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);
}
@@ -585,6 +589,17 @@ public abstract class AbstractProtocol<S> implements
ProtocolHandler,
endpoint.setDomain(domain);
endpoint.init();
+
+ UpgradeProtocol[] upgradeProtocols = findUpgradeProtocols();
+ for (UpgradeProtocol upgradeProtocol : upgradeProtocols) {
+ // Need to do this as we can't add methods to UpgradeProtocol
+ // without running the risk of breaking a custom upgrade protocol
+ if (upgradeProtocol instanceof Http2Protocol) {
+ // Implementation note: Failure of one upgrade protocol fails
the
+ // whole Connector
+ ((Http2Protocol) upgradeProtocol).init();
+ }
+ }
}
@@ -649,6 +664,20 @@ public abstract class AbstractProtocol<S> implements
ProtocolHandler,
getLog().info(sm.getString("abstractProtocolHandler.destroy",
getName()));
}
+ UpgradeProtocol[] upgradeProtocols = findUpgradeProtocols();
+ for (UpgradeProtocol upgradeProtocol : upgradeProtocols) {
+ try {
+ // Need to do this as we can't add methods to UpgradeProtocol
+ // without running the risk of breaking a custom upgrade
protocol
+ if (upgradeProtocol instanceof Http2Protocol) {
+ ((Http2Protocol) upgradeProtocol).destroy();
+ }
+ } catch (Throwable t) {
+ ExceptionUtils.handleThrowable(t);
+
getLog().error(sm.getString("abstractProtocol.upgradeProtocolDestroyError"), t);
+ }
+ }
+
try {
endpoint.destroy();
} finally {
@@ -666,6 +695,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 1aeb314..77911cc 100644
--- a/java/org/apache/coyote/LocalStrings.properties
+++ b/java/org/apache/coyote/LocalStrings.properties
@@ -35,6 +35,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/http2/Http2Protocol.java
b/java/org/apache/coyote/http2/Http2Protocol.java
index 50748a2..79f9314 100644
--- a/java/org/apache/coyote/http2/Http2Protocol.java
+++ b/java/org/apache/coyote/http2/Http2Protocol.java
@@ -27,11 +27,14 @@ import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.regex.Pattern;
+import javax.management.ObjectName;
+
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;
@@ -39,6 +42,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,6 +99,9 @@ public class Http2Protocol implements UpgradeProtocol {
// Reference to HTTP/1.1 protocol that this instance is configured under
private AbstractHttp11Protocol<?> http11Protocol = null;
+ private RequestGroupInfo global = new RequestGroupInfo();
+ private ObjectName rgOname = null;
+
@Override
public String getHttpUpgradeName(boolean isSSLEnabled) {
if (isSSLEnabled) {
@@ -423,7 +430,40 @@ public class Http2Protocol implements UpgradeProtocol {
public AbstractHttp11Protocol<?> getHttp11Protocol() {
return this.http11Protocol;
}
+
+
public void setHttp11Protocol(AbstractHttp11Protocol<?> http11Protocol) {
this.http11Protocol = http11Protocol;
}
+
+
+ public RequestGroupInfo getGlobal() {
+ return global;
+ }
+
+
+ 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);
+ }
+ }
+
+
+ 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 81ed660..119be9e 100644
--- a/java/org/apache/coyote/http2/StreamProcessor.java
+++ b/java/org/apache/coyote/http2/StreamProcessor.java
@@ -333,6 +333,12 @@ class StreamProcessor extends AbstractProcessor {
@Override
public 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 086d3f6..7b4f6a0 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -72,6 +72,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>
</section>
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
