[
https://issues.apache.org/jira/browse/YARN-11897?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18060312#comment-18060312
]
ASF GitHub Bot commented on YARN-11897:
---------------------------------------
K0K0V0K commented on code in PR #8227:
URL: https://github.com/apache/hadoop/pull/8227#discussion_r2840342567
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/webapp/dao/NMResourceInfo.java:
##########
@@ -18,24 +18,15 @@
package org.apache.hadoop.yarn.server.nodemanager.webapp.dao;
+import
org.apache.hadoop.yarn.server.nodemanager.webapp.dao.gpu.NMGpuResourceInfo;
+
import javax.xml.bind.annotation.XmlAccessType;
import javax.xml.bind.annotation.XmlAccessorType;
import javax.xml.bind.annotation.XmlRootElement;
+import javax.xml.bind.annotation.XmlSeeAlso;
@XmlRootElement
@XmlAccessorType(XmlAccessType.FIELD)
+@XmlSeeAlso({NMGpuResourceInfo.class, NMDeviceResourceInfo.class})
public class NMResourceInfo {
-
- private long resourceValue;
Review Comment:
I suspect the original authors created this class to be able create custom
`ResourcePlugin` by
`yarn.nodemanager.pluggable-device-framework.device-classes`.
https://hadoop.apache.org/docs/stable/hadoop-yarn/hadoop-yarn-site/PluggableDeviceFramework.html
Now if we check the following example:
```
<property>
<name>yarn.nodemanager.pluggable-device-framework.device-classes</name>
<value>org.apache.hadoop.yarn.server.nodemanager.containermanager.resourceplugin.com.nvidia.NvidiaGPUPluginForRuntimeV2</value>
</property>
```
We can see some custom Nvidia device plugin can be configured on the
cluster, and than the NM /resources/{resourcename} api will return with
`NMDeviceResourceInfo`.
May i ask you to check is it possible to configure resourceplugin what can
return with different response than NMGpuResourceInfo.class and
NMDeviceResourceInfo.class in case of NM api?
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/webapp/JAXBContextResolver.java:
##########
@@ -18,22 +18,27 @@
package org.apache.hadoop.yarn.server.nodemanager.webapp;
+import java.util.Collections;
import java.util.Set;
import java.util.HashSet;
import java.util.Arrays;
-import org.glassfish.jersey.jettison.JettisonJaxbContext;
+
Review Comment:
Nit: this line can be deleted
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/webapp/jsonprovider/NMJsonProvider.java:
##########
@@ -0,0 +1,88 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.yarn.server.nodemanager.webapp.jsonprovider;
+
+import
org.apache.hadoop.yarn.server.nodemanager.webapp.dao.NMDeviceResourceInfo;
+import
org.apache.hadoop.yarn.server.nodemanager.webapp.dao.gpu.NMGpuResourceInfo;
+import org.apache.hadoop.yarn.server.webapp.dao.ContainerLogsInfoes;
+import org.eclipse.persistence.jaxb.MarshallerProperties;
+import org.eclipse.persistence.jaxb.rs.MOXyJsonProvider;
+
+import javax.ws.rs.Consumes;
+import javax.ws.rs.Produces;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.MultivaluedMap;
+import javax.ws.rs.ext.Provider;
+import javax.xml.bind.JAXBException;
+import javax.xml.bind.Marshaller;
+import javax.xml.bind.Unmarshaller;
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Type;
+
+/**
+ * MOXy JSON provider for NodeManager WebService.
+ *
+ * <p>This class configures a MOXy JSON provider for the NodeManager REST API
endpoints.
+ * The endpoints should be able to provide two types of JSON responses:</p>
+ * <ul>
+ * <li>
+ * <b>Wrapped classes</b> – classes whose JSON representation includes a
root wrapper element.
+ * </li>
+ * <li>
+ * <b>Unwrapped classes</b> – classes whose JSON representation omits a
root wrapper element.
+ * </li>
+ * </ul>
+ *
+ * <p>This behaviour can be configured by the
MarshallerProperties.JSON_INCLUDE_ROOT property.
+ *
+ * By default NodeManager REST API endpoints should include the root wrapper
element in the
+ * responses, however there are some exceptions (e.g. ContainerLogsInfoes
class) which
+ * was introduced to provide backward-compatibility with the Jersey 1 response
format.</p>
+ */
+@Provider
+@Produces(MediaType.APPLICATION_JSON)
+@Consumes(MediaType.APPLICATION_JSON)
+public class NMJsonProvider extends MOXyJsonProvider {
+
+ private boolean isRootElementNeeded(Class<?> type) {
+ return !type.equals(ContainerLogsInfoes.class)
Review Comment:
Seems a bit odd to me that these elements will have the root element in case
of JSON response, but not in the jaxb context.
May i ask to verify the xml api is backward compatible in case of
`http://nm-http-address:port/ws/v1/node/resources/{resourcename}`
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/webapp/TestNMWebServices.java:
##########
@@ -901,53 +929,51 @@ private static String getRedirectURL(String url) {
private List<ContainerLogsInfo> readEntity(Response response) throws
JSONException {
JSONObject jsonObject = response.readEntity(JSONObject.class);
- Iterator<String> keys = jsonObject.keys();
List<ContainerLogsInfo> list = new ArrayList<>();
- while (keys.hasNext()) {
- String key = keys.next();
- JSONObject subJsonObject = jsonObject.getJSONObject(key);
- Iterator<String> subKeys = subJsonObject.keys();
- while (subKeys.hasNext()) {
- String subKeyItem = subKeys.next();
- Object object = subJsonObject.get(subKeyItem);
-
- if (object instanceof JSONObject) {
- JSONObject subKeyItemValue = subJsonObject.getJSONObject(subKeyItem);
- ContainerLogsInfo containerLogsInfo =
parseContainerLogsInfo(subKeyItemValue);
- list.add(containerLogsInfo);
- }
+ Object containerLogsTypeInfo = jsonObject.get("containerLogsInfo");
- if(object instanceof JSONArray) {
- JSONArray jsonArray = subJsonObject.getJSONArray(subKeyItem);
- for (int i = 0; i < jsonArray.length(); i++) {
- JSONObject subKeyItemValue = jsonArray.getJSONObject(i);
- ContainerLogsInfo containerLogsInfo =
parseContainerLogsInfo(subKeyItemValue);
- list.add(containerLogsInfo);
- }
- }
+ if (containerLogsTypeInfo instanceof JSONArray) {
+ JSONArray containerLogsInfoArr = (JSONArray) containerLogsTypeInfo;
+ for (int i = 0; i < containerLogsInfoArr.length(); i++) {
+
list.add(parseContainerLogsInfo(containerLogsInfoArr.getJSONObject(i)));
}
+ } else if (containerLogsTypeInfo instanceof JSONObject) {
+ list.add(parseContainerLogsInfo((JSONObject) containerLogsTypeInfo));
}
return list;
}
- private ContainerLogsInfo parseContainerLogsInfo(JSONObject subKeyItemValue)
+ private ContainerLogsInfo parseContainerLogsInfo(JSONObject jsonLogsInfo)
throws JSONException {
- String logAggregationType =
subKeyItemValue.getString("logAggregationType");
- String containerId = subKeyItemValue.getString("containerId");
- String nodeId = subKeyItemValue.getString("nodeId");
-
- JSONObject containerLogInfo =
subKeyItemValue.getJSONObject("containerLogInfo");
- String fileName = containerLogInfo.getString("fileName");
- String fileSize = containerLogInfo.getString("fileSize");
- String lastModifiedTime = containerLogInfo.getString("lastModifiedTime");
+ String logAggregationType = jsonLogsInfo.getString("logAggregationType");
+ String containerId = jsonLogsInfo.getString("containerId");
+ String nodeId = jsonLogsInfo.getString("nodeId");
ContainerLogMeta containerLogMeta = new ContainerLogMeta(containerId,
nodeId);
- containerLogMeta.addLogMeta(fileName, fileSize, lastModifiedTime);
- ContainerLogsInfo containerLogsInfo =
- new ContainerLogsInfo(containerLogMeta, logAggregationType);
- return containerLogsInfo;
+
+ Object containerLogTypeInfo = jsonLogsInfo.get("containerLogInfo");
+ if (containerLogTypeInfo instanceof JSONArray) {
+ JSONArray containerLogInfoArr = (JSONArray) containerLogTypeInfo;
Review Comment:
Maybe you can add a test here that the array is at least 2 element long
> NodeManager REST API backward compatibility with Jersey1
> --------------------------------------------------------
>
> Key: YARN-11897
> URL: https://issues.apache.org/jira/browse/YARN-11897
> Project: Hadoop YARN
> Issue Type: Improvement
> Components: yarn
> Affects Versions: 3.5.0
> Reporter: Peter Szucs
> Assignee: chhinlinghean
> Priority: Major
> Labels: pull-request-available
>
> Jersey 2 upgrade broke NM REST API, so nodes applications page is not
> rendering in yarn UI v2 and also logs CLI cannot retrieve container logs.
> To fix this we need to:
> * introduce moxy in NM similar to YARN-11874
> * "/containers/\{containerid}/logs" endpoint returns a list, which will
> always be generated as a JSON array with Jersey 2. In Jersey 1 it returned a
> JSON object in case the list had a single element, and logs CLI expects a
> JSON object as well when calling this endpoint. That should be changed to
> keep backward compatibility. With returning a wrapper object instead of a
> list we could achieve the same behaviour.
> * Fix unit tests
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
