Jackie-Jiang commented on code in PR #12663:
URL: https://github.com/apache/pinot/pull/12663#discussion_r1533078267


##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java:
##########
@@ -666,26 +666,40 @@ public SuccessResponse deleteSegment(
   @Path("/segments/{tableName}")
   @Authorize(targetType = TargetType.TABLE, paramName = "tableName", action = 
Actions.Table.DELETE_SEGMENT)
   @Authenticate(AccessType.DELETE)
-  @ApiOperation(value = "Delete all segments", notes = "Delete all segments")
-  public SuccessResponse deleteAllSegments(
+  @ApiOperation(value = "Delete the list of segments provided in the payload 
else all segments",
+      notes = "Delete the list of segments provided in the payload else all 
segments")
+  public SuccessResponse deleteMultipleSegments(
       @ApiParam(value = "Name of the table", required = true) 
@PathParam("tableName") String tableName,
       @ApiParam(value = "OFFLINE|REALTIME", required = true) 
@QueryParam("type") String tableTypeStr,
       @ApiParam(value = "Retention period for the table segments (e.g. 12h, 
3d); If not set, the retention period "
           + "will default to the first config that's not null: the table 
config, then to cluster setting, then '7d'. "
           + "Using 0d or -1d will instantly delete segments without retention")
-      @QueryParam("retention") String retentionPeriod, @Context HttpHeaders 
headers) {
+      @QueryParam("retention") String retentionPeriod,
+      @ApiParam(value = "Segment names to be deleted if not provided deletes 
all segments by default",
+          allowMultiple = true) @QueryParam("segmentNames") List<String> 
segments, @Context HttpHeaders headers) {
     tableName = DatabaseUtils.translateTableName(tableName, headers);
     TableType tableType = Constants.validateTableType(tableTypeStr);
     if (tableType == null) {
       throw new ControllerApplicationException(LOGGER, "Table type must not be 
null", Status.BAD_REQUEST);
     }
     String tableNameWithType =
         
ResourceUtils.getExistingTableNamesWithType(_pinotHelixResourceManager, 
tableName, tableType, LOGGER).get(0);
-    deleteSegmentsInternal(tableNameWithType,
-        
_pinotHelixResourceManager.getSegmentsFromPropertyStore(tableNameWithType), 
retentionPeriod);
-    return new SuccessResponse("All segments of table " + tableNameWithType + 
" deleted");
+    if (segments == null || segments.isEmpty()) {

Review Comment:
   I didn't see the `null` check in `getSegmentMetadata()`, but it doesn't harm 
to check anyway



-- 
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