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

Reply via email to