You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2021/05/26 00:03:44 UTC
[GitHub] [incubator-pinot] mayankshriv opened a new pull request #6977: Enhance debug endpoint for table debugging.
mayankshriv opened a new pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977
Enhanced the existing endpoint for table deugging on controller as well as server
to include segment state transition and consumption related errors and information.
Note: This is a debug endpoint and should not be expected to maintain backward compatibility
from user's perspective. It would, however, ensure that debug calls from controller to
servers maintain compatibility.
- TableDataManager now caches exceptions caused during segment consumption or state transition.
Currently, only 100 exceptions are stored in each TableDataManager.
- Table debug endpoint on server now returns:
- Segment name and size
- Consumption info
- Error info
- Table debug endpoint on the controller returns aggregated segment info's across all server
in addition to existing broker and server info's.
- By default, only segments with issues are reported. To report all segments, an optional
parameter `verbose=true` can be specified in the api call.
Sample output:
```
[ {
"tableName" : "rk_REALTIME",
"numSegments" : 1,
"numServers" : 1,
"numBrokers" : 1,
"segmentDebugInfos" : [ {
"segmentName" : "rk__0__0__20210525T2349Z",
"serverState" : {
"Server_192.168.1.90_7000" : {
"segmentSize" : "0 bytes",
"consumerInfo" : {
"segmentName" : "rk__0__0__20210525T2349Z",
"consumerState" : "CONSUMING",
"lastConsumedTimestamp" : 0,
"partitionToOffsetMap" : {
"0" : "1"
}
},
"errorInfo" : {
"timeStamp" : "2021-05-25 16:49:34 PDT",
"error" : "Caught exception while transforming the record: {}",
"exception" : "java.lang.RuntimeException: Caught exception while transforming data type for column: current_ts\n\tat org.apache.pinot.segment.local.recordtransformer.DataTypeTransformer.transform(DataTypeTransformer.java:120)\n\tat org.apache.pinot.segment.local.recordtransformer.CompositeTransformer.transform(CompositeTransformer.java:82)\n\tat org.apache.pinot.core.data.manager.realtime.LLRealtimeSegmentDataManager.processStreamEvents(LLRealtimeSegmentDataManager.java:510)\n\tat org.apache.pinot.core.data.manager.realtime.LLRealtimeSegmentDataManager.consumeLoop(LLRealtimeSegmentDataManager.java:417)\n\tat org.apache.pinot.core.data.manager.realtime.LLRealtimeSegmentDataManager$PartitionConsumer.run(LLRealtimeSegmentDataManager.java:560)\n\tat java.lang.Thread.run(Thread.java:748)\nCaused by: java.lang.NumberFormatException: For input string: \"2021-05-12T08:13:16.152000\"\n\tat java.lang.NumberFormatException.forInputString(NumberFormatException.java:65)\n\tat java.l
ang.Long.parseLong(Long.java:589)\n\tat java.lang.Long.parseLong(Long.java:631)\n\tat org.apache.pinot.common.utils.PinotDataType$10.toLong(PinotDataType.java:502)\n\tat org.apache.pinot.common.utils.PinotDataType$6.convert(PinotDataType.java:333)\n\tat org.apache.pinot.common.utils.PinotDataType$6.convert(PinotDataType.java:290)\n\tat org.apache.pinot.segment.local.recordtransformer.DataTypeTransformer.transform(DataTypeTransformer.java:114)\n\t... 5 more\n"
},
"isState" : "CONSUMING",
"evState" : "CONSUMING"
}
}
} ],
"serverDebugInfos" : [ ],
"brokerDebugInfos" : [ ],
"tableSize" : {
"reportedSize" : "0 bytes",
"estimatedSize" : "0 bytes"
}
} ]
```
## Description
<!-- Add a description of your PR here.
A good description should include pointers to an issue or design document, etc.
-->
## Upgrade Notes
Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)
* [ ] Yes (Please label as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
Does this PR fix a zero-downtime upgrade introduced earlier?
* [ ] Yes (Please label this as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
Does this PR otherwise need attention when creating release notes? Things to consider:
- New configuration options
- Deprecation of configurations
- Signature changes to public methods/interfaces
- New plugins added or old plugins removed
* [ ] Yes (Please label this PR as **<code>release-notes</code>** and complete the section on Release Notes)
## Release Notes
<!-- If you have tagged this as either backward-incompat or release-notes,
you MUST add text here that you would like to see appear in release notes of the
next release. -->
<!-- If you have a series of commits adding or enabling a feature, then
add this section only in final commit that marks the feature completed.
Refer to earlier release notes to see examples of text.
-->
## Documentation
<!-- If you have introduced a new feature or configuration, please add it to the documentation as well.
See https://docs.pinot.apache.org/developers/developers-and-contributors/update-document
-->
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mayankshriv commented on a change in pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
mayankshriv commented on a change in pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#discussion_r640790441
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentServerDebugInfo.java
##########
@@ -0,0 +1,62 @@
+/**
+ * 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;
+
+
+/**
+ * This class represents the server-side debug information for a segment.
+ */
+@JsonIgnoreProperties(ignoreUnknown = true)
+public class SegmentServerDebugInfo {
Review comment:
Yes, this is to differentiate between server and controller side. I used `Segment` first because I viewed segment to be the primary identifier here.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mayankshriv commented on a change in pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
mayankshriv commented on a change in pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#discussion_r641189609
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/data/manager/offline/DimensionTableDataManager.java
##########
@@ -59,7 +59,7 @@ private DimensionTableDataManager() {
/**
* `createInstanceByTableName` should only be used by the {@link TableDataManagerProvider} and the returned
- * instance should be properly initialized via {@link #init} method before using.
+ * instance should be properly initialized via {@link org.apache.pinot.segment.local.data.manager.TableDataManager#init} method before using.
Review comment:
Seemed like IDE auto change.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mayankshriv commented on a change in pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
mayankshriv commented on a change in pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#discussion_r640796662
##########
File path: pinot-server/src/main/java/org/apache/pinot/server/api/resources/ServerResourceUtils.java
##########
@@ -0,0 +1,58 @@
+/**
+ * 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.server.api.resources;
+
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.Response;
+import org.apache.pinot.core.data.manager.InstanceDataManager;
+import org.apache.pinot.segment.local.data.manager.TableDataManager;
+import org.apache.pinot.server.starter.ServerInstance;
+
+
+/**
+ * Utility class for Server resources.
+ */
+public class ServerResourceUtils {
+
+ // Disable instantiation.
+ private ServerResourceUtils() {
+
+ }
+
+ public static TableDataManager checkGetTableDataManager(ServerInstance serverInstance, String tableName) {
+ InstanceDataManager dataManager = checkGetInstanceDataManager(serverInstance);
+ TableDataManager tableDataManager = dataManager.getTableDataManager(tableName);
+ if (tableDataManager == null) {
+ throw new WebApplicationException("Table " + tableName + " does not exist", Response.Status.NOT_FOUND);
+ }
+ return tableDataManager;
+ }
+
+ public static InstanceDataManager checkGetInstanceDataManager(ServerInstance serverInstance) {
Review comment:
Yeah, will leave it out of the scope of this PR, unsure of side-effects right now.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mayankshriv merged pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
mayankshriv merged pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] codecov-commenter edited a comment on pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#issuecomment-848396506
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6977?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#6977](https://codecov.io/gh/apache/incubator-pinot/pull/6977?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c394b74) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/0185482d9da2ac299b4b15bcd2998165ccbbdf71?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0185482) will **decrease** coverage by `7.83%`.
> The diff coverage is `57.05%`.
> :exclamation: Current head c394b74 differs from pull request most recent head bff31a6. Consider uploading reports for the commit bff31a6 to get more accurate results
[](https://codecov.io/gh/apache/incubator-pinot/pull/6977?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #6977 +/- ##
============================================
- Coverage 73.23% 65.40% -7.84%
Complexity 12 12
============================================
Files 1439 1445 +6
Lines 71333 71609 +276
Branches 10334 10367 +33
============================================
- Hits 52243 46835 -5408
- Misses 15578 21387 +5809
+ Partials 3512 3387 -125
```
| Flag | Coverage Δ | |
|---|---|---|
| integration | `?` | |
| unittests | `65.40% <57.05%> (+0.05%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6977?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...mmon/restlet/resources/SegmentServerDebugInfo.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVzdGxldC9yZXNvdXJjZXMvU2VnbWVudFNlcnZlckRlYnVnSW5mby5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...che/pinot/controller/api/debug/TableDebugInfo.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvZGVidWcvVGFibGVEZWJ1Z0luZm8uamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...ces/PinotSegmentUploadDownloadRestletResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90U2VnbWVudFVwbG9hZERvd25sb2FkUmVzdGxldFJlc291cmNlLmphdmE=) | `10.00% <0.00%> (-45.03%)` | :arrow_down: |
| [...t/controller/api/resources/TableDebugResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1RhYmxlRGVidWdSZXNvdXJjZS5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...ata/manager/offline/DimensionTableDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvb2ZmbGluZS9EaW1lbnNpb25UYWJsZURhdGFNYW5hZ2VyLmphdmE=) | `84.48% <ø> (ø)` | |
| [...ache/pinot/server/api/resources/DebugResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvYXBpL3Jlc291cmNlcy9EZWJ1Z1Jlc291cmNlLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...server/starter/helix/HelixInstanceDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvc3RhcnRlci9oZWxpeC9IZWxpeEluc3RhbmNlRGF0YU1hbmFnZXIuamF2YQ==) | `0.00% <0.00%> (-81.42%)` | :arrow_down: |
| [...r/helix/SegmentOnlineOfflineStateModelFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvc3RhcnRlci9oZWxpeC9TZWdtZW50T25saW5lT2ZmbGluZVN0YXRlTW9kZWxGYWN0b3J5LmphdmE=) | `0.00% <0.00%> (-65.00%)` | :arrow_down: |
| [...src/main/java/org/apache/pinot/spi/utils/Pair.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvUGFpci5qYXZh) | `38.88% <0.00%> (-24.75%)` | :arrow_down: |
| [.../pinot/core/data/manager/BaseTableDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvQmFzZVRhYmxlRGF0YU1hbmFnZXIuamF2YQ==) | `85.05% <14.28%> (-8.70%)` | :arrow_down: |
| ... and [360 more](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6977?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6977?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [0185482...bff31a6](https://codecov.io/gh/apache/incubator-pinot/pull/6977?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mayankshriv commented on a change in pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
mayankshriv commented on a change in pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#discussion_r640870146
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/debug/TableDebugInfo.java
##########
@@ -102,21 +113,71 @@ 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;
+ }
+ }
+
+ /**
+ * 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>
+ */
+ public static class SegmentState {
+ private final String _isState;
Review comment:
I see, ok.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#discussion_r641140241
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentErrorInfo.java
##########
@@ -0,0 +1,94 @@
+/**
+ * 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 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)
+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 _timestamp;
+ private final String _errorMessage;
+ private final String _exceptionStackTrace;
+
+ /**
+ *
+ * @param exceptionStackTrace Exception object
+ * @param errorMessage Error Message
+ * @param timestampMs Timestamp of the error
+ */
+ public SegmentErrorInfo(Exception exceptionStackTrace, String errorMessage, long timestampMs) {
+ _timestamp = epochToSDF(timestampMs);
+ _errorMessage = errorMessage;
+ _exceptionStackTrace = (exceptionStackTrace != null) ? ExceptionUtils.getStackTrace(exceptionStackTrace) : null;
+ }
+
+ /**
+ * This constructor is specifically for JSON ser/de.
+ *
+ * @param exceptionStackTrace Exception stack trace
+ * @param errorMessage Error message
+ * @param timestamp Time stamp of the error in Simple Date Format.
+ *
+ */
+ @JsonCreator
+ public SegmentErrorInfo(@JsonProperty("exception") String exceptionStackTrace,
+ @JsonProperty("error") String errorMessage, @JsonProperty("timeStamp") String timestamp) {
+ _timestamp = timestamp;
+ _errorMessage = errorMessage;
+ _exceptionStackTrace = exceptionStackTrace;
+ }
+
+ public String getTimestamp() {
+ return _timestamp;
+ }
+
+ public String getError() {
+ return _errorMessage;
+ }
+
+ public String getException() {
+ return _exceptionStackTrace;
+ }
Review comment:
Match the variable name
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentErrorInfo.java
##########
@@ -0,0 +1,94 @@
+/**
+ * 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 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)
+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 _timestamp;
+ private final String _errorMessage;
+ private final String _exceptionStackTrace;
+
+ /**
+ *
+ * @param exceptionStackTrace Exception object
+ * @param errorMessage Error Message
+ * @param timestampMs Timestamp of the error
+ */
+ public SegmentErrorInfo(Exception exceptionStackTrace, String errorMessage, long timestampMs) {
Review comment:
```suggestion
public SegmentErrorInfo(Exception exception, String errorMessage, long timestampMs) {
```
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentErrorInfo.java
##########
@@ -0,0 +1,94 @@
+/**
+ * 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 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)
+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 _timestamp;
+ private final String _errorMessage;
+ private final String _exceptionStackTrace;
+
+ /**
+ *
+ * @param exceptionStackTrace Exception object
+ * @param errorMessage Error Message
+ * @param timestampMs Timestamp of the error
+ */
+ public SegmentErrorInfo(Exception exceptionStackTrace, String errorMessage, long timestampMs) {
+ _timestamp = epochToSDF(timestampMs);
+ _errorMessage = errorMessage;
+ _exceptionStackTrace = (exceptionStackTrace != null) ? ExceptionUtils.getStackTrace(exceptionStackTrace) : null;
+ }
+
+ /**
+ * This constructor is specifically for JSON ser/de.
+ *
+ * @param exceptionStackTrace Exception stack trace
+ * @param errorMessage Error message
+ * @param timestamp Time stamp of the error in Simple Date Format.
+ *
+ */
+ @JsonCreator
+ public SegmentErrorInfo(@JsonProperty("exception") String exceptionStackTrace,
+ @JsonProperty("error") String errorMessage, @JsonProperty("timeStamp") String timestamp) {
+ _timestamp = timestamp;
+ _errorMessage = errorMessage;
+ _exceptionStackTrace = exceptionStackTrace;
+ }
+
+ public String getTimestamp() {
+ return _timestamp;
+ }
+
+ public String getError() {
+ return _errorMessage;
+ }
+
+ public String getException() {
+ return _exceptionStackTrace;
+ }
+
+ /**
+ * Utility function to convert epoch in millis to SDF of form "yyyy-MM-dd HH:mm:ss z".
+ *
+ * @param millisSinceEpoch Time in millis to convert
+ * @return SDF equivalent
+ */
+ private static String epochToSDF(long millisSinceEpoch) {
+ SIMPLE_DATE_FORMAT.setTimeZone(TimeZone.getDefault());
Review comment:
Put this line into a static block (only set once)
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java
##########
@@ -59,9 +65,14 @@
protected HelixManager _helixManager;
protected String _authToken;
+ // Fixed size LRU cache with InstanceName - SegmentName pair as key, and segment related
Review comment:
It is `TableName - SegmentName` pair
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableDebugResource.java
##########
@@ -156,38 +179,93 @@ private TableDebugInfo debugTable(PinotHelixResourceManager pinotHelixResourceMa
*
* @param pinotHelixResourceManager Helix Resource Manager
* @param tableNameWithType Name of table with type
+ * @param verbose If true, returns all segment info, else, only segments with errors
* @return Debug information for segments
*/
private List<TableDebugInfo.SegmentDebugInfo> debugSegments(PinotHelixResourceManager pinotHelixResourceManager,
- String tableNameWithType) {
+ String tableNameWithType, int verbose) {
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;
+ }
+
+ int serverRequestTimeoutMs = _controllerConf.getServerAdminRequestTimeoutSeconds() * 1000;
+ final Map<String, List<String>> serverToSegments =
+ _pinotHelixResourceManager.getServerToSegmentsMap(tableNameWithType);
+ Map<String, Map<String, SegmentServerDebugInfo>> segmentsDebugInfoFromServers;
+
+ 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);
}
+ segmentsDebugInfoFromServers =
+ getSegmentsDebugInfoFromServers(tableNameWithType, serverToEndpoints, serverRequestTimeoutMs);
+
for (Map.Entry<String, Map<String, String>> segmentMapEntry : idealState.getRecord().getMapFields().entrySet()) {
- String segment = segmentMapEntry.getKey();
- Map<String, String> instanceStateMap = segmentMapEntry.getValue();
+ 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();
- List<TableDebugInfo.IsEvState> isEvStates = new ArrayList<>();
- for (Map.Entry<String, String> entry : instanceStateMap.entrySet()) {
- String serverName = entry.getKey();
- String isState = entry.getValue();
+ String isState = segmentIsMap.get(instanceName);
+ String evState = (externalView != null) ? externalView.getStateMap(segmentName).get(instanceName) : "null";
Review comment:
(Critical) `externalView.getStateMap(segmentName)` could be `null`?
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/LLRealtimeSegmentDataManager.java
##########
@@ -85,7 +86,7 @@
import org.apache.pinot.spi.stream.StreamPartitionMsgOffset;
import org.apache.pinot.spi.stream.StreamPartitionMsgOffsetFactory;
import org.apache.pinot.spi.stream.TransientConsumerException;
-import org.apache.pinot.spi.utils.CommonConstants.ConsumerState;
+import org.apache.pinot.spi.utils.CommonConstants;
Review comment:
Any specific reason for this change?
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableDebugResource.java
##########
@@ -156,38 +179,93 @@ private TableDebugInfo debugTable(PinotHelixResourceManager pinotHelixResourceMa
*
* @param pinotHelixResourceManager Helix Resource Manager
* @param tableNameWithType Name of table with type
+ * @param verbose If true, returns all segment info, else, only segments with errors
* @return Debug information for segments
*/
private List<TableDebugInfo.SegmentDebugInfo> debugSegments(PinotHelixResourceManager pinotHelixResourceManager,
- String tableNameWithType) {
+ String tableNameWithType, int verbose) {
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;
+ }
+
+ int serverRequestTimeoutMs = _controllerConf.getServerAdminRequestTimeoutSeconds() * 1000;
+ final Map<String, List<String>> serverToSegments =
+ _pinotHelixResourceManager.getServerToSegmentsMap(tableNameWithType);
+ Map<String, Map<String, SegmentServerDebugInfo>> segmentsDebugInfoFromServers;
+
+ 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);
}
+ segmentsDebugInfoFromServers =
+ getSegmentsDebugInfoFromServers(tableNameWithType, serverToEndpoints, serverRequestTimeoutMs);
+
for (Map.Entry<String, Map<String, String>> segmentMapEntry : idealState.getRecord().getMapFields().entrySet()) {
- String segment = segmentMapEntry.getKey();
- Map<String, String> instanceStateMap = segmentMapEntry.getValue();
+ 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();
- List<TableDebugInfo.IsEvState> isEvStates = new ArrayList<>();
- for (Map.Entry<String, String> entry : instanceStateMap.entrySet()) {
- String serverName = entry.getKey();
- String isState = entry.getValue();
+ String isState = segmentIsMap.get(instanceName);
+ String evState = (externalView != null) ? externalView.getStateMap(segmentName).get(instanceName) : "null";
+ Map<String, SegmentServerDebugInfo> segmentServerDebugInfoMap = segmentEntry.getValue();
- String evState = (externalView != null) ? externalView.getStateMap(segment).get(serverName) : "null";
- if (!isState.equals(evState)) {
- isEvStates.add(new TableDebugInfo.IsEvState(serverName, isState, evState));
+ if ((verbose > 0) || segmentHasErrors(segmentServerDebugInfoMap, evState)) {
+ SegmentServerDebugInfo segmentServerDebugInfo = segmentServerDebugInfoMap.get(segmentName);
+
+ segmentServerState.put(instanceName,
+ new TableDebugInfo.SegmentState(isState, evState, segmentServerDebugInfo.getSegmentSize(),
+ segmentServerDebugInfo.getConsumerInfo(), segmentServerDebugInfo.getErrorInfo()));
}
}
- if (isEvStates.size() > 0) {
- segmentDebugInfos.add(new TableDebugInfo.SegmentDebugInfo(segment, isEvStates));
+ if (!segmentServerState.isEmpty()) {
+ result.add(new TableDebugInfo.SegmentDebugInfo(segmentName, segmentServerState));
}
}
- return segmentDebugInfos;
+
+ return result;
+ }
+
+ /**
+ * Helper method to check if a segment has any errors/issues.
+ *
+ * @param segmentServerDebugInfoMap Map from instanceName to segmentDebugInfo
+ * @param externalView external view of segment
+ * @return True if there's any error/issue for the segment, false otherwise.
+ */
+ private boolean segmentHasErrors(Map<String, SegmentServerDebugInfo> segmentServerDebugInfoMap, String externalView) {
+ // For now, we will skip cases where IS is ONLINE and EV is OFFLINE (or vice-versa), as it could happen during state transitions.
+ if (externalView.equals("ERROR")) {
+ return true;
+ }
+
+ for (Map.Entry<String, SegmentServerDebugInfo> entry : segmentServerDebugInfoMap.entrySet()) {
Review comment:
(Critical) this method is per-segment check, and we should only check the `SegmentServerDebugInfo` for this one single segment instead of checking all of them
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentErrorInfo.java
##########
@@ -0,0 +1,94 @@
+/**
+ * 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 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)
+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 _timestamp;
+ private final String _errorMessage;
+ private final String _exceptionStackTrace;
+
+ /**
+ *
+ * @param exceptionStackTrace Exception object
+ * @param errorMessage Error Message
+ * @param timestampMs Timestamp of the error
+ */
+ public SegmentErrorInfo(Exception exceptionStackTrace, String errorMessage, long timestampMs) {
Review comment:
(nit) change the parameter order to match the variable order
##########
File path: pinot-server/src/main/java/org/apache/pinot/server/api/resources/DebugResource.java
##########
@@ -0,0 +1,134 @@
+/**
+ * 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.server.api.resources;
+
+import io.swagger.annotations.Api;
+import io.swagger.annotations.ApiOperation;
+import io.swagger.annotations.ApiParam;
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import javax.inject.Inject;
+import javax.ws.rs.GET;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.MediaType;
+import org.apache.commons.io.FileUtils;
+import org.apache.pinot.common.restlet.resources.SegmentConsumerInfo;
+import org.apache.pinot.common.restlet.resources.SegmentErrorInfo;
+import org.apache.pinot.common.restlet.resources.SegmentServerDebugInfo;
+import org.apache.pinot.core.data.manager.offline.ImmutableSegmentDataManager;
+import org.apache.pinot.core.data.manager.realtime.RealtimeSegmentDataManager;
+import org.apache.pinot.segment.local.data.manager.SegmentDataManager;
+import org.apache.pinot.segment.local.data.manager.TableDataManager;
+import org.apache.pinot.segment.spi.ImmutableSegment;
+import org.apache.pinot.server.starter.ServerInstance;
+import org.apache.pinot.spi.config.table.TableType;
+import org.apache.pinot.spi.utils.builder.TableNameBuilder;
+
+
+/**
+ * Debug resource for Pinot Server.
+ */
+@Api(tags = "Debug")
+@Path("/debug/")
+public class DebugResource {
+
+ @Inject
+ private ServerInstance _serverInstance;
+
+ @GET
+ @Path("tables/{tableName}")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Get segments debug info for this table", notes = "This is a debug endpoint, and won't maintain backward compatibility")
+ public List<SegmentServerDebugInfo> getSegmentsDebugInfo(
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableNameWithType) {
+
+ TableType tableType = TableNameBuilder.getTableTypeFromTableName(tableNameWithType);
+ return getSegmentServerDebugInfo(tableNameWithType, tableType);
+ }
+
+ private List<SegmentServerDebugInfo> getSegmentServerDebugInfo(String tableNameWithType, TableType tableType) {
+ List<SegmentServerDebugInfo> segmentServerDebugInfos = new ArrayList<>();
+
+ TableDataManager tableDataManager =
+ ServerResourceUtils.checkGetTableDataManager(_serverInstance, tableNameWithType);
+
+ Map<String, SegmentErrorInfo> segmentErrorsMap = tableDataManager.getSegmentErrors();
+ Set<String> segmentsWithDataManagers = new HashSet<>();
+ List<SegmentDataManager> segmentDataManagers = tableDataManager.acquireAllSegments();
+ try {
+ for (SegmentDataManager segmentDataManager : segmentDataManagers) {
+ String segmentName = segmentDataManager.getSegmentName();
+ segmentsWithDataManagers.add(segmentName);
+
+ // Get segment consumer info.
+ SegmentConsumerInfo segmentConsumerInfo = getSegmentConsumerInfo(segmentDataManager, tableType);
+
+ // Get segment size.
+ long segmentSize = getSegmentSize(segmentDataManager);
+
+ // Get segment error.
+ SegmentErrorInfo segmentErrorInfo = segmentErrorsMap.get(segmentName);
+
+ segmentServerDebugInfos.add(new SegmentServerDebugInfo(segmentName, segmentConsumerInfo, segmentErrorInfo,
+ FileUtils.byteCountToDisplaySize(segmentSize)));
+ }
+ } catch (Exception e) {
+ throw new WebApplicationException("Caught exception when getting consumer info for table: " + tableNameWithType);
+ } finally {
+ for (SegmentDataManager segmentDataManager : segmentDataManagers) {
+ tableDataManager.releaseSegment(segmentDataManager);
+ }
+ }
+
+ // There may be segment errors for segments without Data Managers (e.g. segment wasn't loaded).
+ for (Map.Entry<String, SegmentErrorInfo> entry : segmentErrorsMap.entrySet()) {
+ String segmentName = entry.getKey();
+
+ if (!segmentsWithDataManagers.contains(segmentName)) {
+ SegmentErrorInfo segmentErrorInfo = entry.getValue();
+ segmentServerDebugInfos.add(new SegmentServerDebugInfo(segmentName, null, segmentErrorInfo, "-"));
+ }
+ }
+ return segmentServerDebugInfos;
+ }
+
+ private long getSegmentSize(SegmentDataManager segmentDataManager) {
+ return (segmentDataManager instanceof ImmutableSegmentDataManager) ? ((ImmutableSegment) segmentDataManager
+ .getSegment()).getSegmentSizeBytes() : 0;
+ }
+
+ private SegmentConsumerInfo getSegmentConsumerInfo(SegmentDataManager segmentDataManager, TableType tableType) {
+ SegmentConsumerInfo segmentConsumerInfo = null;
+ if (tableType.equals(TableType.REALTIME)) {
Review comment:
(nit) this can be `==`
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java
##########
@@ -59,9 +65,14 @@
protected HelixManager _helixManager;
protected String _authToken;
+ // Fixed size LRU cache with InstanceName - SegmentName pair as key, and segment related
+ // errors as the value.
+ protected LoadingCache<Pair<String, String>, SegmentErrorInfo> _errorCache;
+
@Override
public void init(TableDataManagerConfig tableDataManagerConfig, String instanceId,
- ZkHelixPropertyStore<ZNRecord> propertyStore, ServerMetrics serverMetrics, HelixManager helixManager) {
+ ZkHelixPropertyStore<ZNRecord> propertyStore, ServerMetrics serverMetrics, HelixManager helixManager,
+ LoadingCache<Pair<String, String>, SegmentErrorInfo> errorCache) {
Review comment:
nullable
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentErrorInfo.java
##########
@@ -0,0 +1,94 @@
+/**
+ * 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 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)
+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 _timestamp;
+ private final String _errorMessage;
+ private final String _exceptionStackTrace;
+
+ /**
+ *
+ * @param exceptionStackTrace Exception object
+ * @param errorMessage Error Message
+ * @param timestampMs Timestamp of the error
+ */
+ public SegmentErrorInfo(Exception exceptionStackTrace, String errorMessage, long timestampMs) {
+ _timestamp = epochToSDF(timestampMs);
+ _errorMessage = errorMessage;
+ _exceptionStackTrace = (exceptionStackTrace != null) ? ExceptionUtils.getStackTrace(exceptionStackTrace) : null;
+ }
+
+ /**
+ * This constructor is specifically for JSON ser/de.
+ *
+ * @param exceptionStackTrace Exception stack trace
+ * @param errorMessage Error message
+ * @param timestamp Time stamp of the error in Simple Date Format.
+ *
+ */
+ @JsonCreator
+ public SegmentErrorInfo(@JsonProperty("exception") String exceptionStackTrace,
Review comment:
Match the json property with the variable name? Also use the same order
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/data/manager/offline/DimensionTableDataManager.java
##########
@@ -59,7 +59,7 @@ private DimensionTableDataManager() {
/**
* `createInstanceByTableName` should only be used by the {@link TableDataManagerProvider} and the returned
- * instance should be properly initialized via {@link #init} method before using.
+ * instance should be properly initialized via {@link org.apache.pinot.segment.local.data.manager.TableDataManager#init} method before using.
Review comment:
I feel the existing doc is better
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableDebugResource.java
##########
@@ -82,9 +102,10 @@
@Produces(MediaType.APPLICATION_JSON)
@ApiOperation(value = "Get debug information for table.", notes = "Debug information for table.")
@ApiResponses(value = {@ApiResponse(code = 200, message = "Success"), @ApiResponse(code = 500, message = "Internal server error")})
- public String getClusterInfo(
+ public String getTableDebugInfo(
@ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
- @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr)
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr,
+ @ApiParam(value = "Verbosity of debug information") @DefaultValue("0") @QueryParam("verbosity") int verbose)
Review comment:
Rename the argument to `verbosity` as well since it's no longer a boolean. Same for other places in this file
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableDebugResource.java
##########
@@ -156,38 +179,93 @@ private TableDebugInfo debugTable(PinotHelixResourceManager pinotHelixResourceMa
*
* @param pinotHelixResourceManager Helix Resource Manager
* @param tableNameWithType Name of table with type
+ * @param verbose If true, returns all segment info, else, only segments with errors
* @return Debug information for segments
*/
private List<TableDebugInfo.SegmentDebugInfo> debugSegments(PinotHelixResourceManager pinotHelixResourceManager,
- String tableNameWithType) {
+ String tableNameWithType, int verbose) {
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;
+ }
+
+ int serverRequestTimeoutMs = _controllerConf.getServerAdminRequestTimeoutSeconds() * 1000;
+ final Map<String, List<String>> serverToSegments =
+ _pinotHelixResourceManager.getServerToSegmentsMap(tableNameWithType);
+ Map<String, Map<String, SegmentServerDebugInfo>> segmentsDebugInfoFromServers;
+
+ 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);
}
+ segmentsDebugInfoFromServers =
Review comment:
(nit) Put the declaration here
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableDebugResource.java
##########
@@ -156,38 +179,93 @@ private TableDebugInfo debugTable(PinotHelixResourceManager pinotHelixResourceMa
*
* @param pinotHelixResourceManager Helix Resource Manager
* @param tableNameWithType Name of table with type
+ * @param verbose If true, returns all segment info, else, only segments with errors
* @return Debug information for segments
*/
private List<TableDebugInfo.SegmentDebugInfo> debugSegments(PinotHelixResourceManager pinotHelixResourceManager,
- String tableNameWithType) {
+ String tableNameWithType, int verbose) {
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;
+ }
+
+ int serverRequestTimeoutMs = _controllerConf.getServerAdminRequestTimeoutSeconds() * 1000;
+ final Map<String, List<String>> serverToSegments =
+ _pinotHelixResourceManager.getServerToSegmentsMap(tableNameWithType);
+ Map<String, Map<String, SegmentServerDebugInfo>> segmentsDebugInfoFromServers;
+
+ 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);
}
+ segmentsDebugInfoFromServers =
+ getSegmentsDebugInfoFromServers(tableNameWithType, serverToEndpoints, serverRequestTimeoutMs);
+
for (Map.Entry<String, Map<String, String>> segmentMapEntry : idealState.getRecord().getMapFields().entrySet()) {
- String segment = segmentMapEntry.getKey();
- Map<String, String> instanceStateMap = segmentMapEntry.getValue();
+ 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();
- List<TableDebugInfo.IsEvState> isEvStates = new ArrayList<>();
- for (Map.Entry<String, String> entry : instanceStateMap.entrySet()) {
- String serverName = entry.getKey();
- String isState = entry.getValue();
+ String isState = segmentIsMap.get(instanceName);
+ String evState = (externalView != null) ? externalView.getStateMap(segmentName).get(instanceName) : "null";
+ Map<String, SegmentServerDebugInfo> segmentServerDebugInfoMap = segmentEntry.getValue();
- String evState = (externalView != null) ? externalView.getStateMap(segment).get(serverName) : "null";
- if (!isState.equals(evState)) {
- isEvStates.add(new TableDebugInfo.IsEvState(serverName, isState, evState));
+ if ((verbose > 0) || segmentHasErrors(segmentServerDebugInfoMap, evState)) {
+ SegmentServerDebugInfo segmentServerDebugInfo = segmentServerDebugInfoMap.get(segmentName);
+
+ segmentServerState.put(instanceName,
+ new TableDebugInfo.SegmentState(isState, evState, segmentServerDebugInfo.getSegmentSize(),
+ segmentServerDebugInfo.getConsumerInfo(), segmentServerDebugInfo.getErrorInfo()));
}
}
- if (isEvStates.size() > 0) {
- segmentDebugInfos.add(new TableDebugInfo.SegmentDebugInfo(segment, isEvStates));
+ if (!segmentServerState.isEmpty()) {
+ result.add(new TableDebugInfo.SegmentDebugInfo(segmentName, segmentServerState));
}
}
- return segmentDebugInfos;
+
+ return result;
+ }
+
+ /**
+ * Helper method to check if a segment has any errors/issues.
+ *
+ * @param segmentServerDebugInfoMap Map from instanceName to segmentDebugInfo
+ * @param externalView external view of segment
+ * @return True if there's any error/issue for the segment, false otherwise.
+ */
+ private boolean segmentHasErrors(Map<String, SegmentServerDebugInfo> segmentServerDebugInfoMap, String externalView) {
+ // For now, we will skip cases where IS is ONLINE and EV is OFFLINE (or vice-versa), as it could happen during state transitions.
+ if (externalView.equals("ERROR")) {
+ return true;
+ }
+
+ for (Map.Entry<String, SegmentServerDebugInfo> entry : segmentServerDebugInfoMap.entrySet()) {
+ SegmentServerDebugInfo debugInfo = entry.getValue();
+
+ SegmentConsumerInfo consumerInfo = debugInfo.getConsumerInfo();
+ if (consumerInfo != null && consumerInfo.getConsumerState()
+ .equals(CommonConstants.ConsumerState.NOT_CONSUMING.toString())) {
+ return true;
+ }
+
+ SegmentErrorInfo errorInfo = debugInfo.getErrorInfo();
Review comment:
Should we check error info? It won't be cleared even after segment is recovered
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mcvsubbu commented on a change in pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#discussion_r640988649
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java
##########
@@ -59,9 +65,14 @@
protected HelixManager _helixManager;
protected String _authToken;
+ // Fixed size LRU cache with InstanceName - SegmentName pair as key, and segment related
+ // errors as the value.
+ protected LoadingCache<Pair<String, String>, SegmentErrorInfo> _errorCache;
Review comment:
I think you mean single instance hosting multiple tables, in which case, the comment in line 68 should be "tableName - segmetName" pair as opposed to "instanceName - segmentName" pair?
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mayankshriv commented on a change in pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
mayankshriv commented on a change in pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#discussion_r641837401
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/debug/TableDebugInfo.java
##########
@@ -102,21 +113,71 @@ 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;
+ }
+ }
+
+ /**
+ * 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>
+ */
+ public static class SegmentState {
+ private final String _isState;
+ private final String _evState;
+ private final String _segmentSize;
+ private final SegmentConsumerInfo _consumerInfo;
+ private final SegmentErrorInfo _errorInfo;
Review comment:
Merging PR, we can followup in subsequent if there are further concerns.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mayankshriv commented on a change in pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
mayankshriv commented on a change in pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#discussion_r640881513
##########
File path: pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManagerConfig.java
##########
@@ -103,7 +103,11 @@
//
private static final String MAX_PARALLEL_REFRESH_THREADS = "max.parallel.refresh.threads";
+ // Size of cache that holds errors.
+ private static final String ERROR_CACHE_SIZE = "error.cache.size";
+
private final static String[] REQUIRED_KEYS = {INSTANCE_ID, INSTANCE_DATA_DIR, READ_MODE};
+ private static final long DEFAULT_ERROR_CACHE_SIZE = 100L;
Review comment:
Folks in OSS will then need to set this explicitly to be able to debug. I'd rather keep a safe default that does not cause any issues. I did a quick math, with 100 stack traces, we should still be well within a few MBs in the worst case.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mayankshriv commented on a change in pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
mayankshriv commented on a change in pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#discussion_r641734120
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableDebugResource.java
##########
@@ -156,38 +179,93 @@ private TableDebugInfo debugTable(PinotHelixResourceManager pinotHelixResourceMa
*
* @param pinotHelixResourceManager Helix Resource Manager
* @param tableNameWithType Name of table with type
+ * @param verbose If true, returns all segment info, else, only segments with errors
* @return Debug information for segments
*/
private List<TableDebugInfo.SegmentDebugInfo> debugSegments(PinotHelixResourceManager pinotHelixResourceManager,
- String tableNameWithType) {
+ String tableNameWithType, int verbose) {
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;
+ }
+
+ int serverRequestTimeoutMs = _controllerConf.getServerAdminRequestTimeoutSeconds() * 1000;
+ final Map<String, List<String>> serverToSegments =
+ _pinotHelixResourceManager.getServerToSegmentsMap(tableNameWithType);
+ Map<String, Map<String, SegmentServerDebugInfo>> segmentsDebugInfoFromServers;
+
+ 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);
}
+ segmentsDebugInfoFromServers =
+ getSegmentsDebugInfoFromServers(tableNameWithType, serverToEndpoints, serverRequestTimeoutMs);
+
for (Map.Entry<String, Map<String, String>> segmentMapEntry : idealState.getRecord().getMapFields().entrySet()) {
- String segment = segmentMapEntry.getKey();
- Map<String, String> instanceStateMap = segmentMapEntry.getValue();
+ 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();
- List<TableDebugInfo.IsEvState> isEvStates = new ArrayList<>();
- for (Map.Entry<String, String> entry : instanceStateMap.entrySet()) {
- String serverName = entry.getKey();
- String isState = entry.getValue();
+ String isState = segmentIsMap.get(instanceName);
+ String evState = (externalView != null) ? externalView.getStateMap(segmentName).get(instanceName) : "null";
+ Map<String, SegmentServerDebugInfo> segmentServerDebugInfoMap = segmentEntry.getValue();
- String evState = (externalView != null) ? externalView.getStateMap(segment).get(serverName) : "null";
- if (!isState.equals(evState)) {
- isEvStates.add(new TableDebugInfo.IsEvState(serverName, isState, evState));
+ if ((verbose > 0) || segmentHasErrors(segmentServerDebugInfoMap, evState)) {
+ SegmentServerDebugInfo segmentServerDebugInfo = segmentServerDebugInfoMap.get(segmentName);
+
+ segmentServerState.put(instanceName,
+ new TableDebugInfo.SegmentState(isState, evState, segmentServerDebugInfo.getSegmentSize(),
+ segmentServerDebugInfo.getConsumerInfo(), segmentServerDebugInfo.getErrorInfo()));
}
}
- if (isEvStates.size() > 0) {
- segmentDebugInfos.add(new TableDebugInfo.SegmentDebugInfo(segment, isEvStates));
+ if (!segmentServerState.isEmpty()) {
+ result.add(new TableDebugInfo.SegmentDebugInfo(segmentName, segmentServerState));
}
}
- return segmentDebugInfos;
+
+ return result;
+ }
+
+ /**
+ * Helper method to check if a segment has any errors/issues.
+ *
+ * @param segmentServerDebugInfoMap Map from instanceName to segmentDebugInfo
+ * @param externalView external view of segment
+ * @return True if there's any error/issue for the segment, false otherwise.
+ */
+ private boolean segmentHasErrors(Map<String, SegmentServerDebugInfo> segmentServerDebugInfoMap, String externalView) {
+ // For now, we will skip cases where IS is ONLINE and EV is OFFLINE (or vice-versa), as it could happen during state transitions.
+ if (externalView.equals("ERROR")) {
+ return true;
+ }
+
+ for (Map.Entry<String, SegmentServerDebugInfo> entry : segmentServerDebugInfoMap.entrySet()) {
+ SegmentServerDebugInfo debugInfo = entry.getValue();
+
+ SegmentConsumerInfo consumerInfo = debugInfo.getConsumerInfo();
+ if (consumerInfo != null && consumerInfo.getConsumerState()
+ .equals(CommonConstants.ConsumerState.NOT_CONSUMING.toString())) {
+ return true;
+ }
+
+ SegmentErrorInfo errorInfo = debugInfo.getErrorInfo();
Review comment:
The current semantic is that it is the last error associated with the segment. This is also why I added `timestamp` to tell how old the error is. Also, the state will confirm if it is online or offline. In future, we can add functionality to clear the error.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mayankshriv commented on a change in pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
mayankshriv commented on a change in pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#discussion_r640869225
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentServerDebugInfo.java
##########
@@ -0,0 +1,62 @@
+/**
+ * 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;
+
+
+/**
+ * This class represents the server-side debug information for a segment.
+ */
+@JsonIgnoreProperties(ignoreUnknown = true)
+public class SegmentServerDebugInfo {
+ private final String _segmentName;
+
+ private final SegmentConsumerInfo _consumerInfo;
+ private final SegmentErrorInfo _errorInfo;
+ private final String _segmentSize; // Segment Size in Human readable format
Review comment:
Yeah, I made it human readable, so instead of seeing a large numerical value, we will see something like 1.2GB.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#discussion_r641707816
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentErrorInfo.java
##########
@@ -0,0 +1,94 @@
+/**
+ * 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 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)
+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 _timestamp;
+ private final String _errorMessage;
+ private final String _exceptionStackTrace;
+
+ /**
+ *
+ * @param exceptionStackTrace Exception object
+ * @param errorMessage Error Message
+ * @param timestampMs Timestamp of the error
+ */
+ public SegmentErrorInfo(Exception exceptionStackTrace, String errorMessage, long timestampMs) {
Review comment:
This one is still the `Exception` instead of the stack trace. Once you write the stack trace as a string, then name it `exceptionStackTrace`
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mayankshriv commented on a change in pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
mayankshriv commented on a change in pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#discussion_r640870146
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/debug/TableDebugInfo.java
##########
@@ -102,21 +113,71 @@ 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;
+ }
+ }
+
+ /**
+ * 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>
+ */
+ public static class SegmentState {
+ private final String _isState;
Review comment:
I see, ok. I'll just calle them `_idealState` and `_externalView`.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] codecov-commenter commented on pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#issuecomment-848396506
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6977?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#6977](https://codecov.io/gh/apache/incubator-pinot/pull/6977?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (edab332) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/0185482d9da2ac299b4b15bcd2998165ccbbdf71?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0185482) will **decrease** coverage by `0.13%`.
> The diff coverage is `15.23%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/6977?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #6977 +/- ##
============================================
- Coverage 73.23% 73.10% -0.14%
Complexity 12 12
============================================
Files 1439 1443 +4
Lines 71333 71500 +167
Branches 10334 10348 +14
============================================
+ Hits 52243 52272 +29
- Misses 15578 15723 +145
+ Partials 3512 3505 -7
```
| Flag | Coverage Δ | |
|---|---|---|
| integration | `41.90% <4.28%> (+0.05%)` | :arrow_up: |
| unittests | `65.21% <15.23%> (-0.14%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6977?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...mmon/restlet/resources/SegmentServerDebugInfo.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVzdGxldC9yZXNvdXJjZXMvU2VnbWVudFNlcnZlckRlYnVnSW5mby5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...che/pinot/controller/api/debug/TableDebugInfo.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvZGVidWcvVGFibGVEZWJ1Z0luZm8uamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...t/controller/api/resources/TableDebugResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1RhYmxlRGVidWdSZXNvdXJjZS5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...ache/pinot/server/api/resources/DebugResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvYXBpL3Jlc291cmNlcy9EZWJ1Z1Jlc291cmNlLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...r/helix/SegmentOnlineOfflineStateModelFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvc3RhcnRlci9oZWxpeC9TZWdtZW50T25saW5lT2ZmbGluZVN0YXRlTW9kZWxGYWN0b3J5LmphdmE=) | `62.50% <0.00%> (-2.50%)` | :arrow_down: |
| [...manager/realtime/LLRealtimeSegmentDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvTExSZWFsdGltZVNlZ21lbnREYXRhTWFuYWdlci5qYXZh) | `72.86% <26.08%> (-0.20%)` | :arrow_down: |
| [.../pinot/core/data/manager/BaseTableDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvQmFzZVRhYmxlRGF0YU1hbmFnZXIuamF2YQ==) | `89.53% <33.33%> (-4.22%)` | :arrow_down: |
| [...not/common/restlet/resources/SegmentErrorInfo.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVzdGxldC9yZXNvdXJjZXMvU2VnbWVudEVycm9ySW5mby5qYXZh) | `52.94% <52.94%> (ø)` | |
| [...inot/server/api/resources/ServerResourceUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvYXBpL3Jlc291cmNlcy9TZXJ2ZXJSZXNvdXJjZVV0aWxzLmphdmE=) | `53.84% <53.84%> (ø)` | |
| [...che/pinot/server/api/resources/TablesResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvYXBpL3Jlc291cmNlcy9UYWJsZXNSZXNvdXJjZS5qYXZh) | `66.12% <80.00%> (+0.21%)` | :arrow_up: |
| ... and [33 more](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6977?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6977?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [0185482...edab332](https://codecov.io/gh/apache/incubator-pinot/pull/6977?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mayankshriv commented on a change in pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
mayankshriv commented on a change in pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#discussion_r641193715
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/LLRealtimeSegmentDataManager.java
##########
@@ -85,7 +86,7 @@
import org.apache.pinot.spi.stream.StreamPartitionMsgOffset;
import org.apache.pinot.spi.stream.StreamPartitionMsgOffsetFactory;
import org.apache.pinot.spi.stream.TransientConsumerException;
-import org.apache.pinot.spi.utils.CommonConstants.ConsumerState;
+import org.apache.pinot.spi.utils.CommonConstants;
Review comment:
No, seems like IDE auto-refactor, reverted.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#discussion_r640297369
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/debug/TableDebugInfo.java
##########
@@ -102,21 +113,71 @@ 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, SegmentServerState> _serverStateMap;
+
+ @JsonCreator
+ public SegmentDebugInfo(@JsonProperty("segmentName") String segmentName,
+ @JsonProperty("serverState") Map<String, SegmentServerState> segmentServerState) {
+ _segmentName = segmentName;
+ _serverStateMap = segmentServerState;
}
public String getSegmentName() {
return _segmentName;
}
- public List<IsEvState> getSegmentStateInServer() {
- return _states;
+ public Map<String, SegmentServerState> getServerState() {
+ return _serverStateMap;
+ }
+ }
+
+ /**
+ * 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>
+ */
+ public static class SegmentServerState {
Review comment:
Probably just `SegmentState` which is more concise?
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentErrorInfo.java
##########
@@ -0,0 +1,94 @@
+/**
+ * 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 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)
+public class SegmentErrorInfo {
+ private static final String DATE_FORMAT = "yyyy-MM-dd HH:mm:ss z";
+
+ private final String _timeStamp;
+ private final String _error;
+ private final String _exception;
+
+ /**
+ *
+ * @param exception Exception object
+ * @param errorMessage Error Message
+ * @param timeStampInMillisSinceEpoch Timestamp of the error
+ */
+ public SegmentErrorInfo(Exception exception, String errorMessage, long timeStampInMillisSinceEpoch) {
Review comment:
```suggestion
public SegmentErrorInfo(Exception exception, String errorMessage, long timestampMs) {
```
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentErrorInfo.java
##########
@@ -0,0 +1,94 @@
+/**
+ * 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 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)
+public class SegmentErrorInfo {
+ private static final String DATE_FORMAT = "yyyy-MM-dd HH:mm:ss z";
+
+ private final String _timeStamp;
Review comment:
We usually treat "timestamp" as a single word. Same for other places
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java
##########
@@ -228,4 +240,20 @@ public String getTableName() {
public File getTableDataDir() {
return _indexDir;
}
+
+ @Override
+ public void addSegmentError(String segmentName, SegmentErrorInfo segmentErrorInfo) {
+ _errorCache.put(new Pair<>(_tableNameWithType, segmentName), segmentErrorInfo);
+ }
+
+ @Override
+ public Map<String, SegmentErrorInfo> getSegmentErrors() {
+ if (_errorCache == null) {
+ return Collections.emptyMap();
+ } else {
+ // Filter out entries that match the table name.
+ return _errorCache.asMap().entrySet().stream().filter(map -> map.getKey().getFirst().equals(_tableNameWithType))
Review comment:
Avoid `stream` apis for performance concern
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentErrorInfo.java
##########
@@ -0,0 +1,94 @@
+/**
+ * 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 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)
+public class SegmentErrorInfo {
+ private static final String DATE_FORMAT = "yyyy-MM-dd HH:mm:ss z";
+
+ private final String _timeStamp;
+ private final String _error;
+ private final String _exception;
+
+ /**
+ *
+ * @param exception Exception object
+ * @param errorMessage Error Message
+ * @param timeStampInMillisSinceEpoch Timestamp of the error
+ */
+ public SegmentErrorInfo(Exception exception, String errorMessage, long timeStampInMillisSinceEpoch) {
+ _timeStamp = epochToSDF(timeStampInMillisSinceEpoch);
+ _error = errorMessage;
+ _exception = (exception != null) ? ExceptionUtils.getStackTrace(exception) : null;
+ }
+
+ /**
+ * This constructor is specifically for JSON ser/de.
+ *
+ * @param exception Exception stack trace
+ * @param error Error message
+ * @param timeStamp Time stamp of the error in Simple Date Format.
+ *
+ */
+ @JsonCreator
+ public SegmentErrorInfo(@JsonProperty("exception") String exception, @JsonProperty("error") String error,
+ @JsonProperty("timeStamp") String timeStamp) {
+ _timeStamp = timeStamp;
+ _error = error;
+ _exception = exception;
+ }
+
+ public String getTimeStamp() {
+ return _timeStamp;
+ }
+
+ public String getError() {
+ return _error;
+ }
+
+ public String getException() {
+ return _exception;
+ }
+
+ /**
+ * Utility function to convert epoch in millis to SDF of form "yyyy-MM-dd HH:mm:ss z".
+ *
+ * @param millisSinceEpoch Time in millis to convert
+ * @return SDF equivalent
+ */
+ private static String epochToSDF(long millisSinceEpoch) {
+ SimpleDateFormat sdf = new SimpleDateFormat(DATE_FORMAT, Locale.getDefault());
Review comment:
We should put the SDF as a constant without initializing the format for every call
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentErrorInfo.java
##########
@@ -0,0 +1,94 @@
+/**
+ * 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 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)
+public class SegmentErrorInfo {
+ private static final String DATE_FORMAT = "yyyy-MM-dd HH:mm:ss z";
+
+ private final String _timeStamp;
+ private final String _error;
+ private final String _exception;
Review comment:
`_stackTrace` for clarity?
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentErrorInfo.java
##########
@@ -0,0 +1,94 @@
+/**
+ * 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 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)
+public class SegmentErrorInfo {
+ private static final String DATE_FORMAT = "yyyy-MM-dd HH:mm:ss z";
+
+ private final String _timeStamp;
+ private final String _error;
Review comment:
`_errorMessage` for clarity?
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentServerDebugInfo.java
##########
@@ -0,0 +1,62 @@
+/**
+ * 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;
+
+
+/**
+ * This class represents the server-side debug information for a segment.
+ */
+@JsonIgnoreProperties(ignoreUnknown = true)
+public class SegmentServerDebugInfo {
Review comment:
I assume this name it picked to differentiate from the controller `SegmentDebugInfo`? Is `ServerSegmentDebugInfo` better?
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableDebugResource.java
##########
@@ -156,38 +177,94 @@ private TableDebugInfo debugTable(PinotHelixResourceManager pinotHelixResourceMa
*
* @param pinotHelixResourceManager Helix Resource Manager
* @param tableNameWithType Name of table with type
+ * @param verbose If true, returns all segment info, else, only segments with errors
* @return Debug information for segments
*/
private List<TableDebugInfo.SegmentDebugInfo> debugSegments(PinotHelixResourceManager pinotHelixResourceManager,
- String tableNameWithType) {
+ String tableNameWithType, boolean verbose) {
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;
+ }
+
+ int serverRequestTimeoutMs = _controllerConf.getServerAdminRequestTimeoutSeconds() * 1000;
+ final Map<String, List<String>> serverToSegments =
+ _pinotHelixResourceManager.getServerToSegmentsMap(tableNameWithType);
+ Map<String, Map<String, SegmentServerDebugInfo>> segmentsDebugInfoFromServers;
+
+ 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);
}
+ segmentsDebugInfoFromServers =
+ getSegmentsDebugInfoFromServers(tableNameWithType, serverToEndpoints, serverRequestTimeoutMs);
+
for (Map.Entry<String, Map<String, String>> segmentMapEntry : idealState.getRecord().getMapFields().entrySet()) {
- String segment = segmentMapEntry.getKey();
- Map<String, String> instanceStateMap = segmentMapEntry.getValue();
+ String segmentName = segmentMapEntry.getKey();
+ Map<String, String> segmentIsMap = segmentMapEntry.getValue();
+
+ Map<String, TableDebugInfo.SegmentServerState> segmentServerState = new HashMap<>();
+ for (Map.Entry<String, Map<String, SegmentServerDebugInfo>> segmentEntry : segmentsDebugInfoFromServers
+ .entrySet()) {
+ String instanceName = segmentEntry.getKey();
- List<TableDebugInfo.IsEvState> isEvStates = new ArrayList<>();
- for (Map.Entry<String, String> entry : instanceStateMap.entrySet()) {
- String serverName = entry.getKey();
- String isState = entry.getValue();
+ String isState = segmentIsMap.get(instanceName);
+ String evState = (externalView != null) ? externalView.getStateMap(segmentName).get(instanceName) : "null";
+ Map<String, SegmentServerDebugInfo> segmentServerDebugInfoMap = segmentEntry.getValue();
- String evState = (externalView != null) ? externalView.getStateMap(segment).get(serverName) : "null";
- if (!isState.equals(evState)) {
- isEvStates.add(new TableDebugInfo.IsEvState(serverName, isState, evState));
+ if (verbose || segmentHasErrors(segmentServerDebugInfoMap, isState, evState)) {
+ SegmentServerDebugInfo segmentServerDebugInfo = segmentServerDebugInfoMap.get(segmentName);
+
+ segmentServerState.put(instanceName,
+ new TableDebugInfo.SegmentServerState(isState, evState, segmentServerDebugInfo.getSegmentSize(),
+ segmentServerDebugInfo.getConsumerInfo(), segmentServerDebugInfo.getErrorInfo()));
}
}
- if (isEvStates.size() > 0) {
- segmentDebugInfos.add(new TableDebugInfo.SegmentDebugInfo(segment, isEvStates));
+ if (!segmentServerState.isEmpty()) {
+ result.add(new TableDebugInfo.SegmentDebugInfo(segmentName, segmentServerState));
}
}
- return segmentDebugInfos;
+
+ return result;
+ }
+
+ /**
+ * Helper method to check if a segment has any errors/issues.
+ *
+ * @param segmentServerDebugInfoMap Map from instanceName to segmentDebugInfo
+ * @param isState ideal state of segment
+ * @param evState external view of segment
+ * @return True if there's any error/issue for the segment, false otherwise.
+ */
+ private boolean segmentHasErrors(Map<String, SegmentServerDebugInfo> segmentServerDebugInfoMap, String isState,
+ String evState) {
+ if (!isState.equals(evState)) {
+ return true;
Review comment:
Should we count this as error? It is normal case during state transition
##########
File path: pinot-server/src/main/java/org/apache/pinot/server/api/resources/ServerResourceUtils.java
##########
@@ -0,0 +1,58 @@
+/**
+ * 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.server.api.resources;
+
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.Response;
+import org.apache.pinot.core.data.manager.InstanceDataManager;
+import org.apache.pinot.segment.local.data.manager.TableDataManager;
+import org.apache.pinot.server.starter.ServerInstance;
+
+
+/**
+ * Utility class for Server resources.
+ */
+public class ServerResourceUtils {
+
+ // Disable instantiation.
+ private ServerResourceUtils() {
+
+ }
+
+ public static TableDataManager checkGetTableDataManager(ServerInstance serverInstance, String tableName) {
+ InstanceDataManager dataManager = checkGetInstanceDataManager(serverInstance);
+ TableDataManager tableDataManager = dataManager.getTableDataManager(tableName);
+ if (tableDataManager == null) {
+ throw new WebApplicationException("Table " + tableName + " does not exist", Response.Status.NOT_FOUND);
+ }
+ return tableDataManager;
+ }
+
+ public static InstanceDataManager checkGetInstanceDataManager(ServerInstance serverInstance) {
Review comment:
(Maybe out of the scope of this PR, but since this PR refactors this) Seems we don't need these extra checks. When the rest API is up, both `serverInstance` and `instanceDataManager` will never be `null`
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mayankshriv commented on a change in pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
mayankshriv commented on a change in pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#discussion_r641184869
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentErrorInfo.java
##########
@@ -0,0 +1,94 @@
+/**
+ * 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 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)
+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 _timestamp;
+ private final String _errorMessage;
+ private final String _exceptionStackTrace;
+
+ /**
+ *
+ * @param exceptionStackTrace Exception object
+ * @param errorMessage Error Message
+ * @param timestampMs Timestamp of the error
+ */
+ public SegmentErrorInfo(Exception exceptionStackTrace, String errorMessage, long timestampMs) {
+ _timestamp = epochToSDF(timestampMs);
+ _errorMessage = errorMessage;
+ _exceptionStackTrace = (exceptionStackTrace != null) ? ExceptionUtils.getStackTrace(exceptionStackTrace) : null;
+ }
+
+ /**
+ * This constructor is specifically for JSON ser/de.
+ *
+ * @param exceptionStackTrace Exception stack trace
+ * @param errorMessage Error message
+ * @param timestamp Time stamp of the error in Simple Date Format.
+ *
+ */
+ @JsonCreator
+ public SegmentErrorInfo(@JsonProperty("exception") String exceptionStackTrace,
Review comment:
I took the liberty to use slightly different property name. My reasoning is that property name is for JSON output and should increase its readability (without being too verbose on names), and variable names should improve code readability.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mayankshriv commented on a change in pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
mayankshriv commented on a change in pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#discussion_r641206504
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/debug/TableDebugInfo.java
##########
@@ -102,21 +113,71 @@ 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;
+ }
+ }
+
+ /**
+ * 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>
+ */
+ public static class SegmentState {
+ private final String _isState;
+ private final String _evState;
+ private final String _segmentSize;
+ private final SegmentConsumerInfo _consumerInfo;
+ private final SegmentErrorInfo _errorInfo;
Review comment:
We will have to maintain the controller-server protocol regardless, and ensure that upgrades are not broken. The debug endpoints are meant for human debugging, and not to build any systems around them.
Specifically for SegmentErrorInfo, I was in double minds about having two separate classes, one for server side and another for controller side, but wasn't convinced. One question I have is would JSON annotations on the class be honored by rest, if so, it should be equivalent to passing JSON between server-controller?
Thoughts?
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mayankshriv commented on a change in pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
mayankshriv commented on a change in pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#discussion_r641183429
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentErrorInfo.java
##########
@@ -0,0 +1,94 @@
+/**
+ * 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 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)
+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 _timestamp;
+ private final String _errorMessage;
+ private final String _exceptionStackTrace;
+
+ /**
+ *
+ * @param exceptionStackTrace Exception object
+ * @param errorMessage Error Message
+ * @param timestampMs Timestamp of the error
+ */
+ public SegmentErrorInfo(Exception exceptionStackTrace, String errorMessage, long timestampMs) {
Review comment:
In previous review, you suggested to rename `exception` to `stackTrace`, I used `exceptionStackTrace`. Do you want to go back to `exception`? I feel using `stackTrace` or `exceptionStackTrace` is explicit.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mayankshriv commented on a change in pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
mayankshriv commented on a change in pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#discussion_r640880513
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java
##########
@@ -59,9 +65,14 @@
protected HelixManager _helixManager;
protected String _authToken;
+ // Fixed size LRU cache with InstanceName - SegmentName pair as key, and segment related
+ // errors as the value.
+ protected LoadingCache<Pair<String, String>, SegmentErrorInfo> _errorCache;
Review comment:
Good question. This is to avoid single table hosting multiple tables. The size of this cache is per instance, as opposed to per table (which could blow up in MT case).
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mayankshriv commented on a change in pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
mayankshriv commented on a change in pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#discussion_r640879901
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/debug/TableDebugInfo.java
##########
@@ -102,21 +113,71 @@ 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;
+ }
+ }
+
+ /**
+ * 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>
+ */
+ public static class SegmentState {
+ private final String _isState;
+ private final String _evState;
+ private final String _segmentSize;
+ private final SegmentConsumerInfo _consumerInfo;
+ private final SegmentErrorInfo _errorInfo;
Review comment:
Yes. Not sure if I follow the concern.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#discussion_r641813880
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/LLRealtimeSegmentDataManager.java
##########
@@ -520,8 +521,11 @@ private void processStreamEvents(MessageBatch messagesAndOffsets, long idlePipeS
}
}
} catch (Exception e) {
- segmentLogger.error("Caught exception while transforming the record: {}", decodedRow, e);
+ String errorMessage = String.format("Caught exception while transforming the record: {}", decodedRow);
Review comment:
```suggestion
String errorMessage = String.format("Caught exception while transforming the record: %s", decodedRow);
```
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/LLRealtimeSegmentDataManager.java
##########
@@ -801,7 +812,11 @@ protected SegmentBuildDescriptor buildSegmentInternal(boolean forCommit) {
try {
FileUtils.moveDirectory(tempIndexDir, indexDir);
} catch (IOException e) {
- segmentLogger.error("Caught exception while moving index directory from: {} to: {}", tempIndexDir, indexDir, e);
+ String errorMessage =
+ String.format("Caught exception while moving index directory from: % to: %s", tempIndexDir, indexDir);
Review comment:
```suggestion
String.format("Caught exception while moving index directory from: %s to: %s", tempIndexDir, indexDir);
```
##########
File path: pinot-server/src/main/java/org/apache/pinot/server/starter/helix/SegmentOnlineOfflineStateModelFactory.java
##########
@@ -118,6 +119,7 @@ public void onBecomeOnlineFromConsuming(Message message, NotificationContext con
segmentDataManager.goOnlineFromConsuming(metadata);
} catch (InterruptedException e) {
_logger.warn("State transition interrupted", e);
+ tableDataManager.addSegmentError(segmentNameStr, new SegmentErrorInfo(System.currentTimeMillis(), null, e));
Review comment:
Should we put some error message?
##########
File path: pinot-server/src/main/java/org/apache/pinot/server/starter/helix/SegmentOnlineOfflineStateModelFactory.java
##########
@@ -166,6 +168,10 @@ public void onBecomeOnlineFromOffline(Message message, NotificationContext conte
} catch (Exception e) {
_logger.error("Caught exception in state transition from OFFLINE -> ONLINE for resource: {}, partition: {}",
tableNameWithType, segmentName, e);
+ TableDataManager tableDataManager = _instanceDataManager.getTableDataManager(tableNameWithType);
+ if (tableDataManager != null) {
+ tableDataManager.addSegmentError(segmentName, new SegmentErrorInfo(System.currentTimeMillis(), null, e));
Review comment:
Same here
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mayankshriv commented on a change in pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
mayankshriv commented on a change in pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#discussion_r640928843
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableDebugResource.java
##########
@@ -156,38 +177,94 @@ private TableDebugInfo debugTable(PinotHelixResourceManager pinotHelixResourceMa
*
* @param pinotHelixResourceManager Helix Resource Manager
* @param tableNameWithType Name of table with type
+ * @param verbose If true, returns all segment info, else, only segments with errors
* @return Debug information for segments
*/
private List<TableDebugInfo.SegmentDebugInfo> debugSegments(PinotHelixResourceManager pinotHelixResourceManager,
- String tableNameWithType) {
+ String tableNameWithType, boolean verbose) {
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;
+ }
+
+ int serverRequestTimeoutMs = _controllerConf.getServerAdminRequestTimeoutSeconds() * 1000;
+ final Map<String, List<String>> serverToSegments =
+ _pinotHelixResourceManager.getServerToSegmentsMap(tableNameWithType);
+ Map<String, Map<String, SegmentServerDebugInfo>> segmentsDebugInfoFromServers;
+
+ 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);
}
+ segmentsDebugInfoFromServers =
+ getSegmentsDebugInfoFromServers(tableNameWithType, serverToEndpoints, serverRequestTimeoutMs);
+
for (Map.Entry<String, Map<String, String>> segmentMapEntry : idealState.getRecord().getMapFields().entrySet()) {
- String segment = segmentMapEntry.getKey();
- Map<String, String> instanceStateMap = segmentMapEntry.getValue();
+ String segmentName = segmentMapEntry.getKey();
+ Map<String, String> segmentIsMap = segmentMapEntry.getValue();
+
+ Map<String, TableDebugInfo.SegmentServerState> segmentServerState = new HashMap<>();
+ for (Map.Entry<String, Map<String, SegmentServerDebugInfo>> segmentEntry : segmentsDebugInfoFromServers
+ .entrySet()) {
+ String instanceName = segmentEntry.getKey();
- List<TableDebugInfo.IsEvState> isEvStates = new ArrayList<>();
- for (Map.Entry<String, String> entry : instanceStateMap.entrySet()) {
- String serverName = entry.getKey();
- String isState = entry.getValue();
+ String isState = segmentIsMap.get(instanceName);
+ String evState = (externalView != null) ? externalView.getStateMap(segmentName).get(instanceName) : "null";
+ Map<String, SegmentServerDebugInfo> segmentServerDebugInfoMap = segmentEntry.getValue();
- String evState = (externalView != null) ? externalView.getStateMap(segment).get(serverName) : "null";
- if (!isState.equals(evState)) {
- isEvStates.add(new TableDebugInfo.IsEvState(serverName, isState, evState));
+ if (verbose || segmentHasErrors(segmentServerDebugInfoMap, isState, evState)) {
+ SegmentServerDebugInfo segmentServerDebugInfo = segmentServerDebugInfoMap.get(segmentName);
+
+ segmentServerState.put(instanceName,
+ new TableDebugInfo.SegmentServerState(isState, evState, segmentServerDebugInfo.getSegmentSize(),
+ segmentServerDebugInfo.getConsumerInfo(), segmentServerDebugInfo.getErrorInfo()));
}
}
- if (isEvStates.size() > 0) {
- segmentDebugInfos.add(new TableDebugInfo.SegmentDebugInfo(segment, isEvStates));
+ if (!segmentServerState.isEmpty()) {
+ result.add(new TableDebugInfo.SegmentDebugInfo(segmentName, segmentServerState));
}
}
- return segmentDebugInfos;
+
+ return result;
+ }
+
+ /**
+ * Helper method to check if a segment has any errors/issues.
+ *
+ * @param segmentServerDebugInfoMap Map from instanceName to segmentDebugInfo
+ * @param isState ideal state of segment
+ * @param evState external view of segment
+ * @return True if there's any error/issue for the segment, false otherwise.
+ */
+ private boolean segmentHasErrors(Map<String, SegmentServerDebugInfo> segmentServerDebugInfoMap, String isState,
+ String evState) {
+ if (!isState.equals(evState)) {
+ return true;
Review comment:
👍
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mayankshriv commented on a change in pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
mayankshriv commented on a change in pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#discussion_r640791864
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java
##########
@@ -228,4 +240,20 @@ public String getTableName() {
public File getTableDataDir() {
return _indexDir;
}
+
+ @Override
+ public void addSegmentError(String segmentName, SegmentErrorInfo segmentErrorInfo) {
+ _errorCache.put(new Pair<>(_tableNameWithType, segmentName), segmentErrorInfo);
+ }
+
+ @Override
+ public Map<String, SegmentErrorInfo> getSegmentErrors() {
+ if (_errorCache == null) {
+ return Collections.emptyMap();
+ } else {
+ // Filter out entries that match the table name.
+ return _errorCache.asMap().entrySet().stream().filter(map -> map.getKey().getFirst().equals(_tableNameWithType))
Review comment:
In general I do, I took the liberty here because it is not in critical path of any performance tasks.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mcvsubbu commented on a change in pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#discussion_r640869998
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentServerDebugInfo.java
##########
@@ -0,0 +1,62 @@
+/**
+ * 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;
+
+
+/**
+ * This class represents the server-side debug information for a segment.
+ */
+@JsonIgnoreProperties(ignoreUnknown = true)
+public class SegmentServerDebugInfo {
+ private final String _segmentName;
+
+ private final SegmentConsumerInfo _consumerInfo;
+ private final SegmentErrorInfo _errorInfo;
+ private final String _segmentSize; // Segment Size in Human readable format
Review comment:
But this is a Pinot internal API, so we don't need to move strings around, right?
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] codecov-commenter edited a comment on pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#issuecomment-848396506
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6977?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#6977](https://codecov.io/gh/apache/incubator-pinot/pull/6977?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (edab332) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/0185482d9da2ac299b4b15bcd2998165ccbbdf71?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0185482) will **decrease** coverage by `0.07%`.
> The diff coverage is `15.23%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/6977?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #6977 +/- ##
============================================
- Coverage 73.23% 73.16% -0.08%
Complexity 12 12
============================================
Files 1439 1443 +4
Lines 71333 71500 +167
Branches 10334 10348 +14
============================================
+ Hits 52243 52310 +67
- Misses 15578 15689 +111
+ Partials 3512 3501 -11
```
| Flag | Coverage Δ | |
|---|---|---|
| integration | `42.01% <4.28%> (+0.16%)` | :arrow_up: |
| unittests | `65.21% <15.23%> (-0.14%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6977?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...mmon/restlet/resources/SegmentServerDebugInfo.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVzdGxldC9yZXNvdXJjZXMvU2VnbWVudFNlcnZlckRlYnVnSW5mby5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...che/pinot/controller/api/debug/TableDebugInfo.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvZGVidWcvVGFibGVEZWJ1Z0luZm8uamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...t/controller/api/resources/TableDebugResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1RhYmxlRGVidWdSZXNvdXJjZS5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...ache/pinot/server/api/resources/DebugResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvYXBpL3Jlc291cmNlcy9EZWJ1Z1Jlc291cmNlLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...r/helix/SegmentOnlineOfflineStateModelFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvc3RhcnRlci9oZWxpeC9TZWdtZW50T25saW5lT2ZmbGluZVN0YXRlTW9kZWxGYWN0b3J5LmphdmE=) | `62.50% <0.00%> (-2.50%)` | :arrow_down: |
| [...manager/realtime/LLRealtimeSegmentDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvTExSZWFsdGltZVNlZ21lbnREYXRhTWFuYWdlci5qYXZh) | `72.86% <26.08%> (-0.20%)` | :arrow_down: |
| [.../pinot/core/data/manager/BaseTableDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvQmFzZVRhYmxlRGF0YU1hbmFnZXIuamF2YQ==) | `89.53% <33.33%> (-4.22%)` | :arrow_down: |
| [...not/common/restlet/resources/SegmentErrorInfo.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVzdGxldC9yZXNvdXJjZXMvU2VnbWVudEVycm9ySW5mby5qYXZh) | `52.94% <52.94%> (ø)` | |
| [...inot/server/api/resources/ServerResourceUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvYXBpL3Jlc291cmNlcy9TZXJ2ZXJSZXNvdXJjZVV0aWxzLmphdmE=) | `53.84% <53.84%> (ø)` | |
| [...che/pinot/server/api/resources/TablesResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvYXBpL3Jlc291cmNlcy9UYWJsZXNSZXNvdXJjZS5qYXZh) | `66.12% <80.00%> (+0.21%)` | :arrow_up: |
| ... and [35 more](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6977?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6977?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [0185482...edab332](https://codecov.io/gh/apache/incubator-pinot/pull/6977?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] codecov-commenter edited a comment on pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#issuecomment-848396506
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6977?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#6977](https://codecov.io/gh/apache/incubator-pinot/pull/6977?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1da51f0) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/6388c77f52f781791fc018f3bfe4380c2d7a694d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6388c77) will **decrease** coverage by `7.90%`.
> The diff coverage is `13.90%`.
> :exclamation: Current head 1da51f0 differs from pull request most recent head 27fb44e. Consider uploading reports for the commit 27fb44e to get more accurate results
[](https://codecov.io/gh/apache/incubator-pinot/pull/6977?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #6977 +/- ##
============================================
- Coverage 73.31% 65.40% -7.91%
Complexity 12 12
============================================
Files 1441 1445 +4
Lines 71432 71610 +178
Branches 10351 10367 +16
============================================
- Hits 52369 46839 -5530
- Misses 15542 21386 +5844
+ Partials 3521 3385 -136
```
| Flag | Coverage Δ | |
|---|---|---|
| integration | `?` | |
| unittests | `65.40% <13.90%> (-0.13%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6977?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...mmon/restlet/resources/SegmentServerDebugInfo.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVzdGxldC9yZXNvdXJjZXMvU2VnbWVudFNlcnZlckRlYnVnSW5mby5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...che/pinot/controller/api/debug/TableDebugInfo.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvZGVidWcvVGFibGVEZWJ1Z0luZm8uamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...t/controller/api/resources/TableDebugResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1RhYmxlRGVidWdSZXNvdXJjZS5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...ata/manager/offline/DimensionTableDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvb2ZmbGluZS9EaW1lbnNpb25UYWJsZURhdGFNYW5hZ2VyLmphdmE=) | `84.48% <ø> (ø)` | |
| [...ache/pinot/server/api/resources/DebugResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvYXBpL3Jlc291cmNlcy9EZWJ1Z1Jlc291cmNlLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...server/starter/helix/HelixInstanceDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvc3RhcnRlci9oZWxpeC9IZWxpeEluc3RhbmNlRGF0YU1hbmFnZXIuamF2YQ==) | `0.00% <0.00%> (-81.42%)` | :arrow_down: |
| [.../starter/helix/HelixInstanceDataManagerConfig.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvc3RhcnRlci9oZWxpeC9IZWxpeEluc3RhbmNlRGF0YU1hbmFnZXJDb25maWcuamF2YQ==) | `0.00% <0.00%> (-79.60%)` | :arrow_down: |
| [...r/helix/SegmentOnlineOfflineStateModelFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvc3RhcnRlci9oZWxpeC9TZWdtZW50T25saW5lT2ZmbGluZVN0YXRlTW9kZWxGYWN0b3J5LmphdmE=) | `0.00% <0.00%> (-65.00%)` | :arrow_down: |
| [...src/main/java/org/apache/pinot/spi/utils/Pair.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvUGFpci5qYXZh) | `38.88% <0.00%> (-24.75%)` | :arrow_down: |
| [.../pinot/core/data/manager/BaseTableDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvQmFzZVRhYmxlRGF0YU1hbmFnZXIuamF2YQ==) | `85.05% <14.28%> (-8.70%)` | :arrow_down: |
| ... and [352 more](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6977?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6977?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [6388c77...27fb44e](https://codecov.io/gh/apache/incubator-pinot/pull/6977?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mayankshriv commented on a change in pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
mayankshriv commented on a change in pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#discussion_r640791087
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableDebugResource.java
##########
@@ -156,38 +177,94 @@ private TableDebugInfo debugTable(PinotHelixResourceManager pinotHelixResourceMa
*
* @param pinotHelixResourceManager Helix Resource Manager
* @param tableNameWithType Name of table with type
+ * @param verbose If true, returns all segment info, else, only segments with errors
* @return Debug information for segments
*/
private List<TableDebugInfo.SegmentDebugInfo> debugSegments(PinotHelixResourceManager pinotHelixResourceManager,
- String tableNameWithType) {
+ String tableNameWithType, boolean verbose) {
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;
+ }
+
+ int serverRequestTimeoutMs = _controllerConf.getServerAdminRequestTimeoutSeconds() * 1000;
+ final Map<String, List<String>> serverToSegments =
+ _pinotHelixResourceManager.getServerToSegmentsMap(tableNameWithType);
+ Map<String, Map<String, SegmentServerDebugInfo>> segmentsDebugInfoFromServers;
+
+ 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);
}
+ segmentsDebugInfoFromServers =
+ getSegmentsDebugInfoFromServers(tableNameWithType, serverToEndpoints, serverRequestTimeoutMs);
+
for (Map.Entry<String, Map<String, String>> segmentMapEntry : idealState.getRecord().getMapFields().entrySet()) {
- String segment = segmentMapEntry.getKey();
- Map<String, String> instanceStateMap = segmentMapEntry.getValue();
+ String segmentName = segmentMapEntry.getKey();
+ Map<String, String> segmentIsMap = segmentMapEntry.getValue();
+
+ Map<String, TableDebugInfo.SegmentServerState> segmentServerState = new HashMap<>();
+ for (Map.Entry<String, Map<String, SegmentServerDebugInfo>> segmentEntry : segmentsDebugInfoFromServers
+ .entrySet()) {
+ String instanceName = segmentEntry.getKey();
- List<TableDebugInfo.IsEvState> isEvStates = new ArrayList<>();
- for (Map.Entry<String, String> entry : instanceStateMap.entrySet()) {
- String serverName = entry.getKey();
- String isState = entry.getValue();
+ String isState = segmentIsMap.get(instanceName);
+ String evState = (externalView != null) ? externalView.getStateMap(segmentName).get(instanceName) : "null";
+ Map<String, SegmentServerDebugInfo> segmentServerDebugInfoMap = segmentEntry.getValue();
- String evState = (externalView != null) ? externalView.getStateMap(segment).get(serverName) : "null";
- if (!isState.equals(evState)) {
- isEvStates.add(new TableDebugInfo.IsEvState(serverName, isState, evState));
+ if (verbose || segmentHasErrors(segmentServerDebugInfoMap, isState, evState)) {
+ SegmentServerDebugInfo segmentServerDebugInfo = segmentServerDebugInfoMap.get(segmentName);
+
+ segmentServerState.put(instanceName,
+ new TableDebugInfo.SegmentServerState(isState, evState, segmentServerDebugInfo.getSegmentSize(),
+ segmentServerDebugInfo.getConsumerInfo(), segmentServerDebugInfo.getErrorInfo()));
}
}
- if (isEvStates.size() > 0) {
- segmentDebugInfos.add(new TableDebugInfo.SegmentDebugInfo(segment, isEvStates));
+ if (!segmentServerState.isEmpty()) {
+ result.add(new TableDebugInfo.SegmentDebugInfo(segmentName, segmentServerState));
}
}
- return segmentDebugInfos;
+
+ return result;
+ }
+
+ /**
+ * Helper method to check if a segment has any errors/issues.
+ *
+ * @param segmentServerDebugInfoMap Map from instanceName to segmentDebugInfo
+ * @param isState ideal state of segment
+ * @param evState external view of segment
+ * @return True if there's any error/issue for the segment, false otherwise.
+ */
+ private boolean segmentHasErrors(Map<String, SegmentServerDebugInfo> segmentServerDebugInfoMap, String isState,
+ String evState) {
+ if (!isState.equals(evState)) {
+ return true;
Review comment:
Suggestions on identifying whether it is during state transition or real error?
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mayankshriv commented on a change in pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
mayankshriv commented on a change in pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#discussion_r640874473
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableDebugResource.java
##########
@@ -63,6 +79,8 @@
@Api(tags = Constants.CLUSTER_TAG)
@Path("/")
public class TableDebugResource {
+ private static final Logger LOGGER = LoggerFactory.getLogger(TableDebugResource.class);
Review comment:
Will add a comment.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mayankshriv commented on a change in pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
mayankshriv commented on a change in pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#discussion_r641185385
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentErrorInfo.java
##########
@@ -0,0 +1,94 @@
+/**
+ * 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 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)
+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 _timestamp;
+ private final String _errorMessage;
+ private final String _exceptionStackTrace;
+
+ /**
+ *
+ * @param exceptionStackTrace Exception object
+ * @param errorMessage Error Message
+ * @param timestampMs Timestamp of the error
+ */
+ public SegmentErrorInfo(Exception exceptionStackTrace, String errorMessage, long timestampMs) {
+ _timestamp = epochToSDF(timestampMs);
+ _errorMessage = errorMessage;
+ _exceptionStackTrace = (exceptionStackTrace != null) ? ExceptionUtils.getStackTrace(exceptionStackTrace) : null;
+ }
+
+ /**
+ * This constructor is specifically for JSON ser/de.
+ *
+ * @param exceptionStackTrace Exception stack trace
+ * @param errorMessage Error message
+ * @param timestamp Time stamp of the error in Simple Date Format.
+ *
+ */
+ @JsonCreator
+ public SegmentErrorInfo(@JsonProperty("exception") String exceptionStackTrace,
+ @JsonProperty("error") String errorMessage, @JsonProperty("timeStamp") String timestamp) {
+ _timestamp = timestamp;
+ _errorMessage = errorMessage;
+ _exceptionStackTrace = exceptionStackTrace;
+ }
+
+ public String getTimestamp() {
+ return _timestamp;
+ }
+
+ public String getError() {
+ return _errorMessage;
+ }
+
+ public String getException() {
+ return _exceptionStackTrace;
+ }
Review comment:
Chose to do so because in the JSON output the field will show as `exceptionStackTrace` (a bit too long).
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#discussion_r640859147
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableDebugResource.java
##########
@@ -156,38 +177,94 @@ private TableDebugInfo debugTable(PinotHelixResourceManager pinotHelixResourceMa
*
* @param pinotHelixResourceManager Helix Resource Manager
* @param tableNameWithType Name of table with type
+ * @param verbose If true, returns all segment info, else, only segments with errors
* @return Debug information for segments
*/
private List<TableDebugInfo.SegmentDebugInfo> debugSegments(PinotHelixResourceManager pinotHelixResourceManager,
- String tableNameWithType) {
+ String tableNameWithType, boolean verbose) {
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;
+ }
+
+ int serverRequestTimeoutMs = _controllerConf.getServerAdminRequestTimeoutSeconds() * 1000;
+ final Map<String, List<String>> serverToSegments =
+ _pinotHelixResourceManager.getServerToSegmentsMap(tableNameWithType);
+ Map<String, Map<String, SegmentServerDebugInfo>> segmentsDebugInfoFromServers;
+
+ 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);
}
+ segmentsDebugInfoFromServers =
+ getSegmentsDebugInfoFromServers(tableNameWithType, serverToEndpoints, serverRequestTimeoutMs);
+
for (Map.Entry<String, Map<String, String>> segmentMapEntry : idealState.getRecord().getMapFields().entrySet()) {
- String segment = segmentMapEntry.getKey();
- Map<String, String> instanceStateMap = segmentMapEntry.getValue();
+ String segmentName = segmentMapEntry.getKey();
+ Map<String, String> segmentIsMap = segmentMapEntry.getValue();
+
+ Map<String, TableDebugInfo.SegmentServerState> segmentServerState = new HashMap<>();
+ for (Map.Entry<String, Map<String, SegmentServerDebugInfo>> segmentEntry : segmentsDebugInfoFromServers
+ .entrySet()) {
+ String instanceName = segmentEntry.getKey();
- List<TableDebugInfo.IsEvState> isEvStates = new ArrayList<>();
- for (Map.Entry<String, String> entry : instanceStateMap.entrySet()) {
- String serverName = entry.getKey();
- String isState = entry.getValue();
+ String isState = segmentIsMap.get(instanceName);
+ String evState = (externalView != null) ? externalView.getStateMap(segmentName).get(instanceName) : "null";
+ Map<String, SegmentServerDebugInfo> segmentServerDebugInfoMap = segmentEntry.getValue();
- String evState = (externalView != null) ? externalView.getStateMap(segment).get(serverName) : "null";
- if (!isState.equals(evState)) {
- isEvStates.add(new TableDebugInfo.IsEvState(serverName, isState, evState));
+ if (verbose || segmentHasErrors(segmentServerDebugInfoMap, isState, evState)) {
+ SegmentServerDebugInfo segmentServerDebugInfo = segmentServerDebugInfoMap.get(segmentName);
+
+ segmentServerState.put(instanceName,
+ new TableDebugInfo.SegmentServerState(isState, evState, segmentServerDebugInfo.getSegmentSize(),
+ segmentServerDebugInfo.getConsumerInfo(), segmentServerDebugInfo.getErrorInfo()));
}
}
- if (isEvStates.size() > 0) {
- segmentDebugInfos.add(new TableDebugInfo.SegmentDebugInfo(segment, isEvStates));
+ if (!segmentServerState.isEmpty()) {
+ result.add(new TableDebugInfo.SegmentDebugInfo(segmentName, segmentServerState));
}
}
- return segmentDebugInfos;
+
+ return result;
+ }
+
+ /**
+ * Helper method to check if a segment has any errors/issues.
+ *
+ * @param segmentServerDebugInfoMap Map from instanceName to segmentDebugInfo
+ * @param isState ideal state of segment
+ * @param evState external view of segment
+ * @return True if there's any error/issue for the segment, false otherwise.
+ */
+ private boolean segmentHasErrors(Map<String, SegmentServerDebugInfo> segmentServerDebugInfoMap, String isState,
+ String evState) {
+ if (!isState.equals(evState)) {
+ return true;
Review comment:
Check if `evState` is `ERROR`
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mcvsubbu commented on a change in pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#discussion_r640807315
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/debug/TableDebugInfo.java
##########
@@ -102,21 +113,71 @@ 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;
+ }
+ }
+
+ /**
+ * 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>
+ */
+ public static class SegmentState {
+ private final String _isState;
Review comment:
nit: maybe `_intendedState` and `_actualState`? `_isState` is kind of misleading to be a boolean :)
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableDebugResource.java
##########
@@ -156,38 +177,94 @@ private TableDebugInfo debugTable(PinotHelixResourceManager pinotHelixResourceMa
*
* @param pinotHelixResourceManager Helix Resource Manager
* @param tableNameWithType Name of table with type
+ * @param verbose If true, returns all segment info, else, only segments with errors
* @return Debug information for segments
*/
private List<TableDebugInfo.SegmentDebugInfo> debugSegments(PinotHelixResourceManager pinotHelixResourceManager,
- String tableNameWithType) {
+ String tableNameWithType, boolean verbose) {
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;
+ }
+
+ int serverRequestTimeoutMs = _controllerConf.getServerAdminRequestTimeoutSeconds() * 1000;
+ final Map<String, List<String>> serverToSegments =
+ _pinotHelixResourceManager.getServerToSegmentsMap(tableNameWithType);
+ Map<String, Map<String, SegmentServerDebugInfo>> segmentsDebugInfoFromServers;
+
+ 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);
}
+ segmentsDebugInfoFromServers =
+ getSegmentsDebugInfoFromServers(tableNameWithType, serverToEndpoints, serverRequestTimeoutMs);
+
for (Map.Entry<String, Map<String, String>> segmentMapEntry : idealState.getRecord().getMapFields().entrySet()) {
- String segment = segmentMapEntry.getKey();
- Map<String, String> instanceStateMap = segmentMapEntry.getValue();
+ String segmentName = segmentMapEntry.getKey();
+ Map<String, String> segmentIsMap = segmentMapEntry.getValue();
+
+ Map<String, TableDebugInfo.SegmentServerState> segmentServerState = new HashMap<>();
+ for (Map.Entry<String, Map<String, SegmentServerDebugInfo>> segmentEntry : segmentsDebugInfoFromServers
+ .entrySet()) {
+ String instanceName = segmentEntry.getKey();
- List<TableDebugInfo.IsEvState> isEvStates = new ArrayList<>();
- for (Map.Entry<String, String> entry : instanceStateMap.entrySet()) {
- String serverName = entry.getKey();
- String isState = entry.getValue();
+ String isState = segmentIsMap.get(instanceName);
+ String evState = (externalView != null) ? externalView.getStateMap(segmentName).get(instanceName) : "null";
+ Map<String, SegmentServerDebugInfo> segmentServerDebugInfoMap = segmentEntry.getValue();
- String evState = (externalView != null) ? externalView.getStateMap(segment).get(serverName) : "null";
- if (!isState.equals(evState)) {
- isEvStates.add(new TableDebugInfo.IsEvState(serverName, isState, evState));
+ if (verbose || segmentHasErrors(segmentServerDebugInfoMap, isState, evState)) {
+ SegmentServerDebugInfo segmentServerDebugInfo = segmentServerDebugInfoMap.get(segmentName);
+
+ segmentServerState.put(instanceName,
+ new TableDebugInfo.SegmentServerState(isState, evState, segmentServerDebugInfo.getSegmentSize(),
+ segmentServerDebugInfo.getConsumerInfo(), segmentServerDebugInfo.getErrorInfo()));
}
}
- if (isEvStates.size() > 0) {
- segmentDebugInfos.add(new TableDebugInfo.SegmentDebugInfo(segment, isEvStates));
+ if (!segmentServerState.isEmpty()) {
+ result.add(new TableDebugInfo.SegmentDebugInfo(segmentName, segmentServerState));
}
}
- return segmentDebugInfos;
+
+ return result;
+ }
+
+ /**
+ * Helper method to check if a segment has any errors/issues.
+ *
+ * @param segmentServerDebugInfoMap Map from instanceName to segmentDebugInfo
+ * @param isState ideal state of segment
+ * @param evState external view of segment
+ * @return True if there's any error/issue for the segment, false otherwise.
+ */
+ private boolean segmentHasErrors(Map<String, SegmentServerDebugInfo> segmentServerDebugInfoMap, String isState,
+ String evState) {
+ if (!isState.equals(evState)) {
+ return true;
Review comment:
The only way I know is the timestamp difference between IS and EV znodes.
Another thing we need to decide is if the state of `OFFLINE` in both IS and EV, is that counted as an ERROR ? My opinion has always been that it should not, so I am fine with line 249. But I have heard other voices that say that this is an error condition.
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentServerDebugInfo.java
##########
@@ -0,0 +1,62 @@
+/**
+ * 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;
+
+
+/**
+ * This class represents the server-side debug information for a segment.
+ */
+@JsonIgnoreProperties(ignoreUnknown = true)
+public class SegmentServerDebugInfo {
+ private final String _segmentName;
+
+ private final SegmentConsumerInfo _consumerInfo;
+ private final SegmentErrorInfo _errorInfo;
+ private final String _segmentSize; // Segment Size in Human readable format
Review comment:
Any reason this is a string? Server APIs are internal to pinot, and we can represent a long, right? (in that case, please add a unit to the name as in `_segmentSizeMB`)
##########
File path: pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManagerConfig.java
##########
@@ -103,7 +103,11 @@
//
private static final String MAX_PARALLEL_REFRESH_THREADS = "max.parallel.refresh.threads";
+ // Size of cache that holds errors.
+ private static final String ERROR_CACHE_SIZE = "error.cache.size";
+
private final static String[] REQUIRED_KEYS = {INSTANCE_ID, INSTANCE_DATA_DIR, READ_MODE};
+ private static final long DEFAULT_ERROR_CACHE_SIZE = 100L;
Review comment:
Can you make the default as 0 to preserve existing behavior?
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentServerDebugInfo.java
##########
@@ -0,0 +1,62 @@
+/**
+ * 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;
+
+
+/**
+ * This class represents the server-side debug information for a segment.
Review comment:
You may also want to add a comment that this class should not be used on the controller side user-facing interface. I am not sure if there is a way to annotate that. For now, a comment should suffice.
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableDebugResource.java
##########
@@ -253,4 +330,46 @@ private TableDebugInfo debugTable(PinotHelixResourceManager pinotHelixResourceMa
}
return serverDebugInfos;
}
+
+ /**
+ * This method makes a MultiGet call to all servers to get the segments debug info.
+ * @return Map of server to list of segment debug info's on the server.
+ */
+ private Map<String, Map<String, SegmentServerDebugInfo>> getSegmentsDebugInfoFromServers(String tableNameWithType,
+ BiMap<String, String> serverToEndpoints, int timeoutMs) {
+ LOGGER.info("Reading segments debug info from servers: {} for table: {}", serverToEndpoints.keySet(),
+ tableNameWithType);
+
+ List<String> serverUrls = new ArrayList<>(serverToEndpoints.size());
+ BiMap<String, String> endpointsToServers = serverToEndpoints.inverse();
+ for (String endpoint : endpointsToServers.keySet()) {
+ String segmentDebugInfoURI = String.format("%s/debug/tables/%s", endpoint, tableNameWithType);
+ serverUrls.add(segmentDebugInfoURI);
+ }
+
+ CompletionServiceHelper completionServiceHelper =
+ new CompletionServiceHelper(_executor, _connectionManager, endpointsToServers);
+ CompletionServiceHelper.CompletionServiceResponse serviceResponse =
+ completionServiceHelper.doMultiGetRequest(serverUrls, tableNameWithType, timeoutMs);
+
+ Map<String, Map<String, SegmentServerDebugInfo>> serverToSegmentDebugInfoList = new HashMap<>();
+ int failedParses = 0;
+ for (Map.Entry<String, String> streamResponse : serviceResponse._httpResponses.entrySet()) {
+ try {
+ List<SegmentServerDebugInfo> segmentDebugInfos =
+ JsonUtils.stringToObject(streamResponse.getValue(), new TypeReference<List<SegmentServerDebugInfo>>() {
+ });
+ Map<String, SegmentServerDebugInfo> segmentsMap = segmentDebugInfos.stream()
+ .collect(Collectors.toMap(SegmentServerDebugInfo::getSegmentName, Function.identity()));
+ serverToSegmentDebugInfoList.put(streamResponse.getKey(), segmentsMap);
+ } catch (IOException e) {
+ failedParses++;
+ LOGGER.error("Unable to parse server {} response due to an error: ", streamResponse.getKey(), e);
+ }
+ }
+ if (failedParses != 0) {
Review comment:
Return this as metadata so the user knows? The idea is that the administrator does not need to look at logs, right?
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableDebugResource.java
##########
@@ -63,6 +79,8 @@
@Api(tags = Constants.CLUSTER_TAG)
@Path("/")
public class TableDebugResource {
+ private static final Logger LOGGER = LoggerFactory.getLogger(TableDebugResource.class);
Review comment:
Perhaps add a comment at the header of this class that we don't intend to keep this backward compatible. Best is to annotate it somehow, but I am not sure how. We don't want people to use it in scripts, right? Alternatively, add a `/v1`/ in the uri path
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java
##########
@@ -59,9 +65,14 @@
protected HelixManager _helixManager;
protected String _authToken;
+ // Fixed size LRU cache with InstanceName - SegmentName pair as key, and segment related
+ // errors as the value.
+ protected LoadingCache<Pair<String, String>, SegmentErrorInfo> _errorCache;
Review comment:
Why do we need instance name in every key here? BaseTableDataManager lives on an instance, so this part of the key will be sam for every segment.
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/debug/TableDebugInfo.java
##########
@@ -102,21 +113,71 @@ 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;
+ }
+ }
+
+ /**
+ * 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>
+ */
+ public static class SegmentState {
+ private final String _isState;
+ private final String _evState;
+ private final String _segmentSize;
+ private final SegmentConsumerInfo _consumerInfo;
+ private final SegmentErrorInfo _errorInfo;
Review comment:
So, SegmentErrorInfo is propagated as it is to external world?
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableDebugResource.java
##########
@@ -82,9 +100,10 @@
@Produces(MediaType.APPLICATION_JSON)
@ApiOperation(value = "Get debug information for table.", notes = "Debug information for table.")
@ApiResponses(value = {@ApiResponse(code = 200, message = "Success"), @ApiResponse(code = 500, message = "Internal server error")})
- public String getClusterInfo(
+ public String getTableDebugInfo(
@ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
- @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr)
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr,
+ @ApiParam(value = "Get detailed information") @DefaultValue("false") @QueryParam("verbose") boolean verbose)
Review comment:
I suggest make this an integer of verbosity level, since we may have multiple levels? Level 0 can show only abnormal stuff. Otherwise just display "All good" or something like that. Level 1 may include error msgs, and Level 2 stack traces, and so on. Gives us room to play with.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mcvsubbu commented on a change in pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#discussion_r640988017
##########
File path: pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManagerConfig.java
##########
@@ -103,7 +103,11 @@
//
private static final String MAX_PARALLEL_REFRESH_THREADS = "max.parallel.refresh.threads";
+ // Size of cache that holds errors.
+ private static final String ERROR_CACHE_SIZE = "error.cache.size";
+
private final static String[] REQUIRED_KEYS = {INSTANCE_ID, INSTANCE_DATA_DIR, READ_MODE};
+ private static final long DEFAULT_ERROR_CACHE_SIZE = 100L;
Review comment:
ok, fair point. We will change at our end, if needed. And yes, the memory used will be relatively small, given it is a per instance cache
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mayankshriv commented on a change in pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
mayankshriv commented on a change in pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#discussion_r640883715
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableDebugResource.java
##########
@@ -253,4 +330,46 @@ private TableDebugInfo debugTable(PinotHelixResourceManager pinotHelixResourceMa
}
return serverDebugInfos;
}
+
+ /**
+ * This method makes a MultiGet call to all servers to get the segments debug info.
+ * @return Map of server to list of segment debug info's on the server.
+ */
+ private Map<String, Map<String, SegmentServerDebugInfo>> getSegmentsDebugInfoFromServers(String tableNameWithType,
+ BiMap<String, String> serverToEndpoints, int timeoutMs) {
+ LOGGER.info("Reading segments debug info from servers: {} for table: {}", serverToEndpoints.keySet(),
+ tableNameWithType);
+
+ List<String> serverUrls = new ArrayList<>(serverToEndpoints.size());
+ BiMap<String, String> endpointsToServers = serverToEndpoints.inverse();
+ for (String endpoint : endpointsToServers.keySet()) {
+ String segmentDebugInfoURI = String.format("%s/debug/tables/%s", endpoint, tableNameWithType);
+ serverUrls.add(segmentDebugInfoURI);
+ }
+
+ CompletionServiceHelper completionServiceHelper =
+ new CompletionServiceHelper(_executor, _connectionManager, endpointsToServers);
+ CompletionServiceHelper.CompletionServiceResponse serviceResponse =
+ completionServiceHelper.doMultiGetRequest(serverUrls, tableNameWithType, timeoutMs);
+
+ Map<String, Map<String, SegmentServerDebugInfo>> serverToSegmentDebugInfoList = new HashMap<>();
+ int failedParses = 0;
+ for (Map.Entry<String, String> streamResponse : serviceResponse._httpResponses.entrySet()) {
+ try {
+ List<SegmentServerDebugInfo> segmentDebugInfos =
+ JsonUtils.stringToObject(streamResponse.getValue(), new TypeReference<List<SegmentServerDebugInfo>>() {
+ });
+ Map<String, SegmentServerDebugInfo> segmentsMap = segmentDebugInfos.stream()
+ .collect(Collectors.toMap(SegmentServerDebugInfo::getSegmentName, Function.identity()));
+ serverToSegmentDebugInfoList.put(streamResponse.getKey(), segmentsMap);
+ } catch (IOException e) {
+ failedParses++;
+ LOGGER.error("Unable to parse server {} response due to an error: ", streamResponse.getKey(), e);
+ }
+ }
+ if (failedParses != 0) {
Review comment:
Sounds like a good idea. However, this pattern (of getting info from servers and aggregating) is repeated in multiple places (eg table size), I'll think about consolidating and cleaning in a separate PR.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] codecov-commenter edited a comment on pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#issuecomment-848396506
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6977?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#6977](https://codecov.io/gh/apache/incubator-pinot/pull/6977?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (90db52c) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/6388c77f52f781791fc018f3bfe4380c2d7a694d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6388c77) will **decrease** coverage by `7.94%`.
> The diff coverage is `15.29%`.
> :exclamation: Current head 90db52c differs from pull request most recent head 0b11b8a. Consider uploading reports for the commit 0b11b8a to get more accurate results
[](https://codecov.io/gh/apache/incubator-pinot/pull/6977?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #6977 +/- ##
============================================
- Coverage 73.31% 65.36% -7.95%
Complexity 12 12
============================================
Files 1441 1445 +4
Lines 71432 71628 +196
Branches 10351 10370 +19
============================================
- Hits 52369 46823 -5546
- Misses 15542 21410 +5868
+ Partials 3521 3395 -126
```
| Flag | Coverage Δ | |
|---|---|---|
| integration | `?` | |
| unittests | `65.36% <15.29%> (-0.17%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6977?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...mmon/restlet/resources/SegmentServerDebugInfo.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVzdGxldC9yZXNvdXJjZXMvU2VnbWVudFNlcnZlckRlYnVnSW5mby5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...che/pinot/controller/api/debug/TableDebugInfo.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvZGVidWcvVGFibGVEZWJ1Z0luZm8uamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...t/controller/api/resources/TableDebugResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1RhYmxlRGVidWdSZXNvdXJjZS5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...x/core/minion/generator/TaskGeneratorRegistry.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL21pbmlvbi9nZW5lcmF0b3IvVGFza0dlbmVyYXRvclJlZ2lzdHJ5LmphdmE=) | `76.00% <ø> (-4.00%)` | :arrow_down: |
| [...ata/manager/offline/DimensionTableDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvb2ZmbGluZS9EaW1lbnNpb25UYWJsZURhdGFNYW5hZ2VyLmphdmE=) | `84.48% <ø> (ø)` | |
| [...not/minion/event/EventObserverFactoryRegistry.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vZXZlbnQvRXZlbnRPYnNlcnZlckZhY3RvcnlSZWdpc3RyeS5qYXZh) | `0.00% <0.00%> (-56.53%)` | :arrow_down: |
| [...ache/pinot/server/api/resources/DebugResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvYXBpL3Jlc291cmNlcy9EZWJ1Z1Jlc291cmNlLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...server/starter/helix/HelixInstanceDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvc3RhcnRlci9oZWxpeC9IZWxpeEluc3RhbmNlRGF0YU1hbmFnZXIuamF2YQ==) | `0.00% <0.00%> (-81.42%)` | :arrow_down: |
| [.../starter/helix/HelixInstanceDataManagerConfig.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvc3RhcnRlci9oZWxpeC9IZWxpeEluc3RhbmNlRGF0YU1hbmFnZXJDb25maWcuamF2YQ==) | `0.00% <0.00%> (-79.60%)` | :arrow_down: |
| [...r/helix/SegmentOnlineOfflineStateModelFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvc3RhcnRlci9oZWxpeC9TZWdtZW50T25saW5lT2ZmbGluZVN0YXRlTW9kZWxGYWN0b3J5LmphdmE=) | `0.00% <0.00%> (-65.00%)` | :arrow_down: |
| ... and [358 more](https://codecov.io/gh/apache/incubator-pinot/pull/6977/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6977?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6977?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [6388c77...0b11b8a](https://codecov.io/gh/apache/incubator-pinot/pull/6977?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mcvsubbu commented on a change in pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#discussion_r640983000
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/debug/TableDebugInfo.java
##########
@@ -102,21 +113,71 @@ 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;
+ }
+ }
+
+ /**
+ * 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>
+ */
+ public static class SegmentState {
+ private final String _isState;
+ private final String _evState;
+ private final String _segmentSize;
+ private final SegmentConsumerInfo _consumerInfo;
+ private final SegmentErrorInfo _errorInfo;
Review comment:
The concern here is that it is also used in the server-controller protocol. So, it may come to play during upgrades, esp if there are automated systems issuing monitoring commands.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mayankshriv commented on a change in pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
mayankshriv commented on a change in pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#discussion_r640869225
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentServerDebugInfo.java
##########
@@ -0,0 +1,62 @@
+/**
+ * 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;
+
+
+/**
+ * This class represents the server-side debug information for a segment.
+ */
+@JsonIgnoreProperties(ignoreUnknown = true)
+public class SegmentServerDebugInfo {
+ private final String _segmentName;
+
+ private final SegmentConsumerInfo _consumerInfo;
+ private final SegmentErrorInfo _errorInfo;
+ private final String _segmentSize; // Segment Size in Human readable format
Review comment:
Yeah, I made it human readable, so instead of seeing a large numerical value, we will see something like 1.2GB. Also, the util I am using changes the unit depending on size. So it could be 1.2 GB or 2.4MB.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mayankshriv commented on a change in pull request #6977: Enhance debug endpoint for table debugging.
Posted by GitBox <gi...@apache.org>.
mayankshriv commented on a change in pull request #6977:
URL: https://github.com/apache/incubator-pinot/pull/6977#discussion_r641176094
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentServerDebugInfo.java
##########
@@ -0,0 +1,62 @@
+/**
+ * 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;
+
+
+/**
+ * This class represents the server-side debug information for a segment.
+ */
+@JsonIgnoreProperties(ignoreUnknown = true)
+public class SegmentServerDebugInfo {
+ private final String _segmentName;
+
+ private final SegmentConsumerInfo _consumerInfo;
+ private final SegmentErrorInfo _errorInfo;
+ private final String _segmentSize; // Segment Size in Human readable format
Review comment:
One can debug server as well.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org