Multiple disk with capacity based scheduling(Phase3) #121
Conversation
types/setting.go
Outdated
|
|
||
| SettingDefinitionStorageOverProvisioningPercentage = SettingDefinition{ | ||
| DisplayName: "Storage Over Provisioning Percentage", | ||
| Description: "The setting used for multiple disk scheduling, set the upper bound of disks", |
There was a problem hiding this comment.
The over-provisioning percentage defines how much storage can be allocated relative to the hard drive's capacity
types/setting.go
Outdated
|
|
||
| SettingDefinitionStorageMinimalAvailablePercentage = SettingDefinition{ | ||
| DisplayName: "Storage Minimal Available Percentage", | ||
| Description: "The setting used for multiple disk scheduling, set the bottom bound of disks", |
There was a problem hiding this comment.
If one disk's available capacity to it's maximum capacity in % is less than the minimal available percentage, the disk would become unschedulable until more space freed up.
controller/node_controller.go
Outdated
| @@ -240,6 +288,12 @@ func (nc *NodeController) syncDiskStatus(node *longhorn.Node) error { | |||
| return err | |||
| } | |||
|
|
|||
| // get settings of StorageOverProvisioningPercentage and StorageMinimalAvailablePercentage | |||
| overProvisioningPercentage, minimalAvailablePercentage, err := nc.ds.GetSettingsForDisk() | |||
There was a problem hiding this comment.
It's too easy to mess up with returning two identical types of variables. It's better to either pass in as a typed struct or separate them into two functions. I think two functions are better here.
We can replace the datastore function with GetSettingAsInt(). It can check if the type is indeed int before return.
controller/node_controller.go
Outdated
| @@ -266,6 +320,16 @@ func (nc *NodeController) syncDiskStatus(node *longhorn.Node) error { | |||
| diskStatus.StorageAvailable = diskInfo.StorageAvailable | |||
| } | |||
|
|
|||
| // check disk pressure | |||
| if overProvisioningPercentage == 0 || | |||
There was a problem hiding this comment.
The calculation for overProvisioningPercentage and minimalAvailablePercentage should be already covered by the formula. Shouldn't need special case handling for them (except dividing zero if that happens).
controller/node_controller.go
Outdated
| if overProvisioningPercentage == 0 || | ||
| minimalAvailablePercentage == 100 || | ||
| diskInfo == nil || | ||
| diskInfo.StorageAvailable < disk.StorageMaximum*minimalAvailablePercentage/100 { |
There was a problem hiding this comment.
Use <= instead of < so you can cover minimalAvailablePercentage == 100.
datastore/longhorn.go
Outdated
| @@ -692,3 +693,26 @@ func tagNodeLabel(replica *longhorn.Replica) error { | |||
| metadata.SetLabels(labels) | |||
| return nil | |||
| } | |||
|
|
|||
| func (s *DataStore) GetSettingsForDisk() (int64, int64, error) { | |||
There was a problem hiding this comment.
As I said, replace with GetSettingAsInt() (int64, error)
JacieChao
force-pushed
the
dev
branch
2 times, most recently
from
39a3701
to
96ca34e
Compare
|
@yasker Fixed manager |
|
LGTM. Will merge when test is ready. |
…blePercentage to Settings
Add StorageOverProvisioningPercentage and StorageMinimalAvailable to update disk state
Add StorageOverProvisioningPercentage and StorageMinimalAvailablePercentage and disk state to scheduling.
|
@yasker Rebased master and add scheduling category to settings |
Phase 3(longhorn/longhorn#47):
Stateto DiskStatusStorageOverProvisioningPercentageandStorageMinimalAvailablePercentageto settingsStorageOverProvisioningPercentageandStorageMinimalAvailablePercentagesettings and updateDiskStateStorageOverProvisioningPercentageandStorageMinimalAvailablePercentagefrom settingsDiskStateand over-provisioning and minimal available storage settingsintegration test for Phase3(longhorn/longhorn-tests#81)