HDDS-9273. Set CapacityVolumeChoosingPolicy as default VolumeChoosingPolicy.#5285
HDDS-9273. Set CapacityVolumeChoosingPolicy as default VolumeChoosingPolicy.#5285sumitagrawl merged 2 commits intoapache:masterfrom
Conversation
swamirishi
left a comment
swamirishi
left a comment
There was a problem hiding this comment.
@sadanand48 Thanks for the patch. Overall changes look good. Had nitpicky changes.
| volumeChoosingPolicy = conf.getClass( | ||
| HDDS_DATANODE_VOLUME_CHOOSING_POLICY, RoundRobinVolumeChoosingPolicy | ||
| .class, VolumeChoosingPolicy.class).newInstance(); | ||
| volumeChoosingPolicy = VolumeChoosingPolicyFactory.getPolicy(conf); |
There was a problem hiding this comment.
Why is there a need for a volumeChoosingFactory here? Can't we just add the DEFAULT_VOLUME_CHOOSING_POLICY in a constant class?
There was a problem hiding this comment.
Classes inside org.apache.hadoop.ozone.container.common.volume can't be accessed in HDDSConfigKeys as the common module doesn't depend on the container-service module. The other way is to hardcode the class name as a string but this might pose a problem if some one changes the location of the class.
Just checked Even Container placement policy uses a factory class to get its impls.
sumitagrawl
left a comment
sumitagrawl
left a comment
There was a problem hiding this comment.
@sadanand48 Thanks for working over this, LGTM +1
4 participants
What changes were proposed in this pull request?
Current default is round robin which won't take into account the space on the volume while creating/replicating containers which can cause allocate block failures (although they would be retried) or lead to small containers. It is better to make CapacityVolumeChoosingPolicy as default.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-9273
How was this patch tested?
Unit test