Auto distribute count based on Max brick size #1386
Conversation
ghost
assigned 
| @@ -102,10 +104,16 @@ func smartVolumeCreate(cmd *cobra.Command, args []string) { | |||
| failure("Invalid File Size specified", nil, 1) | |||
| } | |||
|
|
|||
| maxBrickSize, err := sizeToBytes(flagCreateMaxBrickSize) | |||
There was a problem hiding this comment.
if flagCreateMaxBrickSize has changed then do size calculation?
There was a problem hiding this comment.
sizeToBytes returns zero when input is empty, no conversion/calculation done
| @@ -407,6 +407,54 @@ func testSmartVolumeDistributeDisperse(t *testing.T) { | |||
| checkZeroLvs(r) | |||
| } | |||
|
|
|||
| func testSmartVolumeAutoDistributeReplicate(t *testing.T) { | |||
There was a problem hiding this comment.
can you add some negative test cases?
There was a problem hiding this comment.
what kind of negative tests I can add? negative max-brick-size or max-brick-size is greater than request size?
There was a problem hiding this comment.
kind of invalid minimum and maximum values for max-brick-size
While creating auto provisioned volume, support added to automatically
calculate the distribute count based on max brick size specified in
the request.
For example, below command creates 2x3(Distributed replicate) volume
```
glustercli volume create gv1 --replica 3 --size 1G \
--max-brick-size 512M
```
Fixes: #999
Signed-off-by: Aravinda VK <avishwan@redhat.com>
|
@Madhu-1 please review |
|
Found one more issue in Volume create flow, opened issue #1389 |
| @@ -345,3 +341,12 @@ func ExtendThinpool(expansionTpSizePerBrick uint64, vgName string, tpName string | |||
| err := utils.ExecuteCommandRun("lvextend", fmt.Sprintf("-L+%dB", expansionTpSizePerBrick), fmt.Sprintf("/dev/%s/%s", vgName, tpName)) | |||
| return err | |||
| } | |||
|
|
|||
| // NormalizeSize converts the value to multiples of 512 | |||
There was a problem hiding this comment.
can we make it unexported function?
There was a problem hiding this comment.
this is used in glusterd2/bricksplanner as well
|
|
||
| // If max Brick size is specified then decide distribute | ||
| // count and Volume Size based on Volume Type | ||
| if req.MaxBrickSize > 0 && req.Size > req.MaxBrickSize { |
There was a problem hiding this comment.
what happens if req.MaxBrickSize if greater than req.Size?
There was a problem hiding this comment.
It will be same as not passing that option
There was a problem hiding this comment.
as this is an input request we need to return an error to the user.
There was a problem hiding this comment.
I don't think that is an error. It is satisfying the user's condition of max-brick-size.
| SubvolZonesOverlap: true, | ||
| } | ||
| volinfo, err = client.VolumeCreate(createReq) | ||
| r.Nil(err) |
There was a problem hiding this comment.
Shouldn't this be r.NotNil (err) , how is this going to succeed if the MaxBrickSize is greater than Size ?
There was a problem hiding this comment.
Ignore this now, basically this will allocate only 1 sub vol of 20 MiB.
There was a problem hiding this comment.
MaxBrickSize > VolumeSize condition will always satisfy. So this satisfies user's condition.
| SubvolZonesOverlap: true, | ||
| } | ||
| volinfo, err = client.VolumeCreate(createReq) | ||
| r.Nil(err) |
There was a problem hiding this comment.
Don't we need a similar test for arbiter volume as well?
There was a problem hiding this comment.
max subvol size is derived based on volume type and req.MaxBrickSize. This calculation is same for Arbiter/Replica. Special handling only for Disperse volume.
Two ways we can distribute now,
Max brick size can be added in Storage class definition irrespective of Volume sizes. That will be passed with every volume create request. (For example, Currently |
|
@aravindavk I'm not sure if distributeCount and maxBricksSize parameters input value should co-exist but if they do then I agree that maxBrickSize should take precedence as distribute count should be more internal and should be managed intelligently by GD2. |
|
@aravindavk @Madhu-1 don't we need a respective change in the gluster-csi-driver once this gets in? If so, I think we need an issue tracking this. |
|
@atinmu currently we are just making use of size and volume type in volume create,we will make required changes in CSI driver. |
|
Fixes: #1398 |
|
Opened issue to support these options in CSI driver gluster/gluster-csi-driver#120 |
|
retest this please |
While creating auto provisioned volume, support added to automatically
calculate the distribute count based on max brick size specified in
the request.
For example, below command creates 2x3(Distributed replicate) volume
Fixes: #999
Signed-off-by: Aravinda VK avishwan@redhat.com