Adds proposal for auto partitioning in LSO #486

rohan47 · 2020-09-25T13:04:25Z

This PR adds a design proposal for the automatic partitioning of local devices and provisioning PVs for those partitions in LSO.

Signed-off-by: rohan47 rohgupta@redhat.com

rohan47 · 2020-09-25T13:19:10Z

/assign @travisn @gnufied @leseb @jarrpa @rohantmp @sp98

rohan47 · 2020-09-25T13:50:02Z

/assign @jsafrane

enhancements/local-storage/automatic-partition.md

jarrpa · 2020-09-25T13:49:19Z

enhancements/local-storage/automatic-partition.md

+For performance reasons the user will want to use partitions.
+On NVMe devices, some of the devices are capable of delevering very high IOPS per device. An OSD tends to bottleneck long before it reaches anywhere close to the device IOPS limit.
+So with single OSD per NVMe device, there will be a big gap between measured or advertised capabilities of these devices and what you can get with Ceph/OCS on top of them.


What is an "OSD"? Why does the LSO or even OpenShift care about these things?

What I'm saying is that you need to phrase this in a way that presents a broader use-case that is not specific to OCS.

enhancements/local-storage/automatic-partition.md

travisn · 2020-09-25T20:00:35Z

enhancements/local-storage/automatic-partition.md

+- Policy for wiping device to clean the partitions
+- Mixed method of PV creation is not supported. i.e., using some devices entirely and some with partitoins in a single LocalVolumeSet is not supported.
+
+### Risks and Mitigations


Not sure I understand the point of the risks section. You are pointing out that we might partition the wrong disk if the user gives the wrong device filter? This risk doesn't seem any different from risks of the LocalVolumeSet already has, so maybe we can just refer back to that doc if it has a risks section.

enhancements/local-storage/automatic-partition.md

travisn · 2020-09-25T20:08:42Z

enhancements/local-storage/automatic-partition.md

+  // TotalProvisionedPartitionCount is the count of the total partitions over which the PVs has been provisioned
+  // This will be diffrent for TotalProvisionedDeviceCount as TotalProvisionedDeviceCount is
+  //the count of the actual devices
+  TotalProvisionedPartitionCount *int32 `json:"totalProvisionedPartitionCount,omitempty"`


How will this be updated? If each node updates the status, we could get conflicts. Was totalProvisionedDeviceCount implemented? It would have the same issue.

totalProvisionedDeviceCount is implemented, looking into the implementation details.

Also, updating this across daemon restart may be complicated.

I would ask maybe a stupid question: why is this value important? How is it different to an user if LSO matches a disk with pre-existing paritionions and only consumes them, to LSO actually partitioning empty disk?

Okay, will remove this part.

enhancements/local-storage/automatic-partition.md

travisn · 2020-09-25T20:17:59Z

enhancements/local-storage/automatic-partition.md

+
+As per the above example the administrator wants partitions of size 30G.
+Lets assume that there are 5 devices of 100G  that are detected and are fit for partitoning, so each device will be partitioned into 3 parts of 30G each and 10G of each device will remain unpartitioned.
+Right now we are keeping it simple by partitioning by size and will move to advanced partitioning policy in the further releases to avoid having unusable disk space.


I would think the admins would most often carve up the devices in sizes that make sense for their disks, so this may not even be a common issue. Sounds good for a future release though if it's common enough.

openshift-ci-robot · 2020-10-05T08:48:44Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: rohan47
To complete the pull request process, please assign jsafrane after the PR has been reviewed.
You can assign the PR to them by writing /assign @jsafrane in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

enhancements/local-storage/automatic-partition.md

leseb · 2020-10-05T12:45:47Z

enhancements/local-storage/automatic-partition.md

