You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "eaugene (via GitHub)" <gi...@apache.org> on 2023/05/27 14:45:54 UTC
[GitHub] [pinot] eaugene opened a new pull request, #10812: Enhancing Table Size API to return table size per replica
eaugene opened a new pull request, #10812:
URL: https://github.com/apache/pinot/pull/10812
PR contains following changes :
1. Enhancing Table Size API to return table size per replica in bytes
2. Code Clean up & removing unused param "detailed" in REST API
3. Adding & Improving the UT's
Related Discussion : https://apache-pinot.slack.com/archives/C011C9JHN7R/p1684396114803769
cc : @mayankshriv
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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] [pinot] eaugene commented on a diff in pull request #10812: Enhancing Table Size API to return table size per replica
Posted by "eaugene (via GitHub)" <gi...@apache.org>.
eaugene commented on code in PR #10812:
URL: https://github.com/apache/pinot/pull/10812#discussion_r1213188242
##########
pinot-controller/src/main/java/org/apache/pinot/controller/util/TableSizeReader.java:
##########
@@ -248,7 +279,7 @@ public TableSubTypeSizeDetails getTableSubtypeSize(String tableNameWithType, int
String segment = entry.getKey();
SegmentSizeDetails sizeDetails = entry.getValue();
// Iterate over all segment size info, update reported size, track max segment size and number of errored servers
- long segmentLevelMax = -1L;
+ long segmentLevelMax = DEFAULT_SIZE_WHEN_MISSING_OR_ERROR;
Review Comment:
Its just used as local variable to store the max value for a segment size in different servers
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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] [pinot] eaugene commented on a diff in pull request #10812: Enhancing Table Size API to return table size per replica
Posted by "eaugene (via GitHub)" <gi...@apache.org>.
eaugene commented on code in PR #10812:
URL: https://github.com/apache/pinot/pull/10812#discussion_r1214709630
##########
pinot-controller/src/main/java/org/apache/pinot/controller/util/TableSizeReader.java:
##########
@@ -132,12 +144,19 @@ public TableSizeDetails getTableSizeDetails(@Nonnull String tableName, @Nonnegat
}
}
if (largestSegmentSizeOnServer != DEFAULT_SIZE_WHEN_MISSING_OR_ERROR) {
- _controllerMetrics.setValueOfTableGauge(offlineTableName,
- ControllerGauge.LARGEST_SEGMENT_SIZE_ON_SERVER,
+ _controllerMetrics.setValueOfTableGauge(offlineTableName, ControllerGauge.LARGEST_SEGMENT_SIZE_ON_SERVER,
largestSegmentSizeOnServer);
}
}
+ // Set the top level sizes to DEFAULT_SIZE_WHEN_MISSING_OR_ERROR when all segments are error
Review Comment:
Assume the case for hybrid Table with all the segments missing .
The current implementation would have value as "-2" ( This is due to both realtime & offline block adding "-1" ). This is what I have tried to explain in my previous 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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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] [pinot] eaugene commented on pull request #10812: Enhancing Table Size API to return table size per replica
Posted by "eaugene (via GitHub)" <gi...@apache.org>.
eaugene commented on PR #10812:
URL: https://github.com/apache/pinot/pull/10812#issuecomment-1572223388
> Not required in the PR, but I feel, enhancing the api to also report the compressed size in deepstore would be quite useful.
Sure @mayankshriv . Yes , this would be useful. Created a issue for this https://github.com/apache/pinot/issues/10828
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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] [pinot] shenyu0127 commented on pull request #10812: Enhancing Table Size API to return table size per replica
Posted by "shenyu0127 (via GitHub)" <gi...@apache.org>.
shenyu0127 commented on PR #10812:
URL: https://github.com/apache/pinot/pull/10812#issuecomment-1575119639
This PR breaks the `PinotTaskManagerStatelessTest`.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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] [pinot] mayankshriv merged pull request #10812: Enhancing Table Size API to return table size per replica
Posted by "mayankshriv (via GitHub)" <gi...@apache.org>.
mayankshriv merged PR #10812:
URL: https://github.com/apache/pinot/pull/10812
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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] [pinot] eaugene commented on a diff in pull request #10812: Enhancing Table Size API to return table size per replica
Posted by "eaugene (via GitHub)" <gi...@apache.org>.
eaugene commented on code in PR #10812:
URL: https://github.com/apache/pinot/pull/10812#discussion_r1213287408
##########
pinot-controller/src/main/java/org/apache/pinot/controller/util/TableSizeReader.java:
##########
@@ -132,12 +144,19 @@ public TableSizeDetails getTableSizeDetails(@Nonnull String tableName, @Nonnegat
}
}
if (largestSegmentSizeOnServer != DEFAULT_SIZE_WHEN_MISSING_OR_ERROR) {
- _controllerMetrics.setValueOfTableGauge(offlineTableName,
- ControllerGauge.LARGEST_SEGMENT_SIZE_ON_SERVER,
+ _controllerMetrics.setValueOfTableGauge(offlineTableName, ControllerGauge.LARGEST_SEGMENT_SIZE_ON_SERVER,
largestSegmentSizeOnServer);
}
}
+ // Set the top level sizes to DEFAULT_SIZE_WHEN_MISSING_OR_ERROR when all segments are error
+ if ((hasRealtimeTableConfig && hasOfflineTableConfig && isMissingAllRealtimeSegments && isMissingAllOfflineSegments)
Review Comment:
1. We would require this condition , else it would fail for hybrid table case when All realtime segments are good, while all offline segments are missing & vice versa case as well .
2. Explained the reason here : https://github.com/apache/pinot/pull/10812/files#r1213281580 . It would be set "-1" when all existing segments of the table are missing
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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] [pinot] eaugene commented on a diff in pull request #10812: Enhancing Table Size API to return table size per replica
Posted by "eaugene (via GitHub)" <gi...@apache.org>.
eaugene commented on code in PR #10812:
URL: https://github.com/apache/pinot/pull/10812#discussion_r1213281580
##########
pinot-controller/src/main/java/org/apache/pinot/controller/util/TableSizeReader.java:
##########
@@ -132,12 +144,19 @@ public TableSizeDetails getTableSizeDetails(@Nonnull String tableName, @Nonnegat
}
}
if (largestSegmentSizeOnServer != DEFAULT_SIZE_WHEN_MISSING_OR_ERROR) {
- _controllerMetrics.setValueOfTableGauge(offlineTableName,
- ControllerGauge.LARGEST_SEGMENT_SIZE_ON_SERVER,
+ _controllerMetrics.setValueOfTableGauge(offlineTableName, ControllerGauge.LARGEST_SEGMENT_SIZE_ON_SERVER,
largestSegmentSizeOnServer);
}
}
+ // Set the top level sizes to DEFAULT_SIZE_WHEN_MISSING_OR_ERROR when all segments are error
Review Comment:
To explicitly say in response that segments are in error ( and its not 0 sized ). However, the much more correct way for the caller to identify is to look into "missingSegment" property.
With the existing implementation, this would return as "-2" which kind of diverges from the default error value we use , so I move this section at the end
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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] [pinot] eaugene commented on a diff in pull request #10812: Enhancing Table Size API to return table size per replica
Posted by "eaugene (via GitHub)" <gi...@apache.org>.
eaugene commented on code in PR #10812:
URL: https://github.com/apache/pinot/pull/10812#discussion_r1214712279
##########
pinot-controller/src/main/java/org/apache/pinot/controller/util/TableSizeReader.java:
##########
@@ -248,7 +279,7 @@ public TableSubTypeSizeDetails getTableSubtypeSize(String tableNameWithType, int
String segment = entry.getKey();
SegmentSizeDetails sizeDetails = entry.getValue();
// Iterate over all segment size info, update reported size, track max segment size and number of errored servers
- long segmentLevelMax = -1L;
+ long segmentLevelMax = DEFAULT_SIZE_WHEN_MISSING_OR_ERROR;
Review Comment:
Sure , I removed the extra variable .
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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] [pinot] codecov-commenter commented on pull request #10812: Enhancing Table Size API to return table size per replica
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #10812:
URL: https://github.com/apache/pinot/pull/10812#issuecomment-1565541340
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/10812?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#10812](https://app.codecov.io/gh/apache/pinot/pull/10812?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (895f708) into [master](https://app.codecov.io/gh/apache/pinot/commit/2e6f1e63f03530a89aaa15dd850186b05d23765b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (2e6f1e6) will **decrease** coverage by `46.28%`.
> The diff coverage is `88.57%`.
```diff
@@ Coverage Diff @@
## master #10812 +/- ##
=============================================
- Coverage 70.26% 23.99% -46.28%
+ Complexity 6517 49 -6468
=============================================
Files 2164 2148 -16
Lines 116312 115959 -353
Branches 17590 17549 -41
=============================================
- Hits 81731 27823 -53908
- Misses 28869 85203 +56334
+ Partials 5712 2933 -2779
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `23.99% <88.57%> (-0.20%)` | :arrow_down: |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `?` | |
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=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://app.codecov.io/gh/apache/pinot/pull/10812?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...ache/pinot/controller/api/resources/TableSize.java](https://app.codecov.io/gh/apache/pinot/pull/10812?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1RhYmxlU2l6ZS5qYXZh) | `0.00% <ø> (-66.67%)` | :arrow_down: |
| [.../apache/pinot/controller/util/TableSizeReader.java](https://app.codecov.io/gh/apache/pinot/pull/10812?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci91dGlsL1RhYmxlU2l6ZVJlYWRlci5qYXZh) | `77.93% <88.57%> (-21.25%)` | :arrow_down: |
... and [1624 files with indirect coverage changes](https://app.codecov.io/gh/apache/pinot/pull/10812/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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] [pinot] zhtaoxiang commented on a diff in pull request #10812: Enhancing Table Size API to return table size per replica
Posted by "zhtaoxiang (via GitHub)" <gi...@apache.org>.
zhtaoxiang commented on code in PR #10812:
URL: https://github.com/apache/pinot/pull/10812#discussion_r1212080683
##########
pinot-controller/src/main/java/org/apache/pinot/controller/util/TableSizeReader.java:
##########
@@ -132,12 +144,19 @@ public TableSizeDetails getTableSizeDetails(@Nonnull String tableName, @Nonnegat
}
}
if (largestSegmentSizeOnServer != DEFAULT_SIZE_WHEN_MISSING_OR_ERROR) {
- _controllerMetrics.setValueOfTableGauge(offlineTableName,
- ControllerGauge.LARGEST_SEGMENT_SIZE_ON_SERVER,
+ _controllerMetrics.setValueOfTableGauge(offlineTableName, ControllerGauge.LARGEST_SEGMENT_SIZE_ON_SERVER,
largestSegmentSizeOnServer);
}
}
+ // Set the top level sizes to DEFAULT_SIZE_WHEN_MISSING_OR_ERROR when all segments are error
Review Comment:
could you please help me understand the reason for this decision?
##########
pinot-controller/src/main/java/org/apache/pinot/controller/util/TableSizeReader.java:
##########
@@ -248,7 +279,7 @@ public TableSubTypeSizeDetails getTableSubtypeSize(String tableNameWithType, int
String segment = entry.getKey();
SegmentSizeDetails sizeDetails = entry.getValue();
// Iterate over all segment size info, update reported size, track max segment size and number of errored servers
- long segmentLevelMax = -1L;
+ long segmentLevelMax = DEFAULT_SIZE_WHEN_MISSING_OR_ERROR;
Review Comment:
with `sizeDetails._maxReportedSizePerReplicaInBytes`, we do not need the variable `segmentLevelMax` any more.
##########
pinot-controller/src/main/java/org/apache/pinot/controller/util/TableSizeReader.java:
##########
@@ -132,12 +144,19 @@ public TableSizeDetails getTableSizeDetails(@Nonnull String tableName, @Nonnegat
}
}
if (largestSegmentSizeOnServer != DEFAULT_SIZE_WHEN_MISSING_OR_ERROR) {
- _controllerMetrics.setValueOfTableGauge(offlineTableName,
- ControllerGauge.LARGEST_SEGMENT_SIZE_ON_SERVER,
+ _controllerMetrics.setValueOfTableGauge(offlineTableName, ControllerGauge.LARGEST_SEGMENT_SIZE_ON_SERVER,
largestSegmentSizeOnServer);
}
}
+ // Set the top level sizes to DEFAULT_SIZE_WHEN_MISSING_OR_ERROR when all segments are error
+ if ((hasRealtimeTableConfig && hasOfflineTableConfig && isMissingAllRealtimeSegments && isMissingAllOfflineSegments)
Review Comment:
1. we can remove "(hasRealtimeTableConfig && hasOfflineTableConfig && isMissingAllRealtimeSegments && isMissingAllOfflineSegments)" if we want to keep this logic
2. could you please help me understand why we want to set the the top level sizes to DEFAULT_SIZE_WHEN_MISSING_OR_ERROR when either the offline or realtime table has problems?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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] [pinot] zhtaoxiang commented on a diff in pull request #10812: Enhancing Table Size API to return table size per replica
Posted by "zhtaoxiang (via GitHub)" <gi...@apache.org>.
zhtaoxiang commented on code in PR #10812:
URL: https://github.com/apache/pinot/pull/10812#discussion_r1214592610
##########
pinot-controller/src/main/java/org/apache/pinot/controller/util/TableSizeReader.java:
##########
@@ -132,12 +144,19 @@ public TableSizeDetails getTableSizeDetails(@Nonnull String tableName, @Nonnegat
}
}
if (largestSegmentSizeOnServer != DEFAULT_SIZE_WHEN_MISSING_OR_ERROR) {
- _controllerMetrics.setValueOfTableGauge(offlineTableName,
- ControllerGauge.LARGEST_SEGMENT_SIZE_ON_SERVER,
+ _controllerMetrics.setValueOfTableGauge(offlineTableName, ControllerGauge.LARGEST_SEGMENT_SIZE_ON_SERVER,
largestSegmentSizeOnServer);
}
}
+ // Set the top level sizes to DEFAULT_SIZE_WHEN_MISSING_OR_ERROR when all segments are error
Review Comment:
Can you please help me understand why the condition is "when all segments are error". What is the major difference between "when all segments are error" and "when some segments are in error state"?
Say that a table has 1k segments, the current logic set the top level size to -1 when all 1k segments are in error state, but will not set the level size to -1 when only 1 segment is in good state. What is the major difference here?
##########
pinot-controller/src/main/java/org/apache/pinot/controller/util/TableSizeReader.java:
##########
@@ -132,12 +144,19 @@ public TableSizeDetails getTableSizeDetails(@Nonnull String tableName, @Nonnegat
}
}
if (largestSegmentSizeOnServer != DEFAULT_SIZE_WHEN_MISSING_OR_ERROR) {
- _controllerMetrics.setValueOfTableGauge(offlineTableName,
- ControllerGauge.LARGEST_SEGMENT_SIZE_ON_SERVER,
+ _controllerMetrics.setValueOfTableGauge(offlineTableName, ControllerGauge.LARGEST_SEGMENT_SIZE_ON_SERVER,
largestSegmentSizeOnServer);
}
}
+ // Set the top level sizes to DEFAULT_SIZE_WHEN_MISSING_OR_ERROR when all segments are error
+ if ((hasRealtimeTableConfig && hasOfflineTableConfig && isMissingAllRealtimeSegments && isMissingAllOfflineSegments)
Review Comment:
1. see the following table, with or without the first condition, the result does not change. Please check.
<img width="1549" alt="Screenshot 2023-06-02 at 10 08 38 AM" src="https://github.com/apache/pinot/assets/9796617/93b38569-79f0-4e13-b049-897a334db3c7">
2. see my reply above
##########
pinot-controller/src/main/java/org/apache/pinot/controller/util/TableSizeReader.java:
##########
@@ -248,7 +279,7 @@ public TableSubTypeSizeDetails getTableSubtypeSize(String tableNameWithType, int
String segment = entry.getKey();
SegmentSizeDetails sizeDetails = entry.getValue();
// Iterate over all segment size info, update reported size, track max segment size and number of errored servers
- long segmentLevelMax = -1L;
+ long segmentLevelMax = DEFAULT_SIZE_WHEN_MISSING_OR_ERROR;
Review Comment:
I agree.
It's just a minor suggestion to simplify the code a little bit.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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] [pinot] eaugene commented on a diff in pull request #10812: Enhancing Table Size API to return table size per replica
Posted by "eaugene (via GitHub)" <gi...@apache.org>.
eaugene commented on code in PR #10812:
URL: https://github.com/apache/pinot/pull/10812#discussion_r1214711171
##########
pinot-controller/src/main/java/org/apache/pinot/controller/util/TableSizeReader.java:
##########
@@ -132,12 +144,19 @@ public TableSizeDetails getTableSizeDetails(@Nonnull String tableName, @Nonnegat
}
}
if (largestSegmentSizeOnServer != DEFAULT_SIZE_WHEN_MISSING_OR_ERROR) {
- _controllerMetrics.setValueOfTableGauge(offlineTableName,
- ControllerGauge.LARGEST_SEGMENT_SIZE_ON_SERVER,
+ _controllerMetrics.setValueOfTableGauge(offlineTableName, ControllerGauge.LARGEST_SEGMENT_SIZE_ON_SERVER,
largestSegmentSizeOnServer);
}
}
+ // Set the top level sizes to DEFAULT_SIZE_WHEN_MISSING_OR_ERROR when all segments are error
+ if ((hasRealtimeTableConfig && hasOfflineTableConfig && isMissingAllRealtimeSegments && isMissingAllOfflineSegments)
Review Comment:
1. Thanks for the Truth Table way. It helped to catch my error in a boolean algebraic fashion. Fixed it
2. replied above
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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] [pinot] eaugene commented on a diff in pull request #10812: Enhancing Table Size API to return table size per replica
Posted by "eaugene (via GitHub)" <gi...@apache.org>.
eaugene commented on code in PR #10812:
URL: https://github.com/apache/pinot/pull/10812#discussion_r1214709630
##########
pinot-controller/src/main/java/org/apache/pinot/controller/util/TableSizeReader.java:
##########
@@ -132,12 +144,19 @@ public TableSizeDetails getTableSizeDetails(@Nonnull String tableName, @Nonnegat
}
}
if (largestSegmentSizeOnServer != DEFAULT_SIZE_WHEN_MISSING_OR_ERROR) {
- _controllerMetrics.setValueOfTableGauge(offlineTableName,
- ControllerGauge.LARGEST_SEGMENT_SIZE_ON_SERVER,
+ _controllerMetrics.setValueOfTableGauge(offlineTableName, ControllerGauge.LARGEST_SEGMENT_SIZE_ON_SERVER,
largestSegmentSizeOnServer);
}
}
+ // Set the top level sizes to DEFAULT_SIZE_WHEN_MISSING_OR_ERROR when all segments are error
Review Comment:
Assume the case for hybrid Table with all the segments missing .
The current implementation would have value as "-2" . This is what I have tried to explain in my previous 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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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