kaisun2000 commented on a change in pull request #1470:
URL: https://github.com/apache/helix/pull/1470#discussion_r508070336
##########
File path: helix-core/src/main/java/org/apache/helix/task/TaskUtil.java
##########
@@ -789,14 +789,19 @@ protected static boolean removeJobsFromWorkflow(final
HelixDataAccessor dataAcce
*/
public static Set<String> getExpiredJobsFromCache(
WorkflowControllerDataProvider workflowControllerDataProvider,
WorkflowConfig workflowConfig,
- WorkflowContext workflowContext) {
+ WorkflowContext workflowContext, HelixManager manager) {
Review comment:
This is a public API change. If there is other place using this API, the
compilation may fail.
##########
File path:
helix-core/src/main/java/org/apache/helix/controller/stages/TaskGarbageCollectionStage.java
##########
@@ -81,8 +81,8 @@ public void process(ClusterEvent event) throws Exception {
if (nextPurgeTime <= currentTime) {
nextPurgeTime = currentTime + purgeInterval;
// Find jobs that are ready to be purged
- Set<String> expiredJobs =
- TaskUtil.getExpiredJobsFromCache(dataProvider, workflowConfig,
workflowContext);
+ Set<String> expiredJobs = TaskUtil
+ .getExpiredJobsFromCache(dataProvider, workflowConfig,
workflowContext, manager);
Review comment:
This is public API change. I know it is added recently, but are we sure
this won't break some other code using TaskUtil?
The other way can be simply add a check after line 85 to see if any
expiredJobs are in the config and remove them from the expiredJob set.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
