Support formatting bricks in a brick set differently based on brick properties #1306
Conversation
apps/glusterfs/brick_allocate.go
Outdated
| @@ -521,6 +523,8 @@ func (bp *StandardBrickPlacer) Replace( | |||
| newDevices := make([]*DeviceEntry, bs.SetSize) | |||
| for i := 0; i < bs.SetSize; i++ { | |||
| if i == index { | |||
| if index == arbiter_index { | |||
There was a problem hiding this comment.
I am sorry, this was extra in my patch.
There was a problem hiding this comment.
np. Like I mentioned in the desc. this will need to be squshed and cleaned up a bit prior to merging.
|
TODO: Add functional test to verify the formatting differences on the brick file systems. |
|
retest this please |
|
Centos-ci is failing in the same place. That's suspicious, I will investigate. |
phlogistonjohn
force-pushed
the
jjm-xfs-plumbing
branch
from
11ba337
to
e6e96b3
Compare
phlogistonjohn
changed the title
phlogistonjohn
force-pushed
the
jjm-xfs-plumbing
branch
from
9635c15
to
c074a9f
Compare
|
retest this please |
| @@ -204,6 +204,8 @@ func tryAllocateBrickOnDevice( | |||
| logger.Debug( | |||
| "Unable to place a brick of size %v & factor %v on device %v", | |||
| brickSize, snapFactor, device.Info.Id) | |||
| } else { | |||
| brick.SubType = NormalSubType | |||
There was a problem hiding this comment.
This seems to be an initialization to a default value. Why initialize here instead of in brick_entry.go:NewBrickEntry()?
There was a problem hiding this comment.
We could set the sub type in NewBrickEntry but the only code that "knows" the correct subtype is the placer. Normal is not the default, Unknown sub type is the default value.
The lifecycle of the new field goes something like:
- All old entries pulled from the db will get an "unknown" sub type
- Placers that create a new brick entry set the correct sub type
- The new sub type values will get stored in the db
- The create brick code path uses the value to determine the formatting for new brick fs
This approach was designed not to need a specific upgrade function. However, adding a flag to indicate that this db can contain subtypes seems reasonable.
There was a problem hiding this comment.
I'd also like to note this is similar to the approach I took for TpName & LvName to the brick entry which does not have a db flag.
Add support to the brick request to tell the executor that a brick file system should be formatted in one fashion or another. Define a normal and arbiter formatting type and use the arbiter formatting type to create a file system that allows for more inodes. Pair-Programmed-With: John Mulligan <jmulligan@redhat.com> Signed-off-by: Raghavendra Talur <rtalur@redhat.com> Signed-off-by: John Mulligan <jmulligan@redhat.com>
Add a new brick entry field for the brick's "sub-type", which is a semi-opaque value that can be used to determine any special formatting for that brick. Signed-off-by: John Mulligan <jmulligan@redhat.com>
Placers must determine what the brick's subtype is because it is the role of the placer to carve out the bricks from the underlying storage and they have the knowledge needed to make the determination of the subtype. Signed-off-by: John Mulligan <jmulligan@redhat.com>
When the brick subtype is arbiter specify an arbiter formatting for the executor. If the subtype is normal use normal formatting. If the subtype is unknown and we're creating a brick request to be used for a new brick, panic to detect misuse of the api. Signed-off-by: John Mulligan <jmulligan@redhat.com>
Add checks to some of the arbiter functional tests to verify that the inode percentage was set differently on the arbiter brick than on the other bricks. Signed-off-by: John Mulligan <jmulligan@redhat.com>
Signed-off-by: John Mulligan <jmulligan@redhat.com>
phlogistonjohn
force-pushed
the
jjm-xfs-plumbing
branch
from
62a20f8
to
15572de
Compare
|
DB feature flag added. |
|
@obnoxxx @phlogistonjohn Changes look good to me. |
|
@obnoxxx have I sufficiently handled your requests? |
|
retest this please |
What does this PR achieve? Why do we need it?
Support formatting arbiter bricks differently than normal bricks (xfs options) with an eye towards making it possible to make other formatting differences in the future.
Does this PR fix issues?
Fixes #1296
Notes for the reviewer
This is a collaboration of @raghavendra-talur and me, some of these patches should probably eventually get squashed, but I have not done so yet.