[
https://issues.apache.org/jira/browse/STORM-1277?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15305225#comment-15305225
]
ASF GitHub Bot commented on STORM-1277:
---------------------------------------
Github user unsleepy22 commented on the pull request:
https://github.com/apache/storm/pull/1445#issuecomment-222295424
@abhishekagarwal87 thanks for the review, comments inline
1. Why is ExecutorData a standalone class? Can we not put the fields of
ExecutorData inside the Executor class itself. We can pass around the Executor
object itself.
[Cody] My intention was to keep Executor itself as clean as possible,
surely we can put the fields of ExecutorData into Executor, but this might make
Executor quite large.
2. How is BaseExecutor different from Executor?
[Cody] Admittedly this is kind of confusing. Currently Executor is more
like a util class while subclasses of BaseExecutor do the real work. I think
I'm ok to make Executor as an abstract class like just like BaseExecutor and
remove BaseExecutor.
3. I assume ExecutorCommons is supposed to be the code which will be called
by both spout and bolt. Can it move to the Executor superclass itself?
[Cody] Since ExecutorCommon contains only static methods, which might be
called in OutputCollector, i.e., Spout/BoltOutputCollectorImpl, I would not put
it as a super class, still, I'm ok to put it into Executor instead of another
ExecutorCommon class (might make Executor even larger...).
4. Does ExecutorShutdown needs to be in its own class?
[Cody] My thought was to expose ExecutorShutdown to worker, which is a
simplified interface(actually the original clojure code does the same way).
Otherwise we have to implement Shutdownable, IRunningExecutor interfaces all in
Executor(as well as interfaces from BaseExecutor).
More thoughts on current design: It more or less follows the design of
JStorm, and I personally would keep a unified structure for both executor and
worker, i.e., they will have ExecutorData/WorkerData, which contain running
information for the executor/worker; they will return Shutdownable's like
ExecutorShutdown/WorkerShutdown, which are simplified interfaces.
> port backtype.storm.daemon.executor to java
> -------------------------------------------
>
> Key: STORM-1277
> URL: https://issues.apache.org/jira/browse/STORM-1277
> Project: Apache Storm
> Issue Type: New Feature
> Components: storm-core
> Reporter: Robert Joseph Evans
> Assignee: Cody
> Labels: java-migration, jstorm-merger
>
> https://github.com/apache/storm/tree/jstorm-import/jstorm-core/src/main/java/com/alibaba/jstorm/task
> kind of. Tasks and executors are combined in jstorm.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
