Jackie-Jiang commented on code in PR #10052: URL: https://github.com/apache/pinot/pull/10052#discussion_r1065180884
########## pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java: ########## @@ -423,6 +415,82 @@ public Response downloadSegment( } } + /** + * Download snapshot for the given immutable segment for upsert table. This endpoint is used when get snapshot from + * peer to avoid recompute when reload segments. + */ + @GET + @Produces(MediaType.APPLICATION_OCTET_STREAM) + @Path("/segments/{tableNameWithType}/{segmentName}/validDocIds") + @ApiOperation(value = "Download validDocIds for an REALTIME immutable segment", notes = "Download validDocIds for " + + "an immutable segment in bitmap format.") + public Response downloadValidDocIds( + @ApiParam(value = "Name of the table with type REALTIME", required = true, example = "myTable_REALTIME") + @PathParam("tableNameWithType") String tableNameWithType, + @ApiParam(value = "Name of the segment", required = true) @PathParam("segmentName") @Encoded String segmentName, + @Context HttpHeaders httpHeaders) + throws Exception { + LOGGER.info("Received a request to download validDocIds for segment {} table {}", segmentName, tableNameWithType); + // Validate data access + ServerResourceUtils.validateDataAccess(_accessControlFactory, tableNameWithType, httpHeaders); + + TableDataManager tableDataManager = + ServerResourceUtils.checkGetTableDataManager(_serverInstance, tableNameWithType); + SegmentDataManager segmentDataManager = tableDataManager.acquireSegment(segmentName); + if (segmentDataManager == null) { + throw new WebApplicationException( + String.format("Table %s segment %s does not exist", tableNameWithType, segmentName), + Response.Status.NOT_FOUND); + } + + try { + IndexSegment indexSegment = segmentDataManager.getSegment(); + if (indexSegment == null) { + throw new WebApplicationException( + String.format("Table %s segment %s does not exist", tableNameWithType, segmentName), + Response.Status.NOT_FOUND); + } Review Comment: This check is redundant ########## pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java: ########## @@ -423,6 +415,82 @@ public Response downloadSegment( } } + /** + * Download snapshot for the given immutable segment for upsert table. This endpoint is used when get snapshot from + * peer to avoid recompute when reload segments. + */ + @GET + @Produces(MediaType.APPLICATION_OCTET_STREAM) + @Path("/segments/{tableNameWithType}/{segmentName}/validDocIds") + @ApiOperation(value = "Download validDocIds for an REALTIME immutable segment", notes = "Download validDocIds for " + + "an immutable segment in bitmap format.") + public Response downloadValidDocIds( + @ApiParam(value = "Name of the table with type REALTIME", required = true, example = "myTable_REALTIME") + @PathParam("tableNameWithType") String tableNameWithType, + @ApiParam(value = "Name of the segment", required = true) @PathParam("segmentName") @Encoded String segmentName, + @Context HttpHeaders httpHeaders) + throws Exception { + LOGGER.info("Received a request to download validDocIds for segment {} table {}", segmentName, tableNameWithType); + // Validate data access + ServerResourceUtils.validateDataAccess(_accessControlFactory, tableNameWithType, httpHeaders); + + TableDataManager tableDataManager = + ServerResourceUtils.checkGetTableDataManager(_serverInstance, tableNameWithType); + SegmentDataManager segmentDataManager = tableDataManager.acquireSegment(segmentName); + if (segmentDataManager == null) { + throw new WebApplicationException( + String.format("Table %s segment %s does not exist", tableNameWithType, segmentName), + Response.Status.NOT_FOUND); + } + + try { + IndexSegment indexSegment = segmentDataManager.getSegment(); + if (indexSegment == null) { + throw new WebApplicationException( + String.format("Table %s segment %s does not exist", tableNameWithType, segmentName), + Response.Status.NOT_FOUND); + } + if (!(indexSegment instanceof ImmutableSegmentImpl)) { + throw new WebApplicationException( + String.format("Table %s segment %s is not a immutable segment", tableNameWithType, segmentName), + Response.Status.BAD_REQUEST); + } + MutableRoaringBitmap validDocIds = + indexSegment.getValidDocIds() != null ? indexSegment.getValidDocIds().getMutableRoaringBitmap() : null; + + File tmpSegmentTarDir = Review Comment: We don't really need to make it a file. Directly returning the serialized bytes should be good. ########## pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java: ########## @@ -423,6 +415,82 @@ public Response downloadSegment( } } + /** + * Download snapshot for the given immutable segment for upsert table. This endpoint is used when get snapshot from + * peer to avoid recompute when reload segments. + */ + @GET + @Produces(MediaType.APPLICATION_OCTET_STREAM) + @Path("/segments/{tableNameWithType}/{segmentName}/validDocIds") + @ApiOperation(value = "Download validDocIds for an REALTIME immutable segment", notes = "Download validDocIds for " + + "an immutable segment in bitmap format.") + public Response downloadValidDocIds( + @ApiParam(value = "Name of the table with type REALTIME", required = true, example = "myTable_REALTIME") + @PathParam("tableNameWithType") String tableNameWithType, + @ApiParam(value = "Name of the segment", required = true) @PathParam("segmentName") @Encoded String segmentName, + @Context HttpHeaders httpHeaders) + throws Exception { + LOGGER.info("Received a request to download validDocIds for segment {} table {}", segmentName, tableNameWithType); + // Validate data access + ServerResourceUtils.validateDataAccess(_accessControlFactory, tableNameWithType, httpHeaders); + + TableDataManager tableDataManager = + ServerResourceUtils.checkGetTableDataManager(_serverInstance, tableNameWithType); + SegmentDataManager segmentDataManager = tableDataManager.acquireSegment(segmentName); + if (segmentDataManager == null) { + throw new WebApplicationException( + String.format("Table %s segment %s does not exist", tableNameWithType, segmentName), + Response.Status.NOT_FOUND); + } + + try { + IndexSegment indexSegment = segmentDataManager.getSegment(); + if (indexSegment == null) { + throw new WebApplicationException( + String.format("Table %s segment %s does not exist", tableNameWithType, segmentName), + Response.Status.NOT_FOUND); + } + if (!(indexSegment instanceof ImmutableSegmentImpl)) { + throw new WebApplicationException( + String.format("Table %s segment %s is not a immutable segment", tableNameWithType, segmentName), + Response.Status.BAD_REQUEST); + } + MutableRoaringBitmap validDocIds = Review Comment: Check if `validDocIds` is `null` before serializing it to avoid NPE -- 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-unsubscr...@pinot.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org