You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@helix.apache.org by dayzzz <gi...@git.apache.org> on 2014/10/02 02:22:46 UTC
[GitHub] helix pull request: [HELIX-524] Add a getProgress to the Task inte...
GitHub user dayzzz opened a pull request:
https://github.com/apache/helix/pull/6
[HELIX-524] Add a getProgress to the Task interface
[HELIX-524] Add a getProgress to the Task interface, this is very helpful for long running tasks, from which we know the status of a task and see if it's blocked. The return value is a double, ranging from 0 to 1.0, 1.0 indicates a task is finished
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/dayzzz/helix master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/helix/pull/6.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 #6
----
commit 5d1b27f81abc4c67301d864f122e82f5a0ce49c3
Author: Hongbo Zeng <hz...@linkedin.com>
Date: 2014-10-02T00:19:49Z
[HELIX-524] Add a getProgress to the Task interface
----
---
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: [HELIX-524] Add a getProgress to the Task inte...
Posted by dayzzz <gi...@git.apache.org>.
Github user dayzzz commented on the pull request:
https://github.com/apache/helix/pull/6#issuecomment-57728028
* About the interface change, IMHO, this is a nature part of the task interface. If the existing customers want to update, adding the support is encouraged. We can have another interface for monitoring the progress, but doesn't seem to be a nice design.
* The task framework should call the progress interface and expose the status somehow, discussed below.
* Persistency story, ZK is a good place if we only want to record the final result. If we want to expose the progress as a task runs, putting these periodical status updates in ZK is not a choice due to the large traffic, generally ZK is not a good place for reporting and monitoring service status. I also discussed this with Jason, we thought about inGraph (which is not an option for open source), Kafka or Riemann. (Greg, JMX sounds a good idea.) Without a conclusion of where to put these status stats, I agree that the progress interface is not of much value. For the first step, it would be good enough if we can monitor the progress. What do you guys think?
* For the bogus progress number, it's the customer themselves who need to track the progress, if they want to see the bogus value, I'm fine with that :). The controller should be set not to act on the bogus value by the customers.
---
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: [HELIX-524] Add a getProgress to the Task inte...
Posted by brandtg <gi...@git.apache.org>.
Github user brandtg commented on the pull request:
https://github.com/apache/helix/pull/6#issuecomment-57597306
Wait, that stuff's gone now? Ok never mind.
Is persistence of the state of a running task completely necessary? (Like would JMX suffice?)
---
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: [HELIX-524] Add a getProgress to the Task inte...
Posted by brandtg <gi...@git.apache.org>.
Github user brandtg commented on the pull request:
https://github.com/apache/helix/pull/6#issuecomment-57596146
For persistence story, what about putting a new health report: `/STAND_ALONE/INSTANCES/{instanceName}/HEALTHREPORT/taskStatus`?
Another thing to consider is that adding `#getProgress` to the task interface requires users to be able to intelligently report task status. I suspect that in practice someone might be annoyed at this extra responsibility, and provide dummy numbers (sounds stupid, but saw it before).
Maybe a better approach would be to try to monitor on things we know a priori about task (e.g. lifetime) and provide tools to inspect ones that seem stuck (e.g. task/partition-addressable stack trace)?
---
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: [HELIX-524] Add a getProgress to the Task inte...
Posted by kanakb <gi...@git.apache.org>.
Github user kanakb commented on the pull request:
https://github.com/apache/helix/pull/6#issuecomment-58448716
Since this is an open source project, the real answer to this question is "I don't know." We've made two public releases with this change, and so if there is anyone who has integrated or is planning to integrate with the task framework, they would need to made a change.
If `ProgressReportingTask` handles the JMX reporting itself (it's an abstract class, not an interface), then `TaskRunner` wouldn't need to care about progress at all.
And back to the "bogus" return value discussion, I think customers who don't have a good sense of progress shouldn't be required to change their integration just to implement `getProgress()`.
---
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: [HELIX-524] Add a getProgress to the Task inte...
Posted by kanakb <gi...@git.apache.org>.
Github user kanakb commented on the pull request:
https://github.com/apache/helix/pull/6#issuecomment-57588016
I like this in general, but I have a couple questions/comments:
* This is an interface change. Is there a way we can provide this functionality without requiring that all existing task implementations be rewritten?
* What is the persistence story (if any)? Who calls getProgress?
---
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: [HELIX-524] Add a getProgress to the Task inte...
Posted by kanakb <gi...@git.apache.org>.
Github user kanakb commented on the pull request:
https://github.com/apache/helix/pull/6#issuecomment-57730714
Why not have an abstract ProgressReportingTask that implements task and includes all the JMX persistence, and then have tasks extends from that and implement getProgress()? That avoids the interface change, but also allows "smarter" tasks to let their progress be known.
---
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: [HELIX-524] Add a getProgress to the Task inte...
Posted by kanakb <gi...@git.apache.org>.
Github user kanakb commented on the pull request:
https://github.com/apache/helix/pull/6#issuecomment-58689390
Kishore from JIRA comments:
```
Yes, thats the idea. With helix-ipc this will be possible rt? This can be
extended to write rebalancers that talk to nodes to get the high water mark
to decide new master. what do you think?
```
Jason from JIRA comments:
```
Yes. Helix-IPC will do the job. We can also extend the idea to get high
water mark, etc. Helix task frame exposes Task interface, so users are not
implementing StateModel directly. How can we make this available to Task
also?
```
---
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: [HELIX-524] Add a getProgress to the Task inte...
Posted by kanakb <gi...@git.apache.org>.
Github user kanakb commented on the pull request:
https://github.com/apache/helix/pull/6#issuecomment-58459371
Email from Kishore:
```
Apologize to throw in another idea into the mix. Why not have additional methods in the implementation that the agent can invoke on demand based on annotation?. This extends the way we invoke methods on the statemodel.
For example we have
@Transition(TO=MASTER, FROM=SLAVE)
void fromSlaveTOMaster(Message m, NotificationContext ctx){
}
Can we have something similar as
@Method(name="getProgress")
Response getProgress(Message m, NotificationContext ctx){
}
Helix provides a generic way to invoke a method on a partition. I think this is more powerful , does not disturb any interfaces and can be extending to do custom stuff.
Also these methods will not be invoked via ZK, instead the controller can directly invoke the method on the participant.
Feedback?
```
---
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: [HELIX-524] Add a getProgress to the Task inte...
Posted by hongbozeng <gi...@git.apache.org>.
Github user hongbozeng closed the pull request at:
https://github.com/apache/helix/pull/6
---
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: [HELIX-524] Add a getProgress to the Task inte...
Posted by dayzzz <gi...@git.apache.org>.
Github user dayzzz commented on the pull request:
https://github.com/apache/helix/pull/6#issuecomment-58433144
Sorry for the delay. Are there any customers outside the Espresso team? If we are the only customer right now, it would not be too costly to update the implementation with the interface. Making the getProgress into the interface has a good thing that it naturally goes into TaskRunner and we don't need to do something like "instance of ProgressReportingTask" and then call the getProgress, or a subclass of TaskRunner which is specific for ProgressReportingTask and call the getProgress.
---
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: [HELIX-524] Add a getProgress to the Task inte...
Posted by zzhang5 <gi...@git.apache.org>.
Github user zzhang5 commented on the pull request:
https://github.com/apache/helix/pull/6#issuecomment-58587902
If we are not using ZK to invoke these methods, are we opening some kind of end-point e.g. via Netty or JMX on each participant?
---
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: [HELIX-524] Add a getProgress to the Task inte...
Posted by kanakb <gi...@git.apache.org>.
Github user kanakb commented on the pull request:
https://github.com/apache/helix/pull/6#issuecomment-58459614
I think having a `@ProgressReporter` could be an elegant way to allow users to plug in additional functionality. Progress could be just the first thing a task could expose, and in the future if we think of others, we just need to add a new annotation.
---
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.
---