New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GOBBLIN-1052: Create a spec consumer path if it does not exist in FS … #2892
Conversation
@autumnust please review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -293,6 +294,10 @@ private static void deleteStoppedHelixJob(HelixManager helixManager, String work | |||
Map<String, WorkflowConfig> workflowConfigMap = taskDriver.getWorkflows(); | |||
for (String workflow : workflowConfigMap.keySet()) { | |||
WorkflowConfig workflowConfig = taskDriver.getWorkflowConfig(workflow); | |||
//Filter out any stale Helix workflows which are not running. | |||
if (workflowConfig.getTargetState() != TargetState.START) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why filter out stopped job if the method call is to get a list of workFlowId from Gobblin job name ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this comment. The purpose of this method is two fold: 1. The caller may want to cancel a Helix workflow given a job name and 2. The caller may want to get the workflow id and use that to query other information such as the workunits processed by the job. For both 1 and 2, the caller is only interested in the active workflows. In fact, the stale workflows are auto-pruged by Helix based on an expiry interval.
I have modified the javadoc for this method to be more explicit about which workflows are returned.
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/api/FsSpecConsumer.java
Outdated
Show resolved
Hide resolved
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/api/FsSpecProducer.java
Outdated
Show resolved
Hide resolved
@autumnust Addressed your comments. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Closes apache#2892 from sv2000/specConsumer
Closes apache#2892 from sv2000/specConsumer
…SpecConsumer
Dear Gobblin maintainers,
Please accept this PR. I understand that it will not be reviewed until I have checked off all the steps below!
JIRA
Description
Create a spec consumer path if it does not exist in FS SpecConsumer. This PR also makes the following improvements:
Tests
Modified existing unit test cases.
Commits