You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by GitBox <gi...@apache.org> on 2021/05/19 14:51:45 UTC
[GitHub] [phoenix-queryserver] anmolnar opened a new pull request #65: PHOENIX-6473 Add Hadoop JMXServlet as /jmx endpoint
anmolnar opened a new pull request #65:
URL: https://github.com/apache/phoenix-queryserver/pull/65
Added new Avatica server customizer to create `/jmx` endpoint powered by JMXServlet. Also had to add jetty-servlet dependency and remove shading of jetty, otherwise the Servlet class cannot be instantiated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix-queryserver] anmolnar commented on pull request #65: PHOENIX-6473 Add Hadoop JMXServlet as /jmx endpoint
Posted by GitBox <gi...@apache.org>.
anmolnar commented on pull request #65:
URL: https://github.com/apache/phoenix-queryserver/pull/65#issuecomment-845802332
@stoty suggested to remove the entire relocations part which caused another runtime problem:
```
javax.servlet.UnavailableException: Servlet class org.apache.hadoop.jmx.JMXJsonServlet is not a javax.servlet.Servlet
at org.eclipse.jetty.servlet.ServletHolder.checkServletType(ServletHolder.java:555)
at org.eclipse.jetty.servlet.ServletHolder.doStart(ServletHolder.java:385)
at org.eclipse.jetty.util.component.AbstractLifeCycle.start(AbstractLifeCycle.java:68)
at org.eclipse.jetty.servlet.ServletHandler.lambda$initialize$0(ServletHandler.java:749)
```
I also wanted to add an integration test to validate the new endpoint, but I see the above error no matter how I tweak relocation. To be honest shading is completely jibbrish to me, so I've given up adding a test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix-queryserver] stoty commented on pull request #65: PHOENIX-6473 Add Hadoop JMXServlet as /jmx endpoint
Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #65:
URL: https://github.com/apache/phoenix-queryserver/pull/65#issuecomment-849680888
I tried to figure out if the Jmx servlet is authenticatd in Hadoop, but I got lost in the code.
Did you check if you can access Jmx without authentication in Hadoop ?
If yes, then it's fine, if not then, then we should also require authentication here.
Also, I think that a property to optionally disable the JMX server would be useful.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix-queryserver] anmolnar commented on a change in pull request #65: PHOENIX-6473 Add Hadoop JMXServlet as /jmx endpoint
Posted by GitBox <gi...@apache.org>.
anmolnar commented on a change in pull request #65:
URL: https://github.com/apache/phoenix-queryserver/pull/65#discussion_r636755496
##########
File path: phoenix-queryserver/src/main/java/org/apache/phoenix/queryserver/server/customizers/JMXJsonEndpointServerCustomizer.java
##########
@@ -0,0 +1,46 @@
+package org.apache.phoenix.queryserver.server.customizers;
+
+import static org.apache.hadoop.http.HttpServer2.CONF_CONTEXT_ATTRIBUTE;
+
+import org.apache.calcite.avatica.server.ServerCustomizer;
+import org.apache.hadoop.hbase.HBaseConfiguration;
+import org.apache.hadoop.jmx.JMXJsonServlet;
+import org.eclipse.jetty.server.Handler;
+import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.server.handler.HandlerList;
+import org.eclipse.jetty.servlet.ServletContextHandler;
+import org.eclipse.jetty.servlet.ServletHolder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Arrays;
+
+import javax.servlet.Servlet;
+
+public class JMXJsonEndpointServerCustomizer implements ServerCustomizer<Server> {
+ private static final Logger LOG = LoggerFactory.getLogger(JMXJsonEndpointServerCustomizer.class);
+
+ @Override
+ public void customize(Server server) {
+ Handler[] handlers = server.getHandlers();
+ if (handlers.length != 1) {
+ LOG.warn("Observed handlers on server {}", Arrays.toString(handlers));
+ throw new IllegalStateException("Expected to find one handler");
+ }
+ HandlerList list = (HandlerList) handlers[0];
+
+ ServletContextHandler ctx = new ServletContextHandler();
+ ctx.setContextPath("/jmx");
+ ctx.getServletContext().setAttribute(CONF_CONTEXT_ATTRIBUTE, HBaseConfiguration.create());
+
+ Servlet servlet = new JMXJsonServlet();
+ ServletHolder holder = new ServletHolder(servlet);
+ ctx.addServlet(holder, "/");
+
+ Handler[] realHandlers = list.getChildHandlers();
+ Handler[] newHandlers = new Handler[realHandlers.length + 1];
+ newHandlers[0] = ctx;
+ System.arraycopy(realHandlers, 0, newHandlers, 1, realHandlers.length);
+ server.setHandler(new HandlerList(newHandlers));
Review comment:
No, it doesn't need authentication (currently).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix-queryserver] joshelser commented on a change in pull request #65: PHOENIX-6473 Add Hadoop JMXServlet as /jmx endpoint
Posted by GitBox <gi...@apache.org>.
joshelser commented on a change in pull request #65:
URL: https://github.com/apache/phoenix-queryserver/pull/65#discussion_r636592386
##########
File path: phoenix-queryserver/src/main/java/org/apache/phoenix/queryserver/server/customizers/JMXJsonEndpointServerCustomizer.java
##########
@@ -0,0 +1,46 @@
+package org.apache.phoenix.queryserver.server.customizers;
+
+import static org.apache.hadoop.http.HttpServer2.CONF_CONTEXT_ATTRIBUTE;
+
+import org.apache.calcite.avatica.server.ServerCustomizer;
+import org.apache.hadoop.hbase.HBaseConfiguration;
+import org.apache.hadoop.jmx.JMXJsonServlet;
+import org.eclipse.jetty.server.Handler;
+import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.server.handler.HandlerList;
+import org.eclipse.jetty.servlet.ServletContextHandler;
+import org.eclipse.jetty.servlet.ServletHolder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Arrays;
+
+import javax.servlet.Servlet;
+
+public class JMXJsonEndpointServerCustomizer implements ServerCustomizer<Server> {
+ private static final Logger LOG = LoggerFactory.getLogger(JMXJsonEndpointServerCustomizer.class);
+
+ @Override
+ public void customize(Server server) {
+ Handler[] handlers = server.getHandlers();
+ if (handlers.length != 1) {
+ LOG.warn("Observed handlers on server {}", Arrays.toString(handlers));
+ throw new IllegalStateException("Expected to find one handler");
+ }
+ HandlerList list = (HandlerList) handlers[0];
+
+ ServletContextHandler ctx = new ServletContextHandler();
+ ctx.setContextPath("/jmx");
+ ctx.getServletContext().setAttribute(CONF_CONTEXT_ATTRIBUTE, HBaseConfiguration.create());
+
+ Servlet servlet = new JMXJsonServlet();
+ ServletHolder holder = new ServletHolder(servlet);
+ ctx.addServlet(holder, "/");
+
+ Handler[] realHandlers = list.getChildHandlers();
+ Handler[] newHandlers = new Handler[realHandlers.length + 1];
+ newHandlers[0] = ctx;
+ System.arraycopy(realHandlers, 0, newHandlers, 1, realHandlers.length);
+ server.setHandler(new HandlerList(newHandlers));
Review comment:
Does this end up requiring authentication to access it? I don't remember if we do authentication at the Server level or in the PQS Handler itself.
##########
File path: phoenix-queryserver/src/main/java/org/apache/phoenix/queryserver/server/customizers/JMXJsonEndpointServerCustomizer.java
##########
@@ -0,0 +1,46 @@
+package org.apache.phoenix.queryserver.server.customizers;
Review comment:
Needs a license header
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix-queryserver] anmolnar commented on pull request #65: PHOENIX-6473 Add Hadoop JMXServlet as /jmx endpoint
Posted by GitBox <gi...@apache.org>.
anmolnar commented on pull request #65:
URL: https://github.com/apache/phoenix-queryserver/pull/65#issuecomment-845204971
@stoty @richardantal PTAL.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix-queryserver] stoty closed pull request #65: PHOENIX-6473 Add Hadoop JMXServlet as /jmx endpoint
Posted by GitBox <gi...@apache.org>.
stoty closed pull request #65:
URL: https://github.com/apache/phoenix-queryserver/pull/65
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix-queryserver] joshelser commented on pull request #65: PHOENIX-6473 Add Hadoop JMXServlet as /jmx endpoint
Posted by GitBox <gi...@apache.org>.
joshelser commented on pull request #65:
URL: https://github.com/apache/phoenix-queryserver/pull/65#issuecomment-851521967
I forgot to reply last week, but I think this is fine to leave it unauthenticated. I can't think of anything harmful exposed via the servlet. Thanks for adding the option to disable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix-queryserver] anmolnar commented on pull request #65: PHOENIX-6473 Add Hadoop JMXServlet as /jmx endpoint
Posted by GitBox <gi...@apache.org>.
anmolnar commented on pull request #65:
URL: https://github.com/apache/phoenix-queryserver/pull/65#issuecomment-845800279
@joshelser This is the error that I see if I don't touch relocations:
```
21/05/20 17:31:58 ERROR server.QueryServer: Unrecoverable service error. Shutting down.
java.lang.NoClassDefFoundError: javax/servlet/Servlet
at org.apache.phoenix.queryserver.server.ServerCustomizersFactory$ServerCustomizersFactoryImpl.createServerCustomizers(ServerCustomizersFactory.java:75)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org