-
Notifications
You must be signed in to change notification settings - Fork 275
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add HelixVcrCluster and HelixVcrClusterFactory. #1158
Conversation
There will be another PR for ReplicationEngine to dynamically add/remove partition. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First round of review comments. I need to understand the Helix messaging a bit better.
ambry-cloud/src/main/java/com.github.ambry.cloud/HelixVcrCluster.java
Outdated
Show resolved
Hide resolved
ambry-cloud/src/main/java/com.github.ambry.cloud/HelixVcrCluster.java
Outdated
Show resolved
Hide resolved
ambry-cloud/src/main/java/com.github.ambry.cloud/HelixVcrCluster.java
Outdated
Show resolved
Hide resolved
ambry-cloud/src/main/java/com.github.ambry.cloud/HelixVcrCluster.java
Outdated
Show resolved
Hide resolved
ambry-cloud/src/main/java/com.github.ambry.cloud/HelixVcrStateModel.java
Show resolved
Hide resolved
ambry-cloud/src/main/java/com.github.ambry.cloud/HelixVcrStateModel.java
Show resolved
Hide resolved
ambry-utils/src/test/java/com.github.ambry.utils/HelixControllerManager.java
Outdated
Show resolved
Hide resolved
ambry-utils/src/test/java/com.github.ambry.utils/HelixControllerManager.java
Show resolved
Hide resolved
Codecov Report
@@ Coverage Diff @@
## master #1158 +/- ##
============================================
- Coverage 69.84% 69.78% -0.06%
- Complexity 5316 5332 +16
============================================
Files 420 425 +5
Lines 32547 32644 +97
Branches 4139 4147 +8
============================================
+ Hits 22732 22782 +50
- Misses 8695 8724 +29
- Partials 1120 1138 +18
Continue to review full report at Codecov.
|
private final HelixManager manager; | ||
private final HelixAdmin helixAdmin; | ||
private final Map<String, PartitionId> partitionIdMap; | ||
private final Set<PartitionId> assignedPartitionIds = new ConcurrentHashMap<PartitionId, Boolean>().newKeySet(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can just make this ConcurrentHashMap.newKeySet()
; it's a static method
* Helix Based VCR Cluster. | ||
*/ | ||
public class HelixVcrCluster implements VirtualReplicatorCluster { | ||
private final static Logger logger = LoggerFactory.getLogger(HelixVcrCluster.class); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private static final
ambry-cloud/src/main/java/com.github.ambry.cloud/HelixVcrCluster.java
Outdated
Show resolved
Hide resolved
ambry-api/src/main/java/com.github.ambry/clustermap/VirtualReplicatorCluster.java
Outdated
Show resolved
Hide resolved
ambry-cloud/src/test/java/com.github.ambry.cloud/HelixVcrClusterTest.java
Outdated
Show resolved
Hide resolved
ambry-cloud/src/main/java/com.github.ambry.cloud/HelixVcrStateModelFactory.java
Outdated
Show resolved
Hide resolved
ambry-cloud/src/main/java/com.github.ambry.cloud/StaticVcrCluster.java
Outdated
Show resolved
Hide resolved
ambry-cloud/src/test/java/com.github.ambry.cloud/HelixVcrClusterTest.java
Outdated
Show resolved
Hide resolved
ambry-cloud/src/test/java/com.github.ambry.cloud/HelixVcrClusterTest.java
Outdated
Show resolved
Hide resolved
A few minor style comments, we can merge after addressing those. |
Add HelixVcrCluster and HelixVcrClusterFactory.