You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@helix.apache.org by lei-xia <gi...@git.apache.org> on 2016/01/11 21:06:25 UTC
[GitHub] helix pull request: A few more task framework improvement
GitHub user lei-xia opened a pull request:
https://github.com/apache/helix/pull/41
A few more task framework improvement
This pull request includes four diffs (with each described as below):
1. [HELIX-622] Add new resource configuration option to allow resource to disable emmiting monitoring bean.
Description:
Helix creates a set of metrics for each resource. Since job is treated as a regular resource by Helix, each job will emit a set of new metrics to our internal monitoring system. But these metrics are dynamic date metrics, most of them are empty, it is meaningless to put any alerts on them, they are barely used in practice, but merely consuming the metric name space.
On the other hand, however, we still need some stable metrics (fix set of metric names) for operational team to monitor the queue and job running status.
For short term solution, we can add an option in JobConfig to enable emitting a metric for this job, by default, this is disabled. As a next step, we will need to add a new set of metrics for jobs and workflows.
2. Do not expose internal configuration field name, this field names should be used only by Helix, Client should always use JobConfig.Builder to create jobConfig, and construct jobConfig from HelixProperty before get fields from JobConfig. Client is not recommended to interpret fields from ZNRecord directly.
3. Clean up integration tests for task framework, move shared parts to TaskTestUtil.java.
4. Job hung if the target resource does not exist anymore at the time when it is scheduled.
Problem: When the job gets scheduled, if the target resource does not exist any more (e,g, database already deleted but the backup job is still there), the job is stuck and all the rest of jobs are stuck.
Change:If the target resource of a job does not exist, the job should be failed immediately.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/lei-xia/helix helix-0.6.x
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/helix/pull/41.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #41
----
commit 32c463d9156017f048fe53830872efc26e99b7db
Author: Lei Xia <lx...@linkedin.com>
Date: 2016-01-09T01:14:00Z
[HELIX-622] Add new resource configuration option to allow resource to disable emmiting monitoring bean.
commit a108cfb348b8ea0fdac3764b6c1672755fe64489
Author: Lei Xia <lx...@linkedin.com>
Date: 2016-01-09T01:25:09Z
[HELIX-623] Do not expose internal configuration field name. Client should use JobConfig.Builder to create jobConfig.
commit f72627c7d2c7aa9b31fa69c5832226396995c20a
Author: Lei Xia <lx...@linkedin.com>
Date: 2016-01-09T01:27:01Z
Clean up unit tests for task framework.
commit 8e2bf24c293afebd83076d9ee810cef4e43ab915
Author: Lei Xia <lx...@linkedin.com>
Date: 2016-01-09T01:28:17Z
[HELIX-618] Job hung if the target resource does not exist anymore at the time when it is scheduled.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] helix pull request #41: A few more task framework improvement
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/helix/pull/41
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] helix pull request: A few more task framework improvement
Posted by lei-xia <gi...@git.apache.org>.
Github user lei-xia commented on the pull request:
https://github.com/apache/helix/pull/41#issuecomment-182151763
Ping for reviewing!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] helix pull request: A few more task framework improvement
Posted by kishoreg <gi...@git.apache.org>.
Github user kishoreg commented on the pull request:
https://github.com/apache/helix/pull/41#issuecomment-182482218
Will review this today
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] helix pull request: A few more task framework improvement
Posted by lei-xia <gi...@git.apache.org>.
Github user lei-xia commented on a diff in the pull request:
https://github.com/apache/helix/pull/41#discussion_r52551311
--- Diff: helix-core/src/main/java/org/apache/helix/task/JobRebalancer.java ---
@@ -206,9 +205,23 @@ private ResourceAssignment computeResourceMapping(String jobResource,
TaskAssignmentCalculator taskAssignmentCal = getAssignmentCalulator(jobCfg);
Set<Integer> allPartitions =
taskAssignmentCal.getAllTaskPartitions(jobCfg, jobCtx, workflowConfig, workflowCtx, cache);
+
+ if (allPartitions == null || allPartitions.isEmpty()) {
+ // Empty target partitions, mark the job as FAILED.
+ LOG.info(
+ "Missing task partition mapping for job " + jobResource + ", marked the job as FAILED!");
--- End diff --
Changed to warn.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
Re: [GitHub] helix pull request: A few more task framework improvement
Posted by kishore g <g....@gmail.com>.
Hey missed this. Will merge it tomorrow.
On Fri, Mar 11, 2016 at 4:30 PM, lei-xia <gi...@git.apache.org> wrote:
> Github user lei-xia commented on the pull request:
>
> https://github.com/apache/helix/pull/41#issuecomment-195615244
>
> Could you please merge the pull request if it looks good to you.
> Thanks!
>
>
> ---
> If your project is set up for it, you can reply to this email and have your
> reply appear on GitHub as well. If your project does not have this feature
> enabled and wishes so, or if the feature is enabled but not working, please
> contact infrastructure at infrastructure@apache.org or file a JIRA ticket
> with INFRA.
> ---
>
[GitHub] helix pull request: A few more task framework improvement
Posted by lei-xia <gi...@git.apache.org>.
Github user lei-xia commented on the pull request:
https://github.com/apache/helix/pull/41#issuecomment-195615244
Could you please merge the pull request if it looks good to you. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] helix pull request: A few more task framework improvement
Posted by kishoreg <gi...@git.apache.org>.
Github user kishoreg commented on the pull request:
https://github.com/apache/helix/pull/41#issuecomment-209564520
Thanks, will apply the change today
On Wed, Apr 13, 2016 at 10:46 AM, Lei Xia <no...@github.com> wrote:
> Rebased to the head.
>
> —
> You are receiving this because you commented.
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/helix/pull/41#issuecomment-209563974>
>
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] helix pull request: A few more task framework improvement
Posted by lei-xia <gi...@git.apache.org>.
Github user lei-xia commented on the pull request:
https://github.com/apache/helix/pull/41#issuecomment-182658641
Thanks for reviewing!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] helix pull request: A few more task framework improvement
Posted by lei-xia <gi...@git.apache.org>.
Github user lei-xia commented on the pull request:
https://github.com/apache/helix/pull/41#issuecomment-209563974
Rebased to the head.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] helix pull request: A few more task framework improvement
Posted by kishoreg <gi...@git.apache.org>.
Github user kishoreg commented on a diff in the pull request:
https://github.com/apache/helix/pull/41#discussion_r52489589
--- Diff: helix-core/src/main/java/org/apache/helix/task/JobRebalancer.java ---
@@ -206,9 +205,23 @@ private ResourceAssignment computeResourceMapping(String jobResource,
TaskAssignmentCalculator taskAssignmentCal = getAssignmentCalulator(jobCfg);
Set<Integer> allPartitions =
taskAssignmentCal.getAllTaskPartitions(jobCfg, jobCtx, workflowConfig, workflowCtx, cache);
+
+ if (allPartitions == null || allPartitions.isEmpty()) {
+ // Empty target partitions, mark the job as FAILED.
+ LOG.info(
+ "Missing task partition mapping for job " + jobResource + ", marked the job as FAILED!");
--- End diff --
should this be warn
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] helix pull request: A few more task framework improvement
Posted by kishoreg <gi...@git.apache.org>.
Github user kishoreg commented on a diff in the pull request:
https://github.com/apache/helix/pull/41#discussion_r52488945
--- Diff: helix-core/src/main/java/org/apache/helix/controller/stages/BestPossibleStateCalcStage.java.rej ---
@@ -0,0 +1,18 @@
+diff a/helix-core/src/main/java/org/apache/helix/controller/stages/BestPossibleStateCalcStage.java b/helix-core/src/main/java/org/apache/helix/controller/stages/BestPossibleStateCalcStage.java (rejected hunks)
--- End diff --
why is this file in the source?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] helix pull request: A few more task framework improvement
Posted by lei-xia <gi...@git.apache.org>.
Github user lei-xia commented on a diff in the pull request:
https://github.com/apache/helix/pull/41#discussion_r52551298
--- Diff: helix-core/src/main/java/org/apache/helix/controller/stages/BestPossibleStateCalcStage.java.rej ---
@@ -0,0 +1,18 @@
+diff a/helix-core/src/main/java/org/apache/helix/controller/stages/BestPossibleStateCalcStage.java b/helix-core/src/main/java/org/apache/helix/controller/stages/BestPossibleStateCalcStage.java (rejected hunks)
--- End diff --
sorry, forget to delete the file. Fixed it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] helix pull request: A few more task framework improvement
Posted by kishoreg <gi...@git.apache.org>.
Github user kishoreg commented on the pull request:
https://github.com/apache/helix/pull/41#issuecomment-182487167
Looks good, please try to split the changes into smaller RB's. Its very hard to review such a big RB. Can you address the RB comments. I will merge it
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---