You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@tajo.apache.org by Hyunsik Choi <hy...@apache.org> on 2014/02/26 01:53:33 UTC
Review Request 18496: TAJO-602: WorkerResourceManager should be broke down
into 3 parts.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18496/
-----------------------------------------------------------
Review request for Tajo, Min Zhou and Jinho Kim.
Bugs: TAJO-602
https://issues.apache.org/jira/browse/TAJO-602
Repository: tajo
Description
-------
See the description of this issue at https://issues.apache.org/jira/browse/TAJO-602.
I've attached the patch for this issue. Above all, I'm very sorry for doing the job to separate the state part from TajoWorkerResourceManager.
I tried to separate the job into another jira issue, but the job was very overlapped in this issue (TAJO-602). This is because TAJO-602 is for separating heartbeat service from WorkerResourceManager and the state machine is mostly for heartbeat service. Instead, I made much effort to narrow the scope of changes and keep existing logic as possible; although I found some parts which should be improved. This patch mostly changes WorkerResourceManager and TajoWorkerResourceManager. So, I believe tha this patch mostly does not affect Min's work (TAJO-603)
During this work, I found some issues to be improved. Later, I'll create additional issues for them.
In detail, this patch does as follows:
* Separate the heartbeat service from TajoWorkerResourceManager into TajoResourceTracker class
* Separate the worker information from WorkerResource into Worker class
* Separate the ping expired node checker into WorkerLivelinessMonitor which extends AbstractLivelinessMonitor
* Separate the resource heartbeat from TajoHeartbeat into NodeHeartbeat
** Add one more heartbeat protocol and its service for that
* Add TajoRMContext which contains active or inactive worker list managed by TajoWorkerResourceManager.
* Extract the part to choose and start QueryMaster from WorkerResourceManager into QueryInProgress.
** I still keep allocateQueryMaster method in order to avoid radical API changes.
** But, allocateQueryMaster is changed to internally use allocateWorkerResources to allocate resources.
** In other words, unlike before, TajoWorkerResourceManager manages resources for both QueryMaster and worker in one resource pool management.
* Separate the heartbeat report service from Worker into WorkerHeartbeatService
The unit tests are passed sucessfully. I've tested membership management of active/inactive workers, resource heartbeat, and some queries in a local cluster.
In addition, I think that I don't have good naming sense. If you suggest better names for newly added classes, I'll appreciate it.
Thanks!
Diffs
-----
tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/CatalogServer.java dc70305
tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java 25caf13
tajo-common/src/main/java/org/apache/tajo/util/ProtoBufUtil.java PRE-CREATION
tajo-core/tajo-core-backend/pom.xml fce9925
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoContainerProxy.java e44947e
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoMaster.java 7dcc55f
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoMasterClientService.java 856566a
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoMasterService.java c3a0829
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/metrics/WorkerResourceMetricsGaugeSet.java 1924041
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryInProgress.java 6dc437f
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryInfo.java 058352f
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryJobManager.java 09d1d26
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryMaster.java 8a8772d
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/TajoRMContext.java PRE-CREATION
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/TajoResourceTracker.java PRE-CREATION
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/TajoWorkerContainer.java 5ac6e39
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/TajoWorkerResourceManager.java 2c3572c
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/Worker.java PRE-CREATION
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerEvent.java PRE-CREATION
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerEventType.java PRE-CREATION
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerLivelinessMonitor.java PRE-CREATION
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerReconnectEvent.java PRE-CREATION
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerResource.java e8c9a9e
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerResourceManager.java 1ce2c9f
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerState.java PRE-CREATION
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerStatus.java 21ad7d7
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerStatusEvent.java PRE-CREATION
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/YarnTajoResourceManager.java 80aab9b
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/worker/TajoResourceAllocator.java 222d355
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/worker/TajoWorker.java 2f763e3
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/worker/WorkerHeartbeatService.java PRE-CREATION
tajo-core/tajo-core-backend/src/main/proto/ResourceTrackerProtocol.proto PRE-CREATION
tajo-core/tajo-core-backend/src/main/proto/TajoMasterProtocol.proto 5e4088e
tajo-core/tajo-core-backend/src/main/resources/webapps/admin/cluster.jsp 0600145
tajo-core/tajo-core-backend/src/main/resources/webapps/admin/index.jsp f652ea5
tajo-core/tajo-core-backend/src/main/resources/webapps/admin/query.jsp e3a356d
tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/TajoTestingCluster.java 9c96e0e
tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/master/TestTajoResourceManager.java 9504927
tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/master/rm/TestTajoResourceManager.java PRE-CREATION
tajo-rpc/src/main/java/org/apache/tajo/rpc/NettyServerBase.java 371f879
Diff: https://reviews.apache.org/r/18496/diff/
Testing
-------
mvn clean install
Thanks,
Hyunsik Choi
Re: Review Request 18496: TAJO-602: WorkerResourceManager should be broke
down into 3 parts.
Posted by Hyunsik Choi <hy...@apache.org>.
> On Feb. 27, 2014, 2:51 p.m., Min Zhou wrote:
> > tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerResourceManager.java, line 50
> > <https://reviews.apache.org/r/18496/diff/1/?file=503961#file503961line50>
> >
> > Why don't you separate the interfaces allocateQueryMaster and allocateWorker.
> > I mean QueryMaster related methods should be run in TajoMaster, but worker related methods should be run in QueryMaster. Still keeping those two will confuse the following developer where this interface be run. How about start 2 ResourceManger , one is WorkerResourceManager, the other is QueryMasterResourceManager?
>
> Min Zhou wrote:
> oops, my fault. please ignore this previous comment
It's Ok :)
> On Feb. 27, 2014, 2:51 p.m., Min Zhou wrote:
> > tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerResourceManager.java, line 61
> > <https://reviews.apache.org/r/18496/diff/1/?file=503961#file503961line61>
> >
> > Modification on this file will conflict with TAJO-603, but never mind. It's better than the original code.
Thank you for your understanding.
> On Feb. 27, 2014, 2:51 p.m., Min Zhou wrote:
> > tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerState.java, line 24
> > <https://reviews.apache.org/r/18496/diff/1/?file=503962#file503962line24>
> >
> > It's good to have a state machine for worker. I wonder know this state will reside in TajoMaster or QueryMaster?
This state is for a worker, actually running as one java processor. This state is managed in TajoWorkerResourceManager in TajoMaster.
- Hyunsik
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18496/#review35601
-----------------------------------------------------------
On Feb. 26, 2014, 9:53 a.m., Hyunsik Choi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18496/
> -----------------------------------------------------------
>
> (Updated Feb. 26, 2014, 9:53 a.m.)
>
>
> Review request for Tajo, Min Zhou and Jinho Kim.
>
>
> Bugs: TAJO-602
> https://issues.apache.org/jira/browse/TAJO-602
>
>
> Repository: tajo
>
>
> Description
> -------
>
> See the description of this issue at https://issues.apache.org/jira/browse/TAJO-602.
>
> I've attached the patch for this issue. Above all, I'm very sorry for doing the job to separate the state part from TajoWorkerResourceManager.
> I tried to separate the job into another jira issue, but the job was very overlapped in this issue (TAJO-602). This is because TAJO-602 is for separating heartbeat service from WorkerResourceManager and the state machine is mostly for heartbeat service. Instead, I made much effort to narrow the scope of changes and keep existing logic as possible; although I found some parts which should be improved. This patch mostly changes WorkerResourceManager and TajoWorkerResourceManager. So, I believe tha this patch mostly does not affect Min's work (TAJO-603)
>
> During this work, I found some issues to be improved. Later, I'll create additional issues for them.
>
> In detail, this patch does as follows:
>
> * Separate the heartbeat service from TajoWorkerResourceManager into TajoResourceTracker class
> * Separate the worker information from WorkerResource into Worker class
> * Separate the ping expired node checker into WorkerLivelinessMonitor which extends AbstractLivelinessMonitor
> * Separate the resource heartbeat from TajoHeartbeat into NodeHeartbeat
> ** Add one more heartbeat protocol and its service for that
> * Add TajoRMContext which contains active or inactive worker list managed by TajoWorkerResourceManager.
> * Extract the part to choose and start QueryMaster from WorkerResourceManager into QueryInProgress.
> ** I still keep allocateQueryMaster method in order to avoid radical API changes.
> ** But, allocateQueryMaster is changed to internally use allocateWorkerResources to allocate resources.
> ** In other words, unlike before, TajoWorkerResourceManager manages resources for both QueryMaster and worker in one resource pool management.
> * Separate the heartbeat report service from Worker into WorkerHeartbeatService
>
> The unit tests are passed sucessfully. I've tested membership management of active/inactive workers, resource heartbeat, and some queries in a local cluster.
>
> In addition, I think that I don't have good naming sense. If you suggest better names for newly added classes, I'll appreciate it.
>
> Thanks!
>
>
> Diffs
> -----
>
> tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/CatalogServer.java dc70305
> tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java 25caf13
> tajo-common/src/main/java/org/apache/tajo/util/ProtoBufUtil.java PRE-CREATION
> tajo-core/tajo-core-backend/pom.xml fce9925
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoContainerProxy.java e44947e
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoMaster.java 7dcc55f
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoMasterClientService.java 856566a
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoMasterService.java c3a0829
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/metrics/WorkerResourceMetricsGaugeSet.java 1924041
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryInProgress.java 6dc437f
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryInfo.java 058352f
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryJobManager.java 09d1d26
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryMaster.java 8a8772d
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/TajoRMContext.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/TajoResourceTracker.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/TajoWorkerContainer.java 5ac6e39
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/TajoWorkerResourceManager.java 2c3572c
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/Worker.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerEvent.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerEventType.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerLivelinessMonitor.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerReconnectEvent.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerResource.java e8c9a9e
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerResourceManager.java 1ce2c9f
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerState.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerStatus.java 21ad7d7
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerStatusEvent.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/YarnTajoResourceManager.java 80aab9b
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/worker/TajoResourceAllocator.java 222d355
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/worker/TajoWorker.java 2f763e3
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/worker/WorkerHeartbeatService.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/proto/ResourceTrackerProtocol.proto PRE-CREATION
> tajo-core/tajo-core-backend/src/main/proto/TajoMasterProtocol.proto 5e4088e
> tajo-core/tajo-core-backend/src/main/resources/webapps/admin/cluster.jsp 0600145
> tajo-core/tajo-core-backend/src/main/resources/webapps/admin/index.jsp f652ea5
> tajo-core/tajo-core-backend/src/main/resources/webapps/admin/query.jsp e3a356d
> tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/TajoTestingCluster.java 9c96e0e
> tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/master/TestTajoResourceManager.java 9504927
> tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/master/rm/TestTajoResourceManager.java PRE-CREATION
> tajo-rpc/src/main/java/org/apache/tajo/rpc/NettyServerBase.java 371f879
>
> Diff: https://reviews.apache.org/r/18496/diff/
>
>
> Testing
> -------
>
> mvn clean install
>
>
> Thanks,
>
> Hyunsik Choi
>
>
Re: Review Request 18496: TAJO-602: WorkerResourceManager should be broke
down into 3 parts.
Posted by Min Zhou <co...@gmail.com>.
> On Feb. 27, 2014, 5:51 a.m., Min Zhou wrote:
> > tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerResourceManager.java, line 50
> > <https://reviews.apache.org/r/18496/diff/1/?file=503961#file503961line50>
> >
> > Why don't you separate the interfaces allocateQueryMaster and allocateWorker.
> > I mean QueryMaster related methods should be run in TajoMaster, but worker related methods should be run in QueryMaster. Still keeping those two will confuse the following developer where this interface be run. How about start 2 ResourceManger , one is WorkerResourceManager, the other is QueryMasterResourceManager?
oops, my fault. please ignore this previous comment
- Min
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18496/#review35601
-----------------------------------------------------------
On Feb. 26, 2014, 12:53 a.m., Hyunsik Choi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18496/
> -----------------------------------------------------------
>
> (Updated Feb. 26, 2014, 12:53 a.m.)
>
>
> Review request for Tajo, Min Zhou and Jinho Kim.
>
>
> Bugs: TAJO-602
> https://issues.apache.org/jira/browse/TAJO-602
>
>
> Repository: tajo
>
>
> Description
> -------
>
> See the description of this issue at https://issues.apache.org/jira/browse/TAJO-602.
>
> I've attached the patch for this issue. Above all, I'm very sorry for doing the job to separate the state part from TajoWorkerResourceManager.
> I tried to separate the job into another jira issue, but the job was very overlapped in this issue (TAJO-602). This is because TAJO-602 is for separating heartbeat service from WorkerResourceManager and the state machine is mostly for heartbeat service. Instead, I made much effort to narrow the scope of changes and keep existing logic as possible; although I found some parts which should be improved. This patch mostly changes WorkerResourceManager and TajoWorkerResourceManager. So, I believe tha this patch mostly does not affect Min's work (TAJO-603)
>
> During this work, I found some issues to be improved. Later, I'll create additional issues for them.
>
> In detail, this patch does as follows:
>
> * Separate the heartbeat service from TajoWorkerResourceManager into TajoResourceTracker class
> * Separate the worker information from WorkerResource into Worker class
> * Separate the ping expired node checker into WorkerLivelinessMonitor which extends AbstractLivelinessMonitor
> * Separate the resource heartbeat from TajoHeartbeat into NodeHeartbeat
> ** Add one more heartbeat protocol and its service for that
> * Add TajoRMContext which contains active or inactive worker list managed by TajoWorkerResourceManager.
> * Extract the part to choose and start QueryMaster from WorkerResourceManager into QueryInProgress.
> ** I still keep allocateQueryMaster method in order to avoid radical API changes.
> ** But, allocateQueryMaster is changed to internally use allocateWorkerResources to allocate resources.
> ** In other words, unlike before, TajoWorkerResourceManager manages resources for both QueryMaster and worker in one resource pool management.
> * Separate the heartbeat report service from Worker into WorkerHeartbeatService
>
> The unit tests are passed sucessfully. I've tested membership management of active/inactive workers, resource heartbeat, and some queries in a local cluster.
>
> In addition, I think that I don't have good naming sense. If you suggest better names for newly added classes, I'll appreciate it.
>
> Thanks!
>
>
> Diffs
> -----
>
> tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/CatalogServer.java dc70305
> tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java 25caf13
> tajo-common/src/main/java/org/apache/tajo/util/ProtoBufUtil.java PRE-CREATION
> tajo-core/tajo-core-backend/pom.xml fce9925
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoContainerProxy.java e44947e
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoMaster.java 7dcc55f
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoMasterClientService.java 856566a
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoMasterService.java c3a0829
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/metrics/WorkerResourceMetricsGaugeSet.java 1924041
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryInProgress.java 6dc437f
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryInfo.java 058352f
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryJobManager.java 09d1d26
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryMaster.java 8a8772d
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/TajoRMContext.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/TajoResourceTracker.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/TajoWorkerContainer.java 5ac6e39
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/TajoWorkerResourceManager.java 2c3572c
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/Worker.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerEvent.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerEventType.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerLivelinessMonitor.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerReconnectEvent.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerResource.java e8c9a9e
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerResourceManager.java 1ce2c9f
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerState.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerStatus.java 21ad7d7
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerStatusEvent.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/YarnTajoResourceManager.java 80aab9b
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/worker/TajoResourceAllocator.java 222d355
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/worker/TajoWorker.java 2f763e3
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/worker/WorkerHeartbeatService.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/proto/ResourceTrackerProtocol.proto PRE-CREATION
> tajo-core/tajo-core-backend/src/main/proto/TajoMasterProtocol.proto 5e4088e
> tajo-core/tajo-core-backend/src/main/resources/webapps/admin/cluster.jsp 0600145
> tajo-core/tajo-core-backend/src/main/resources/webapps/admin/index.jsp f652ea5
> tajo-core/tajo-core-backend/src/main/resources/webapps/admin/query.jsp e3a356d
> tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/TajoTestingCluster.java 9c96e0e
> tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/master/TestTajoResourceManager.java 9504927
> tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/master/rm/TestTajoResourceManager.java PRE-CREATION
> tajo-rpc/src/main/java/org/apache/tajo/rpc/NettyServerBase.java 371f879
>
> Diff: https://reviews.apache.org/r/18496/diff/
>
>
> Testing
> -------
>
> mvn clean install
>
>
> Thanks,
>
> Hyunsik Choi
>
>
Re: Review Request 18496: TAJO-602: WorkerResourceManager should be broke
down into 3 parts.
Posted by Min Zhou <co...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18496/#review35601
-----------------------------------------------------------
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerResourceManager.java
<https://reviews.apache.org/r/18496/#comment66249>
Why don't you separate the interfaces allocateQueryMaster and allocateWorker.
I mean QueryMaster related methods should be run in TajoMaster, but worker related methods should be run in QueryMaster. Still keeping those two will confuse the following developer where this interface be run. How about start 2 ResourceManger , one is WorkerResourceManager, the other is QueryMasterResourceManager?
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerResourceManager.java
<https://reviews.apache.org/r/18496/#comment66247>
Modification on this file will conflict with TAJO-603, but never mind. It's better than the original code.
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerResourceManager.java
<https://reviews.apache.org/r/18496/#comment66246>
Great! It's really helpful for TAJO-611 to uniform the service discovery interface
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerState.java
<https://reviews.apache.org/r/18496/#comment66248>
It's good to have a state machine for worker. I wonder know this state will reside in TajoMaster or QueryMaster?
- Min Zhou
On Feb. 26, 2014, 12:53 a.m., Hyunsik Choi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18496/
> -----------------------------------------------------------
>
> (Updated Feb. 26, 2014, 12:53 a.m.)
>
>
> Review request for Tajo, Min Zhou and Jinho Kim.
>
>
> Bugs: TAJO-602
> https://issues.apache.org/jira/browse/TAJO-602
>
>
> Repository: tajo
>
>
> Description
> -------
>
> See the description of this issue at https://issues.apache.org/jira/browse/TAJO-602.
>
> I've attached the patch for this issue. Above all, I'm very sorry for doing the job to separate the state part from TajoWorkerResourceManager.
> I tried to separate the job into another jira issue, but the job was very overlapped in this issue (TAJO-602). This is because TAJO-602 is for separating heartbeat service from WorkerResourceManager and the state machine is mostly for heartbeat service. Instead, I made much effort to narrow the scope of changes and keep existing logic as possible; although I found some parts which should be improved. This patch mostly changes WorkerResourceManager and TajoWorkerResourceManager. So, I believe tha this patch mostly does not affect Min's work (TAJO-603)
>
> During this work, I found some issues to be improved. Later, I'll create additional issues for them.
>
> In detail, this patch does as follows:
>
> * Separate the heartbeat service from TajoWorkerResourceManager into TajoResourceTracker class
> * Separate the worker information from WorkerResource into Worker class
> * Separate the ping expired node checker into WorkerLivelinessMonitor which extends AbstractLivelinessMonitor
> * Separate the resource heartbeat from TajoHeartbeat into NodeHeartbeat
> ** Add one more heartbeat protocol and its service for that
> * Add TajoRMContext which contains active or inactive worker list managed by TajoWorkerResourceManager.
> * Extract the part to choose and start QueryMaster from WorkerResourceManager into QueryInProgress.
> ** I still keep allocateQueryMaster method in order to avoid radical API changes.
> ** But, allocateQueryMaster is changed to internally use allocateWorkerResources to allocate resources.
> ** In other words, unlike before, TajoWorkerResourceManager manages resources for both QueryMaster and worker in one resource pool management.
> * Separate the heartbeat report service from Worker into WorkerHeartbeatService
>
> The unit tests are passed sucessfully. I've tested membership management of active/inactive workers, resource heartbeat, and some queries in a local cluster.
>
> In addition, I think that I don't have good naming sense. If you suggest better names for newly added classes, I'll appreciate it.
>
> Thanks!
>
>
> Diffs
> -----
>
> tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/CatalogServer.java dc70305
> tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java 25caf13
> tajo-common/src/main/java/org/apache/tajo/util/ProtoBufUtil.java PRE-CREATION
> tajo-core/tajo-core-backend/pom.xml fce9925
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoContainerProxy.java e44947e
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoMaster.java 7dcc55f
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoMasterClientService.java 856566a
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoMasterService.java c3a0829
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/metrics/WorkerResourceMetricsGaugeSet.java 1924041
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryInProgress.java 6dc437f
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryInfo.java 058352f
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryJobManager.java 09d1d26
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryMaster.java 8a8772d
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/TajoRMContext.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/TajoResourceTracker.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/TajoWorkerContainer.java 5ac6e39
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/TajoWorkerResourceManager.java 2c3572c
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/Worker.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerEvent.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerEventType.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerLivelinessMonitor.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerReconnectEvent.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerResource.java e8c9a9e
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerResourceManager.java 1ce2c9f
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerState.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerStatus.java 21ad7d7
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerStatusEvent.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/YarnTajoResourceManager.java 80aab9b
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/worker/TajoResourceAllocator.java 222d355
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/worker/TajoWorker.java 2f763e3
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/worker/WorkerHeartbeatService.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/proto/ResourceTrackerProtocol.proto PRE-CREATION
> tajo-core/tajo-core-backend/src/main/proto/TajoMasterProtocol.proto 5e4088e
> tajo-core/tajo-core-backend/src/main/resources/webapps/admin/cluster.jsp 0600145
> tajo-core/tajo-core-backend/src/main/resources/webapps/admin/index.jsp f652ea5
> tajo-core/tajo-core-backend/src/main/resources/webapps/admin/query.jsp e3a356d
> tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/TajoTestingCluster.java 9c96e0e
> tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/master/TestTajoResourceManager.java 9504927
> tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/master/rm/TestTajoResourceManager.java PRE-CREATION
> tajo-rpc/src/main/java/org/apache/tajo/rpc/NettyServerBase.java 371f879
>
> Diff: https://reviews.apache.org/r/18496/diff/
>
>
> Testing
> -------
>
> mvn clean install
>
>
> Thanks,
>
> Hyunsik Choi
>
>
Re: Review Request 18496: TAJO-602: WorkerResourceManager should be broke
down into 3 parts.
Posted by Jung JaeHwa <jh...@gruter.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18496/#review35753
-----------------------------------------------------------
Ship it!
+1.
Thanks Hyunsik.
I love this great patch.
Push it now. :)
- Jung JaeHwa
On Feb. 26, 2014, 12:53 a.m., Hyunsik Choi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18496/
> -----------------------------------------------------------
>
> (Updated Feb. 26, 2014, 12:53 a.m.)
>
>
> Review request for Tajo, Min Zhou and Jinho Kim.
>
>
> Bugs: TAJO-602
> https://issues.apache.org/jira/browse/TAJO-602
>
>
> Repository: tajo
>
>
> Description
> -------
>
> See the description of this issue at https://issues.apache.org/jira/browse/TAJO-602.
>
> I've attached the patch for this issue. Above all, I'm very sorry for doing the job to separate the state part from TajoWorkerResourceManager.
> I tried to separate the job into another jira issue, but the job was very overlapped in this issue (TAJO-602). This is because TAJO-602 is for separating heartbeat service from WorkerResourceManager and the state machine is mostly for heartbeat service. Instead, I made much effort to narrow the scope of changes and keep existing logic as possible; although I found some parts which should be improved. This patch mostly changes WorkerResourceManager and TajoWorkerResourceManager. So, I believe tha this patch mostly does not affect Min's work (TAJO-603)
>
> During this work, I found some issues to be improved. Later, I'll create additional issues for them.
>
> In detail, this patch does as follows:
>
> * Separate the heartbeat service from TajoWorkerResourceManager into TajoResourceTracker class
> * Separate the worker information from WorkerResource into Worker class
> * Separate the ping expired node checker into WorkerLivelinessMonitor which extends AbstractLivelinessMonitor
> * Separate the resource heartbeat from TajoHeartbeat into NodeHeartbeat
> ** Add one more heartbeat protocol and its service for that
> * Add TajoRMContext which contains active or inactive worker list managed by TajoWorkerResourceManager.
> * Extract the part to choose and start QueryMaster from WorkerResourceManager into QueryInProgress.
> ** I still keep allocateQueryMaster method in order to avoid radical API changes.
> ** But, allocateQueryMaster is changed to internally use allocateWorkerResources to allocate resources.
> ** In other words, unlike before, TajoWorkerResourceManager manages resources for both QueryMaster and worker in one resource pool management.
> * Separate the heartbeat report service from Worker into WorkerHeartbeatService
>
> The unit tests are passed sucessfully. I've tested membership management of active/inactive workers, resource heartbeat, and some queries in a local cluster.
>
> In addition, I think that I don't have good naming sense. If you suggest better names for newly added classes, I'll appreciate it.
>
> Thanks!
>
>
> Diffs
> -----
>
> tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/CatalogServer.java dc70305
> tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java 25caf13
> tajo-common/src/main/java/org/apache/tajo/util/ProtoBufUtil.java PRE-CREATION
> tajo-core/tajo-core-backend/pom.xml fce9925
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoContainerProxy.java e44947e
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoMaster.java 7dcc55f
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoMasterClientService.java 856566a
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoMasterService.java c3a0829
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/metrics/WorkerResourceMetricsGaugeSet.java 1924041
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryInProgress.java 6dc437f
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryInfo.java 058352f
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryJobManager.java 09d1d26
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryMaster.java 8a8772d
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/TajoRMContext.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/TajoResourceTracker.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/TajoWorkerContainer.java 5ac6e39
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/TajoWorkerResourceManager.java 2c3572c
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/Worker.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerEvent.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerEventType.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerLivelinessMonitor.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerReconnectEvent.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerResource.java e8c9a9e
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerResourceManager.java 1ce2c9f
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerState.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerStatus.java 21ad7d7
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerStatusEvent.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/YarnTajoResourceManager.java 80aab9b
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/worker/TajoResourceAllocator.java 222d355
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/worker/TajoWorker.java 2f763e3
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/worker/WorkerHeartbeatService.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/proto/ResourceTrackerProtocol.proto PRE-CREATION
> tajo-core/tajo-core-backend/src/main/proto/TajoMasterProtocol.proto 5e4088e
> tajo-core/tajo-core-backend/src/main/resources/webapps/admin/cluster.jsp 0600145
> tajo-core/tajo-core-backend/src/main/resources/webapps/admin/index.jsp f652ea5
> tajo-core/tajo-core-backend/src/main/resources/webapps/admin/query.jsp e3a356d
> tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/TajoTestingCluster.java 9c96e0e
> tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/master/TestTajoResourceManager.java 9504927
> tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/master/rm/TestTajoResourceManager.java PRE-CREATION
> tajo-rpc/src/main/java/org/apache/tajo/rpc/NettyServerBase.java 371f879
>
> Diff: https://reviews.apache.org/r/18496/diff/
>
>
> Testing
> -------
>
> mvn clean install
>
>
> Thanks,
>
> Hyunsik Choi
>
>
Re: Review Request 18496: TAJO-602: WorkerResourceManager should be broke
down into 3 parts.
Posted by Min Zhou <co...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18496/#review35612
-----------------------------------------------------------
Ship it!
+1, Looks good to me.
- Min Zhou
On Feb. 26, 2014, 12:53 a.m., Hyunsik Choi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18496/
> -----------------------------------------------------------
>
> (Updated Feb. 26, 2014, 12:53 a.m.)
>
>
> Review request for Tajo, Min Zhou and Jinho Kim.
>
>
> Bugs: TAJO-602
> https://issues.apache.org/jira/browse/TAJO-602
>
>
> Repository: tajo
>
>
> Description
> -------
>
> See the description of this issue at https://issues.apache.org/jira/browse/TAJO-602.
>
> I've attached the patch for this issue. Above all, I'm very sorry for doing the job to separate the state part from TajoWorkerResourceManager.
> I tried to separate the job into another jira issue, but the job was very overlapped in this issue (TAJO-602). This is because TAJO-602 is for separating heartbeat service from WorkerResourceManager and the state machine is mostly for heartbeat service. Instead, I made much effort to narrow the scope of changes and keep existing logic as possible; although I found some parts which should be improved. This patch mostly changes WorkerResourceManager and TajoWorkerResourceManager. So, I believe tha this patch mostly does not affect Min's work (TAJO-603)
>
> During this work, I found some issues to be improved. Later, I'll create additional issues for them.
>
> In detail, this patch does as follows:
>
> * Separate the heartbeat service from TajoWorkerResourceManager into TajoResourceTracker class
> * Separate the worker information from WorkerResource into Worker class
> * Separate the ping expired node checker into WorkerLivelinessMonitor which extends AbstractLivelinessMonitor
> * Separate the resource heartbeat from TajoHeartbeat into NodeHeartbeat
> ** Add one more heartbeat protocol and its service for that
> * Add TajoRMContext which contains active or inactive worker list managed by TajoWorkerResourceManager.
> * Extract the part to choose and start QueryMaster from WorkerResourceManager into QueryInProgress.
> ** I still keep allocateQueryMaster method in order to avoid radical API changes.
> ** But, allocateQueryMaster is changed to internally use allocateWorkerResources to allocate resources.
> ** In other words, unlike before, TajoWorkerResourceManager manages resources for both QueryMaster and worker in one resource pool management.
> * Separate the heartbeat report service from Worker into WorkerHeartbeatService
>
> The unit tests are passed sucessfully. I've tested membership management of active/inactive workers, resource heartbeat, and some queries in a local cluster.
>
> In addition, I think that I don't have good naming sense. If you suggest better names for newly added classes, I'll appreciate it.
>
> Thanks!
>
>
> Diffs
> -----
>
> tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/CatalogServer.java dc70305
> tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java 25caf13
> tajo-common/src/main/java/org/apache/tajo/util/ProtoBufUtil.java PRE-CREATION
> tajo-core/tajo-core-backend/pom.xml fce9925
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoContainerProxy.java e44947e
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoMaster.java 7dcc55f
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoMasterClientService.java 856566a
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoMasterService.java c3a0829
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/metrics/WorkerResourceMetricsGaugeSet.java 1924041
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryInProgress.java 6dc437f
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryInfo.java 058352f
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryJobManager.java 09d1d26
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryMaster.java 8a8772d
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/TajoRMContext.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/TajoResourceTracker.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/TajoWorkerContainer.java 5ac6e39
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/TajoWorkerResourceManager.java 2c3572c
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/Worker.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerEvent.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerEventType.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerLivelinessMonitor.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerReconnectEvent.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerResource.java e8c9a9e
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerResourceManager.java 1ce2c9f
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerState.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerStatus.java 21ad7d7
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerStatusEvent.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/YarnTajoResourceManager.java 80aab9b
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/worker/TajoResourceAllocator.java 222d355
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/worker/TajoWorker.java 2f763e3
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/worker/WorkerHeartbeatService.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/proto/ResourceTrackerProtocol.proto PRE-CREATION
> tajo-core/tajo-core-backend/src/main/proto/TajoMasterProtocol.proto 5e4088e
> tajo-core/tajo-core-backend/src/main/resources/webapps/admin/cluster.jsp 0600145
> tajo-core/tajo-core-backend/src/main/resources/webapps/admin/index.jsp f652ea5
> tajo-core/tajo-core-backend/src/main/resources/webapps/admin/query.jsp e3a356d
> tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/TajoTestingCluster.java 9c96e0e
> tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/master/TestTajoResourceManager.java 9504927
> tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/master/rm/TestTajoResourceManager.java PRE-CREATION
> tajo-rpc/src/main/java/org/apache/tajo/rpc/NettyServerBase.java 371f879
>
> Diff: https://reviews.apache.org/r/18496/diff/
>
>
> Testing
> -------
>
> mvn clean install
>
>
> Thanks,
>
> Hyunsik Choi
>
>
Re: Review Request 18496: TAJO-602: WorkerResourceManager should be broke
down into 3 parts.
Posted by Hyunsik Choi <hy...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18496/#review35766
-----------------------------------------------------------
Thank you for the your reviews. I'll commit it to master branch. Thanks.
- Hyunsik Choi
On Feb. 26, 2014, 9:53 a.m., Hyunsik Choi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18496/
> -----------------------------------------------------------
>
> (Updated Feb. 26, 2014, 9:53 a.m.)
>
>
> Review request for Tajo, Min Zhou and Jinho Kim.
>
>
> Bugs: TAJO-602
> https://issues.apache.org/jira/browse/TAJO-602
>
>
> Repository: tajo
>
>
> Description
> -------
>
> See the description of this issue at https://issues.apache.org/jira/browse/TAJO-602.
>
> I've attached the patch for this issue. Above all, I'm very sorry for doing the job to separate the state part from TajoWorkerResourceManager.
> I tried to separate the job into another jira issue, but the job was very overlapped in this issue (TAJO-602). This is because TAJO-602 is for separating heartbeat service from WorkerResourceManager and the state machine is mostly for heartbeat service. Instead, I made much effort to narrow the scope of changes and keep existing logic as possible; although I found some parts which should be improved. This patch mostly changes WorkerResourceManager and TajoWorkerResourceManager. So, I believe tha this patch mostly does not affect Min's work (TAJO-603)
>
> During this work, I found some issues to be improved. Later, I'll create additional issues for them.
>
> In detail, this patch does as follows:
>
> * Separate the heartbeat service from TajoWorkerResourceManager into TajoResourceTracker class
> * Separate the worker information from WorkerResource into Worker class
> * Separate the ping expired node checker into WorkerLivelinessMonitor which extends AbstractLivelinessMonitor
> * Separate the resource heartbeat from TajoHeartbeat into NodeHeartbeat
> ** Add one more heartbeat protocol and its service for that
> * Add TajoRMContext which contains active or inactive worker list managed by TajoWorkerResourceManager.
> * Extract the part to choose and start QueryMaster from WorkerResourceManager into QueryInProgress.
> ** I still keep allocateQueryMaster method in order to avoid radical API changes.
> ** But, allocateQueryMaster is changed to internally use allocateWorkerResources to allocate resources.
> ** In other words, unlike before, TajoWorkerResourceManager manages resources for both QueryMaster and worker in one resource pool management.
> * Separate the heartbeat report service from Worker into WorkerHeartbeatService
>
> The unit tests are passed sucessfully. I've tested membership management of active/inactive workers, resource heartbeat, and some queries in a local cluster.
>
> In addition, I think that I don't have good naming sense. If you suggest better names for newly added classes, I'll appreciate it.
>
> Thanks!
>
>
> Diffs
> -----
>
> tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/CatalogServer.java dc70305
> tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java 25caf13
> tajo-common/src/main/java/org/apache/tajo/util/ProtoBufUtil.java PRE-CREATION
> tajo-core/tajo-core-backend/pom.xml fce9925
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoContainerProxy.java e44947e
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoMaster.java 7dcc55f
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoMasterClientService.java 856566a
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoMasterService.java c3a0829
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/metrics/WorkerResourceMetricsGaugeSet.java 1924041
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryInProgress.java 6dc437f
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryInfo.java 058352f
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryJobManager.java 09d1d26
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryMaster.java 8a8772d
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/TajoRMContext.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/TajoResourceTracker.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/TajoWorkerContainer.java 5ac6e39
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/TajoWorkerResourceManager.java 2c3572c
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/Worker.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerEvent.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerEventType.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerLivelinessMonitor.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerReconnectEvent.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerResource.java e8c9a9e
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerResourceManager.java 1ce2c9f
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerState.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerStatus.java 21ad7d7
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/WorkerStatusEvent.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/YarnTajoResourceManager.java 80aab9b
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/worker/TajoResourceAllocator.java 222d355
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/worker/TajoWorker.java 2f763e3
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/worker/WorkerHeartbeatService.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/proto/ResourceTrackerProtocol.proto PRE-CREATION
> tajo-core/tajo-core-backend/src/main/proto/TajoMasterProtocol.proto 5e4088e
> tajo-core/tajo-core-backend/src/main/resources/webapps/admin/cluster.jsp 0600145
> tajo-core/tajo-core-backend/src/main/resources/webapps/admin/index.jsp f652ea5
> tajo-core/tajo-core-backend/src/main/resources/webapps/admin/query.jsp e3a356d
> tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/TajoTestingCluster.java 9c96e0e
> tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/master/TestTajoResourceManager.java 9504927
> tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/master/rm/TestTajoResourceManager.java PRE-CREATION
> tajo-rpc/src/main/java/org/apache/tajo/rpc/NettyServerBase.java 371f879
>
> Diff: https://reviews.apache.org/r/18496/diff/
>
>
> Testing
> -------
>
> mvn clean install
>
>
> Thanks,
>
> Hyunsik Choi
>
>