Jackie-Jiang commented on a change in pull request #6977: URL: https://github.com/apache/incubator-pinot/pull/6977#discussion_r641762916
########## File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/debug/TableDebugInfo.java ########## @@ -102,54 +113,93 @@ public int getNumBrokers() { return _brokerDebugInfos; } + @JsonPropertyOrder({"segmentName", "serverState"}) public static class SegmentDebugInfo { private final String _segmentName; - private final List<IsEvState> _states; - public SegmentDebugInfo(String name, List<IsEvState> states) { - _segmentName = name; - _states = states; + private final Map<String, SegmentState> _serverStateMap; + + @JsonCreator + public SegmentDebugInfo(@JsonProperty("segmentName") String segmentName, + @JsonProperty("serverState") Map<String, SegmentState> segmentServerState) { + _segmentName = segmentName; + _serverStateMap = segmentServerState; } public String getSegmentName() { return _segmentName; } - public List<IsEvState> getSegmentStateInServer() { - return _states; + public Map<String, SegmentState> getServerState() { + return _serverStateMap; } } - public static class IsEvState { - private final String _serverName; - private final String _idealStateStatus; - private final String _externalViewStatus; + /** + * This class represents the state of segment on the server: + * + * <ul> + * <li>Ideal State vs External view.</li> + * <li>Segment related errors and consumer information.</li> + * <li>Segment size.</li> + * </ul> + */ + @JsonIgnoreProperties(ignoreUnknown = true) + @JsonPropertyOrder({"idealState", "externalView", "segmentSize", "consumerInfo", "errorInfo"}) + public static class SegmentState { + private final String _idealState; + private final String _externalView; + private final String _segmentSize; + private final SegmentConsumerInfo _consumerInfo; + private final SegmentErrorInfo _errorInfo; + + @JsonCreator + public SegmentState(@JsonProperty("idealState") String idealState, + @JsonProperty("externalView") String externalView, @JsonProperty("segmentSize") String segmentSize, + @JsonProperty("consumerInfo") SegmentConsumerInfo consumerInfo, + @JsonProperty("errorInfo") SegmentErrorInfo errorInfo) { + _idealState = idealState; + _externalView = externalView; + _segmentSize = segmentSize; + _consumerInfo = consumerInfo; + _errorInfo = errorInfo; + } - public IsEvState(String name, String idealStateStatus, String externalViewStatus) { - _serverName = name; - _idealStateStatus = idealStateStatus; - _externalViewStatus = externalViewStatus; + public String getIdealState() { + return _idealState; } - public String getServerName() { - return _serverName; + public String getExternalView() { + return _externalView; } - public String getIdealStateStatus() { - return _idealStateStatus; + public SegmentConsumerInfo getConsumerInfo() { + return _consumerInfo; } - public String getExternalViewStatus() { - return _externalViewStatus; + public SegmentErrorInfo getErrorInfo() { + return _errorInfo; + } + + public String getSegmentSize() { + return _segmentSize; } } + /** + * Debug information related to Server. + */ + Review comment: (nit) remove empty line ########## File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableDebugResource.java ########## @@ -156,48 +180,102 @@ private TableDebugInfo debugTable(PinotHelixResourceManager pinotHelixResourceMa * * @param pinotHelixResourceManager Helix Resource Manager * @param tableNameWithType Name of table with type - * @return Debug information for segments + * @param verbosity Verbosity level to include debug information. For level 0, only segments + * with errors are included. For level > 0, all segments are included. * @return Debug information for segments */ private List<TableDebugInfo.SegmentDebugInfo> debugSegments(PinotHelixResourceManager pinotHelixResourceManager, - String tableNameWithType) { + String tableNameWithType, int verbosity) { ExternalView externalView = pinotHelixResourceManager.getTableExternalView(tableNameWithType); IdealState idealState = pinotHelixResourceManager.getTableIdealState(tableNameWithType); - List<TableDebugInfo.SegmentDebugInfo> segmentDebugInfos = new ArrayList<>(); + List<TableDebugInfo.SegmentDebugInfo> result = new ArrayList<>(); if (idealState == null) { - return segmentDebugInfos; + return result; } - for (Map.Entry<String, Map<String, String>> segmentMapEntry : idealState.getRecord().getMapFields().entrySet()) { - String segment = segmentMapEntry.getKey(); - Map<String, String> instanceStateMap = segmentMapEntry.getValue(); + int serverRequestTimeoutMs = _controllerConf.getServerAdminRequestTimeoutSeconds() * 1000; + final Map<String, List<String>> serverToSegments = + _pinotHelixResourceManager.getServerToSegmentsMap(tableNameWithType); - List<TableDebugInfo.IsEvState> isEvStates = new ArrayList<>(); - for (Map.Entry<String, String> entry : instanceStateMap.entrySet()) { - String serverName = entry.getKey(); - String isState = entry.getValue(); + BiMap<String, String> serverToEndpoints; + try { + serverToEndpoints = _pinotHelixResourceManager.getDataInstanceAdminEndpoints(serverToSegments.keySet()); + } catch (InvalidConfigException e) { + throw new WebApplicationException( + "Caught exception when getting segment debug info for table: " + tableNameWithType); + } + + Map<String, Map<String, SegmentServerDebugInfo>> segmentsDebugInfoFromServers = + getSegmentsDebugInfoFromServers(tableNameWithType, serverToEndpoints, serverRequestTimeoutMs); - String evState = (externalView != null) ? externalView.getStateMap(segment).get(serverName) : "null"; - if (!isState.equals(evState)) { - isEvStates.add(new TableDebugInfo.IsEvState(serverName, isState, evState)); + for (Map.Entry<String, Map<String, String>> segmentMapEntry : idealState.getRecord().getMapFields().entrySet()) { + String segmentName = segmentMapEntry.getKey(); + Map<String, String> segmentIsMap = segmentMapEntry.getValue(); + + Map<String, TableDebugInfo.SegmentState> segmentServerState = new HashMap<>(); + for (Map.Entry<String, Map<String, SegmentServerDebugInfo>> segmentEntry : segmentsDebugInfoFromServers + .entrySet()) { + String instanceName = segmentEntry.getKey(); + String isState = segmentIsMap.get(instanceName); + + Map<String, String> evStateMap = (externalView != null) ? externalView.getStateMap(segmentName) : null; + String evState = (evStateMap != null) ? evStateMap.get(instanceName) : null; + + if (evState != null) { + Map<String, SegmentServerDebugInfo> segmentServerDebugInfoMap = segmentEntry.getValue(); + SegmentServerDebugInfo segmentServerDebugInfo = segmentServerDebugInfoMap.get(segmentName); + + if ((verbosity > 0) || (segmentServerDebugInfo != null) && segmentHasErrors(segmentServerDebugInfo, + evState)) { Review comment: For readability ```suggestion if (verbosity > 0 || (segmentServerDebugInfo != null && segmentHasErrors(segmentServerDebugInfo, evState))) { ``` ########## File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentErrorInfo.java ########## @@ -0,0 +1,100 @@ +/** + * 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.pinot.common.restlet.resources; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonPropertyOrder; +import java.text.SimpleDateFormat; +import java.util.Date; +import java.util.Locale; +import java.util.TimeZone; +import org.apache.commons.lang3.exception.ExceptionUtils; + + +/** + * This class is used to represent errors related to a segment. + */ +@SuppressWarnings("unused") +@JsonIgnoreProperties(ignoreUnknown = true) +@JsonPropertyOrder({"timestamp", "errorMessage", "stackTrace"}) // For readability of JSON output +public class SegmentErrorInfo { + + private static final String DATE_FORMAT = "yyyy-MM-dd HH:mm:ss z"; + private static final SimpleDateFormat SIMPLE_DATE_FORMAT = new SimpleDateFormat(DATE_FORMAT, Locale.getDefault()); + + private final String _stackTrace; Review comment: (nit) Can we keep the order of `@JsonPropertyOrder({"timestamp", "errorMessage", "stackTrace"})` -- 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: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org