somandal commented on code in PR #14250:
URL: https://github.com/apache/pinot/pull/14250#discussion_r2294891812
##########
pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java:
##########
@@ -409,6 +411,62 @@ public String getSegmentMetadata(
}
}
+ @GET
+ @Encoded
+ @Path("/tables/{tableName}/segments/metadata")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Provide segments metadata", notes = "Provide segments
metadata for the segments on server")
+ @ApiResponses(value = {
+ @ApiResponse(code = 200, message = "Success"),
+ @ApiResponse(code = 500, message = "Internal server error", response =
ErrorInfo.class),
+ @ApiResponse(code = 404, message = "Table or segment not found",
response = ErrorInfo.class)
+ })
+ public String getSegmentsMetadata(
+ @ApiParam(value = "Table name including type", required = true, example
= "myTable_OFFLINE")
+ @PathParam("tableName") String tableName,
+ @ApiParam(value = "Segments name", allowMultiple = true)
@QueryParam("segments")
+ @DefaultValue("") List<String> segments,
+ @ApiParam(value = "Column name", allowMultiple = true)
@QueryParam("columns") @DefaultValue("")
+ List<String> columns, @Context HttpHeaders headers) {
+ tableName = DatabaseUtils.translateTableName(tableName, headers);
+ TableDataManager tableDataManager =
ServerResourceUtils.checkGetTableDataManager(_serverInstance, tableName);
+ // decode columns and segments
+ List<String> decodedSegments = new ArrayList<>();
+ if (CollectionUtils.isNotEmpty(segments)) {
+ for (String segment : segments) {
+ if (!segment.isEmpty()) {
+ decodedSegments.add(URIUtils.decode(segment));
+ }
+ }
+ }
+ List<SegmentDataManager> segmentDataManagers;
+ if (!decodedSegments.isEmpty()) {
+ segmentDataManagers = tableDataManager.acquireSegments(decodedSegments,
new ArrayList<>());
+ } else {
+ segmentDataManagers = tableDataManager.acquireAllSegments();
+ }
+ for (int i = 0; i < columns.size(); i++) {
+ columns.set(i, URIUtils.decode(columns.get(i)));
+ }
+ // get metadata for every segment in the list
+ Map<String, JsonNode> response = new HashMap<>();
+ for (SegmentDataManager segmentDataManager: segmentDataManagers) {
+ String segmentName = segmentDataManager.getSegmentName();
+ try {
+ String segmentMetadata =
SegmentMetadataFetcher.getSegmentMetadata(segmentDataManager, columns);
+ JsonNode segmentMetadataJson =
JsonUtils.stringToJsonNode(segmentMetadata);
+ response.put(segmentName, segmentMetadataJson);
+ } catch (Exception e) {
+ LOGGER.error("Failed to convert table {} segment {} to json",
tableName, segmentName);
+ throw new WebApplicationException("Failed to convert segment metadata
to json",
+ Response.Status.INTERNAL_SERVER_ERROR);
+ } finally {
+ tableDataManager.releaseSegment(segmentDataManager);
+ }
Review Comment:
won't this leave some of the `segmentDataManager` unreleased, if an
exception is thrown while processing one?
Perhaps instead we should put the whole for loop under try-catch and release
everything at one go 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
