qubole / rubix

Cache File System optimized for columnar formats and object stores
Apache License 2.0
183 stars 74 forks source link

new: usr: #162: Added Hadoop2ClusterManagerV2 implememtation of hadoop cluster manager for correct consistent hashing logic #163

Closed abhishekdas99 closed 6 years ago

coveralls commented 6 years ago

Pull Request Test Coverage Report for Build 160


Changes Missing Coverage Covered Lines Changed/Added Lines %
rubix-core/src/main/java/com/qubole/rubix/core/utils/DummyClusterManager.java 0 2 0.0%
rubix-hadoop2/src/main/java/com/qubole/rubix/hadoop2/Hadoop2ClusterManager.java 4 6 66.67%
rubix-presto/src/main/java/com/qubole/rubix/presto/PrestoClusterManager.java 0 2 0.0%
rubix-spi/src/main/java/com/qubole/rubix/spi/ClusterManager.java 0 6 0.0%
rubix-hadoop2/src/main/java/com/qubole/rubix/hadoop2/Hadoop2ClusterManagerUtil.java 30 49 61.22%
rubix-hadoop2/src/main/java/com/qubole/rubix/hadoop2/Hadoop2ClusterManagerV2.java 38 57 66.67%
<!-- Total: 72 122 59.02% -->
Totals Coverage Status
Change from base Build 155: 1.0%
Covered Lines: 573
Relevant Lines: 2377

💛 - Coveralls
vrajat commented 6 years ago

One big question is composition vs inheritance between the two implementations of Hadoop Cluster Managers. I probably prefer inheritance in this specific case because tests that are valid for V1 is also valid for V2 except for a few scenarios where you want different scenarios. If you choose composition, then you have to repeat all the tests.

abhishekdas99 commented 6 years ago

@vrajat Yes I thought about it. First the tests are same in terms of checking the condition. The output of the tests are going to be different. For example, for testUnhealthyNodeCluster_decommissioned, the old HadoopClusterManager will return 1 node whereas the new implementation is going to return 2 nodes. Both the clustermanagers are extending the abstract class and quite a few implementations are different for these two cases such as initialize, getNextRunningNodeIndex and getPreviousRunningNodeIndex. The common methods which can be used as utility methods have been extracted out to a different Utility class. Same logic has been applied to TestClasses as well.