----------------------------------------------------------- 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 > >
