mcvsubbu commented on code in PR #13803:
URL: https://github.com/apache/pinot/pull/13803#discussion_r1717511427
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotRealtimeTableResource.java:
##########
@@ -103,12 +104,14 @@ public class PinotRealtimeTableResource {
@ApiOperation(value = "Pause consumption of a realtime table", notes =
"Pause the consumption of a realtime table")
public Response pauseConsumption(
@ApiParam(value = "Name of the table", required = true)
@PathParam("tableName") String tableName,
+ @ApiParam(value = "Comment on pausing the consumption")
@QueryParam("comment") String comment,
Review Comment:
The new argument is optional, right?
I suppose the compatibility story is that we will keep it optional for one
release, and then turn it into a mandatory argument?
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotRealtimeTableResource.java:
##########
@@ -103,12 +104,14 @@ public class PinotRealtimeTableResource {
@ApiOperation(value = "Pause consumption of a realtime table", notes =
"Pause the consumption of a realtime table")
public Response pauseConsumption(
@ApiParam(value = "Name of the table", required = true)
@PathParam("tableName") String tableName,
+ @ApiParam(value = "Comment on pausing the consumption")
@QueryParam("comment") String comment,
Review Comment:
Also, how about we introduce default comment string in this class, and pass
that down?
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotRealtimeTableResource.java:
##########
@@ -125,6 +128,7 @@ public Response pauseConsumption(
+ "available offsets are picked to minimize the data loss.")
public Response resumeConsumption(
@ApiParam(value = "Name of the table", required = true)
@PathParam("tableName") String tableName,
+ @ApiParam(value = "Comment on pausing the consumption")
@QueryParam("comment") String comment,
Review Comment:
same regarding compatibility
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java:
##########
@@ -963,10 +969,27 @@ void updateIdealStateOnSegmentCompletion(String
realtimeTableName, String commit
}, RetryPolicies.exponentialBackoffRetryPolicy(10, 1000L, 1.2f));
}
- private boolean isTablePaused(IdealState idealState) {
+ public static boolean isTablePaused(IdealState idealState) {
+ TablePauseStatus pauseStatus = extractTablePauseStatus(idealState);
+ if (pauseStatus != null) {
+ return pauseStatus.isPaused();
+ }
+ // backward compatibility
Review Comment:
this will be removed after some release? Please add a TODO for that
--
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