+
+type PartitioningSpec struct {
+  // PartSize determines the exact size of each partition.
+  PartSize *resource.Quantity `json:"partSize"`


I feel like we need to have a Count as well. It looks like a waste to use the entire device all the time.

If you're going to partition the disk, seems like you'd want to partition it all at the same time since the unpartitioned space wouldn't be usable. There could certainly be other options to indicate how partitions should be created, but I want to be clear about the scenarios to keep it simple.

If you're going to partition the disk, seems like you'd want to partition it all at the same time since the unpartitioned space wouldn't be usable. There could certainly be other options to indicate how partitions should be created, but I want to be clear about the scenarios to keep it simple.

@travisn
I didn't understand by partition it all at the same time.
Do you suggest that we should not keep count for the current scenario?

I am in favor of count as it is also a simple parameter to partition the disks.

I guess the rationale is why should we use the entire device? If we device is an NVMe it could certainly be used by other applications. I don't think we (can?) restrict disks just for Ceph.

So if we decide that Ceph needs 3 partitions of 40GiB (making this up) then the rest can still be used by other applications.

The result of the partitioning will be that we have a PV created on each partition. We don't know who the consumer is, it might be Ceph or it might be some other app, depending on who requests the volumes. So why wouldn't we partition the whole disk? If you're partitioning it for PVs, do we need to allow apps to use it for non-PVs? I didn't think so, but deciding not to partition the whole device can certainly be part of the policy.

For the parameters, is this the suggested policy in this thread?

If only size is specified, create partitions for the whole disk all of that size. If there is space remaining, create a PV for the remaining amount.

If both count and size are specified, only create that number of partitions and leave the remaining disk unpartitioned.

That seems reasonable and simple to me.

For the parameters, is this the suggested policy in this thread?

* If only `size` is specified, create partitions for the whole disk all of that size. If there is space remaining, create a PV for the remaining amount. * If both `count` and `size` are specified, only create that number of partitions and leave the remaining disk unpartitioned.

My understanding of count is to create count no of partitions i.e., Size of the entire device/count
for example we have a device of total capacity of 128G and the value of count is 4, then 4 partitions of 32G each will be created.

That seems reasonable and simple to me.

Ah, I was stuck thinking about the size as being a required setting. What about this?

If only size is specified, create partitions for the whole disk all of that size. If there is space remaining, create a PV for the remaining amount.

If only count is specified, divide the disk up into that many partitions and calculate the size.

If both count and size are specified, only create that number of partitions of the desired size and leave the remaining disk unpartitioned.

enhancements/local-storage/automatic-partition.md

jsafrane · 2020-11-02T15:32:24Z

enhancements/local-storage/automatic-partition.md

+
+### Workflow for Local PV creation via `LocalVolumeSet CR`
+- The admin decides they want to create partitions on some devices in the cluster
+- He creates a LocalVolumeSet CR that includes the partitioning policy


Probably: LocalVolumeSet matching the disk they want to partition.

enhancements/local-storage/automatic-partition.md

jsafrane · 2020-11-02T15:36:10Z

enhancements/local-storage/automatic-partition.md

+  // TotalProvisionedPartitionCount is the count of the total partitions over which the PVs has been provisioned
+  // This will be diffrent for TotalProvisionedDeviceCount as TotalProvisionedDeviceCount is
+  //the count of the actual devices
+  TotalProvisionedPartitionCount *int32 `json:"totalProvisionedPartitionCount,omitempty"`


Also, updating this across daemon restart may be complicated.

I would ask maybe a stupid question: why is this value important? How is it different to an user if LSO matches a disk with pre-existing paritionions and only consumes them, to LSO actually partitioning empty disk?

jsafrane · 2020-11-02T15:38:26Z

enhancements/local-storage/automatic-partition.md

+
+## Partitioning policy
+
+* Size: If only size is specified, create partitions for the whole disk all of that size. If there is space remaining, create a PV for the remaining amount.


I would make it consistent with "If both count and size are specified" - only create partitions of the desired size and leave the remaining disk unpartitioned.

Do you mean if only size is specified then create partitions of desired size and leave the remaining disk unpartitioned?

Signed-off-by: rohan47 <rohgupta@redhat.com>

openshift-bot · 2021-04-05T18:29:11Z

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

openshift-bot · 2021-05-05T22:33:57Z

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

openshift-bot · 2021-06-05T01:24:06Z

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

openshift-ci · 2021-06-05T01:24:14Z

@openshift-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

rohan47 · 2021-06-08T13:52:39Z

/reopen

openshift-ci · 2021-06-08T13:52:47Z

@rohan47: Reopened this PR.

In response to this:

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci · 2021-06-08T13:52:57Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please ask for approval from jsafrane after the PR has been reviewed.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-bot · 2021-07-08T19:49:45Z

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

openshift-ci · 2021-07-08T19:49:53Z

@openshift-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot requested review from eparis and markmc September 25, 2020 13:04

rohan47 force-pushed the auto-partition branch 3 times, most recently from dbf2c5e to 0623a2d Compare September 25, 2020 13:15

openshift-ci-robot assigned gnufied, jarrpa, leseb, rohantmp, sp98 and travisn Sep 25, 2020

openshift-ci-robot assigned jsafrane Sep 25, 2020

jarrpa suggested changes Sep 25, 2020

View reviewed changes

travisn suggested changes Sep 25, 2020

View reviewed changes

rohan47 force-pushed the auto-partition branch from 0623a2d to 6bba608 Compare October 5, 2020 08:48

leseb suggested changes Oct 5, 2020

View reviewed changes

rohan47 force-pushed the auto-partition branch from 6bba608 to 47ef026 Compare October 9, 2020 11:56

rohan47 force-pushed the auto-partition branch from 47ef026 to 5b9f181 Compare October 19, 2020 18:49

rohan47 force-pushed the auto-partition branch from 5b9f181 to 20797ee Compare October 30, 2020 13:35

jsafrane reviewed Nov 2, 2020

View reviewed changes

rohan47 force-pushed the auto-partition branch 2 times, most recently from bb80b8e to 38c19c8 Compare November 20, 2020 13:25

Adds proposal for auto partitioning in LSO

51bf7d5

Signed-off-by: rohan47 <rohgupta@redhat.com>

rohan47 force-pushed the auto-partition branch from 38c19c8 to 51bf7d5 Compare November 20, 2020 13:49

rohan47 marked this pull request as draft January 5, 2021 14:20

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 5, 2021

openshift-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 5, 2021

openshift-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels May 5, 2021

openshift-ci bot closed this Jun 5, 2021

openshift-ci bot reopened this Jun 8, 2021

openshift-ci bot closed this Jul 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds proposal for auto partitioning in LSO #486

Adds proposal for auto partitioning in LSO #486

rohan47 commented Sep 25, 2020

rohan47 commented Sep 25, 2020

rohan47 commented Sep 25, 2020

jarrpa Sep 25, 2020

travisn Sep 25, 2020

travisn Sep 25, 2020

rohan47 Oct 9, 2020

jsafrane Nov 2, 2020

rohan47 Nov 20, 2020

travisn Sep 25, 2020

openshift-ci-robot commented Oct 5, 2020

leseb Oct 5, 2020

travisn Oct 8, 2020

rohan47 Oct 9, 2020 •

edited

Loading

rohan47 Oct 9, 2020 •

edited

Loading

leseb Oct 9, 2020

travisn Oct 9, 2020

travisn Oct 9, 2020

rohan47 Oct 9, 2020

travisn Oct 9, 2020

rohan47 Oct 9, 2020

jsafrane Nov 2, 2020

jsafrane Nov 2, 2020

jsafrane Nov 2, 2020

rohan47 Nov 20, 2020

jsafrane Nov 20, 2020

openshift-bot commented Apr 5, 2021

openshift-bot commented May 5, 2021

openshift-bot commented Jun 5, 2021

openshift-ci bot commented Jun 5, 2021

rohan47 commented Jun 8, 2021

openshift-ci bot commented Jun 8, 2021

openshift-ci bot commented Jun 8, 2021

openshift-bot commented Jul 8, 2021

openshift-ci bot commented Jul 8, 2021


		## Partitioning policy

		* Size: If only size is specified, create partitions for the whole disk all of that size. If there is space remaining, create a PV for the remaining amount.

Adds proposal for auto partitioning in LSO #486

Adds proposal for auto partitioning in LSO #486

Conversation

rohan47 commented Sep 25, 2020

rohan47 commented Sep 25, 2020

rohan47 commented Sep 25, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openshift-ci-robot commented Oct 5, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rohan47 Oct 9, 2020 • edited Loading

Choose a reason for hiding this comment

rohan47 Oct 9, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openshift-bot commented Apr 5, 2021

openshift-bot commented May 5, 2021

openshift-bot commented Jun 5, 2021

openshift-ci bot commented Jun 5, 2021

rohan47 commented Jun 8, 2021

openshift-ci bot commented Jun 8, 2021

openshift-ci bot commented Jun 8, 2021

openshift-bot commented Jul 8, 2021

openshift-ci bot commented Jul 8, 2021

rohan47 Oct 9, 2020 •

edited

Loading

rohan47 Oct 9, 2020 •

edited

Loading