Skip to content

Enhancing Table Size API to return table size per replica #10812

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mayankshriv merged 4 commits into from
Jun 2, 2023
Merged

Enhancing Table Size API to return table size per replica #10812

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
679380a
1. Enhancing Table Size API to return size per replica
eaugene May 27, 2023
895f708
Fix Check Style
eaugene May 27, 2023
4e79d52
Fix if condition case
eaugene Jun 2, 2023
843d109
code clean up
eaugene Jun 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 4 additions & 9 deletions pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableSize.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,10 @@
import io.swagger.annotations.SwaggerDefinition;
import java.util.concurrent.Executor;
import javax.inject.Inject;
import javax.ws.rs.DefaultValue;
import javax.ws.rs.GET;
import javax.ws.rs.Path;
import javax.ws.rs.PathParam;
import javax.ws.rs.Produces;
import javax.ws.rs.QueryParam;
import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
Expand Down Expand Up @@ -72,18 +70,15 @@ public class TableSize {
@GET
@Path("/tables/{tableName}/size")
@Produces(MediaType.APPLICATION_JSON)
@ApiOperation(value = "Read table sizes",
notes = "Get table size details. Table size is the size of untarred segments including replication")
@ApiOperation(value = "Read table sizes", notes = "Get table size details. Table size is the size of untarred "
+ "segments including replication")
@ApiResponses(value = {
@ApiResponse(code = 200, message = "Success"),
@ApiResponse(code = 404, message = "Table not found"),
@ApiResponse(code = 200, message = "Success"), @ApiResponse(code = 404, message = "Table not found"),
@ApiResponse(code = 500, message = "Internal server error")
})
public TableSizeReader.TableSizeDetails getTableSize(
@ApiParam(value = "Table name without type", required = true, example = "myTable | myTable_OFFLINE")
@PathParam("tableName") String tableName,
@ApiParam(value = "Get detailed information", required = false) @DefaultValue("true") @QueryParam("detailed")
boolean detailed) {
@PathParam("tableName") String tableName) {
TableSizeReader tableSizeReader =
new TableSizeReader(_executor, _connectionManager, _controllerMetrics, _pinotHelixResourceManager);
TableSizeReader.TableSizeDetails tableSizeDetails = null;
Expand Down
76 changes: 56 additions & 20 deletions pinot-controller/src/main/java/org/apache/pinot/controller/util/TableSizeReader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,21 +85,30 @@ public TableSizeDetails getTableSizeDetails(@Nonnull String tableName, @Nonnegat
TableConfig realtimeTableConfig =
ZKMetadataProvider.getRealtimeTableConfig(_helixResourceManager.getPropertyStore(), tableName);

if (offlineTableConfig == null && realtimeTableConfig == null) {
boolean hasRealtimeTableConfig = (realtimeTableConfig != null);
boolean hasOfflineTableConfig = (offlineTableConfig != null);
boolean isMissingAllRealtimeSegments = false;
boolean isMissingAllOfflineSegments = false;

if (!hasRealtimeTableConfig && !hasOfflineTableConfig) {
return null;
}
TableSizeDetails tableSizeDetails = new TableSizeDetails(tableName);
if (realtimeTableConfig != null) {
if (hasRealtimeTableConfig) {
String realtimeTableName = TableNameBuilder.REALTIME.tableNameWithType(tableName);
tableSizeDetails._realtimeSegments = getTableSubtypeSize(realtimeTableName, timeoutMsec);
tableSizeDetails._reportedSizeInBytes += tableSizeDetails._realtimeSegments._reportedSizeInBytes;
tableSizeDetails._estimatedSizeInBytes += tableSizeDetails._realtimeSegments._estimatedSizeInBytes;

// taking max(0,value) as values as set to -1 if all the segments are in error
tableSizeDetails._reportedSizeInBytes += Math.max(tableSizeDetails._realtimeSegments._reportedSizeInBytes, 0L);
tableSizeDetails._estimatedSizeInBytes += Math.max(tableSizeDetails._realtimeSegments._estimatedSizeInBytes, 0L);
tableSizeDetails._reportedSizePerReplicaInBytes +=
Math.max(tableSizeDetails._realtimeSegments._reportedSizePerReplicaInBytes, 0L);
isMissingAllRealtimeSegments =
(tableSizeDetails._realtimeSegments._missingSegments == tableSizeDetails._realtimeSegments._segments.size());
_controllerMetrics.setValueOfTableGauge(realtimeTableName, ControllerGauge.TABLE_TOTAL_SIZE_ON_SERVER,
tableSizeDetails._realtimeSegments._estimatedSizeInBytes);
_controllerMetrics.setValueOfTableGauge(realtimeTableName, ControllerGauge.TABLE_SIZE_PER_REPLICA_ON_SERVER,
tableSizeDetails._realtimeSegments._estimatedSizeInBytes / _helixResourceManager.getNumReplicas(
realtimeTableConfig));
tableSizeDetails._realtimeSegments._estimatedSizeInBytes / _helixResourceManager
.getNumReplicas(realtimeTableConfig));

long largestSegmentSizeOnServer = DEFAULT_SIZE_WHEN_MISSING_OR_ERROR;
for (SegmentSizeDetails segmentSizeDetail : tableSizeDetails._realtimeSegments._segments.values()) {
Expand All @@ -108,22 +117,25 @@ public TableSizeDetails getTableSizeDetails(@Nonnull String tableName, @Nonnegat
}
}
if (largestSegmentSizeOnServer != DEFAULT_SIZE_WHEN_MISSING_OR_ERROR) {
_controllerMetrics.setValueOfTableGauge(realtimeTableName,
ControllerGauge.LARGEST_SEGMENT_SIZE_ON_SERVER,
_controllerMetrics.setValueOfTableGauge(realtimeTableName, ControllerGauge.LARGEST_SEGMENT_SIZE_ON_SERVER,
largestSegmentSizeOnServer);
}
}
if (offlineTableConfig != null) {
if (hasOfflineTableConfig) {
String offlineTableName = TableNameBuilder.OFFLINE.tableNameWithType(tableName);
tableSizeDetails._offlineSegments = getTableSubtypeSize(offlineTableName, timeoutMsec);
tableSizeDetails._reportedSizeInBytes += tableSizeDetails._offlineSegments._reportedSizeInBytes;
tableSizeDetails._estimatedSizeInBytes += tableSizeDetails._offlineSegments._estimatedSizeInBytes;

// taking max(0,value) as values as set to -1 if all the segments are in error
tableSizeDetails._reportedSizeInBytes += Math.max(tableSizeDetails._offlineSegments._reportedSizeInBytes, 0L);
tableSizeDetails._estimatedSizeInBytes += Math.max(tableSizeDetails._offlineSegments._estimatedSizeInBytes, 0L);
tableSizeDetails._reportedSizePerReplicaInBytes +=
Math.max(tableSizeDetails._offlineSegments._reportedSizePerReplicaInBytes, 0L);
isMissingAllOfflineSegments =
(tableSizeDetails._offlineSegments._missingSegments == tableSizeDetails._offlineSegments._segments.size());
_controllerMetrics.setValueOfTableGauge(offlineTableName, ControllerGauge.TABLE_TOTAL_SIZE_ON_SERVER,
tableSizeDetails._offlineSegments._estimatedSizeInBytes);
_controllerMetrics.setValueOfTableGauge(offlineTableName, ControllerGauge.TABLE_SIZE_PER_REPLICA_ON_SERVER,
tableSizeDetails._offlineSegments._estimatedSizeInBytes / _helixResourceManager.getNumReplicas(
offlineTableConfig));
tableSizeDetails._offlineSegments._estimatedSizeInBytes / _helixResourceManager
.getNumReplicas(offlineTableConfig));

long largestSegmentSizeOnServer = DEFAULT_SIZE_WHEN_MISSING_OR_ERROR;
for (SegmentSizeDetails segmentSizeDetail : tableSizeDetails._offlineSegments._segments.values()) {
Expand All @@ -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
Copy link
Contributor

@zhtaoxiang zhtaoxiang May 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you please help me understand the reason for this decision?

Copy link
Contributor Author

@eaugene eaugene Jun 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@zhtaoxiang zhtaoxiang Jun 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

@eaugene eaugene Jun 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

if ((hasRealtimeTableConfig && hasOfflineTableConfig && isMissingAllRealtimeSegments && isMissingAllOfflineSegments)
Copy link
Contributor

@zhtaoxiang zhtaoxiang May 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  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?

Copy link
Contributor Author

@eaugene eaugene Jun 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  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

Copy link
Contributor

@zhtaoxiang zhtaoxiang Jun 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. see the following table, with or without the first condition, the result does not change. Please check.
Screenshot 2023-06-02 at 10 08 38 AM
  1. see my reply above

Copy link
Contributor Author

@eaugene eaugene Jun 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Thanks for the Truth Table way. It helped to catch my error in a boolean algebraic fashion. Fixed it
  2. replied above

|| (hasOfflineTableConfig && !hasRealtimeTableConfig && isMissingAllOfflineSegments) || (hasRealtimeTableConfig
&& !hasOfflineTableConfig && isMissingAllRealtimeSegments)) {
tableSizeDetails._reportedSizeInBytes = DEFAULT_SIZE_WHEN_MISSING_OR_ERROR;
tableSizeDetails._estimatedSizeInBytes = DEFAULT_SIZE_WHEN_MISSING_OR_ERROR;
tableSizeDetails._reportedSizePerReplicaInBytes = DEFAULT_SIZE_WHEN_MISSING_OR_ERROR;
}
return tableSizeDetails;
}

Expand All @@ -158,6 +177,10 @@ static public class TableSizeDetails {
@JsonProperty("estimatedSizeInBytes")
public long _estimatedSizeInBytes = 0;

// reported size per replica
@JsonProperty("reportedSizePerReplicaInBytes")
public long _reportedSizePerReplicaInBytes = 0;

@JsonProperty("offlineSegments")
public TableSubTypeSizeDetails _offlineSegments;

Expand Down Expand Up @@ -186,6 +209,10 @@ static public class TableSubTypeSizeDetails {
@JsonProperty("missingSegments")
public int _missingSegments = 0;

// reported size per replica
@JsonProperty("reportedSizePerReplicaInBytes")
public long _reportedSizePerReplicaInBytes = 0;

@JsonProperty("segments")
public Map<String, SegmentSizeDetails> _segments = new HashMap<>();
}
Expand All @@ -198,6 +225,10 @@ static public class SegmentSizeDetails {
@JsonProperty("estimatedSizeInBytes")
public long _estimatedSizeInBytes = 0;

// Max Reported size per replica
@JsonProperty("maxReportedSizePerReplicaInBytes")
public long _maxReportedSizePerReplicaInBytes = 0;

@JsonProperty("serverInfo")
public Map<String, SegmentSizeInfo> _serverInfo = new HashMap<>();
}
Expand Down Expand Up @@ -248,25 +279,29 @@ 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;
sizeDetails._maxReportedSizePerReplicaInBytes = DEFAULT_SIZE_WHEN_MISSING_OR_ERROR;
int errors = 0;
for (SegmentSizeInfo sizeInfo : sizeDetails._serverInfo.values()) {
if (sizeInfo.getDiskSizeInBytes() != DEFAULT_SIZE_WHEN_MISSING_OR_ERROR) {
sizeDetails._reportedSizeInBytes += sizeInfo.getDiskSizeInBytes();
segmentLevelMax = Math.max(segmentLevelMax, sizeInfo.getDiskSizeInBytes());
sizeDetails._maxReportedSizePerReplicaInBytes =
Math.max(sizeDetails._maxReportedSizePerReplicaInBytes, sizeInfo.getDiskSizeInBytes());
} else {
errors++;
}
}
// Update estimated size, track segments that are missing from all servers
if (errors != sizeDetails._serverInfo.size()) {
// Use max segment size from other servers to estimate the segment size not reported
sizeDetails._estimatedSizeInBytes = sizeDetails._reportedSizeInBytes + errors * segmentLevelMax;
sizeDetails._estimatedSizeInBytes =
sizeDetails._reportedSizeInBytes + (errors * sizeDetails._maxReportedSizePerReplicaInBytes);
subTypeSizeDetails._reportedSizeInBytes += sizeDetails._reportedSizeInBytes;
subTypeSizeDetails._estimatedSizeInBytes += sizeDetails._estimatedSizeInBytes;
subTypeSizeDetails._reportedSizePerReplicaInBytes += sizeDetails._maxReportedSizePerReplicaInBytes;
} else {
// Segment is missing from all servers
missingSegments.add(segment);
sizeDetails._maxReportedSizePerReplicaInBytes = DEFAULT_SIZE_WHEN_MISSING_OR_ERROR;
sizeDetails._reportedSizeInBytes = DEFAULT_SIZE_WHEN_MISSING_OR_ERROR;
sizeDetails._estimatedSizeInBytes = DEFAULT_SIZE_WHEN_MISSING_OR_ERROR;
subTypeSizeDetails._missingSegments++;
Expand All @@ -284,6 +319,7 @@ public TableSubTypeSizeDetails getTableSubtypeSize(String tableNameWithType, int
LOGGER.warn("Failed to get size report for all {} segments of table: {}", numSegments, tableNameWithType);
subTypeSizeDetails._reportedSizeInBytes = DEFAULT_SIZE_WHEN_MISSING_OR_ERROR;
subTypeSizeDetails._estimatedSizeInBytes = DEFAULT_SIZE_WHEN_MISSING_OR_ERROR;
subTypeSizeDetails._reportedSizePerReplicaInBytes = DEFAULT_SIZE_WHEN_MISSING_OR_ERROR;
} else {
LOGGER.warn("Missing size report for {} out of {} segments for table {}", subTypeSizeDetails._missingSegments,
numSegments, tableNameWithType);
Expand Down
Loading