[LU-15565] lfs getstripe --yaml should use an array for components

• Type: Bug
• Resolution: Fixed
• Priority: Minor
• Fix Version/s: Lustre 2.16.0
• Severity: 3
• Assignee: Fred Dilger
• Reporter: John Hammond

Description

For composite files lfs getstripe --yaml uses a key for each component rather than an array:

$ lfs getstripe --yaml f0
  lcm_layout_gen:    1
  lcm_mirror_count:  2
  lcm_entry_count:   2
  component0:
    lcme_id:             65537
    lcme_mirror_id:      1
    lcme_flags:          init
    lcme_extent.e_start: 0
    lcme_extent.e_end:   EOF
    sub_layout:
      lmm_stripe_count:  1
      lmm_stripe_size:   1048576
      lmm_pattern:       raid0
      lmm_layout_gen:    0
      lmm_stripe_offset: 2
      lmm_objects:
      - l_ost_idx: 2
        l_fid:     0x100020000:0xd1:0x0

  component1:
    lcme_id:             131073
    lcme_mirror_id:      2
    lcme_flags:          init
    lcme_extent.e_start: 0
    lcme_extent.e_end:   EOF
    sub_layout:
      lmm_stripe_count:  1
      lmm_stripe_size:   1048576
      lmm_pattern:       raid0
      lmm_layout_gen:    0
      lmm_stripe_offset: 3
      lmm_objects:
      - l_ost_idx: 3
        l_fid:     0x100030000:0xd1:0x0

Using "components0..." instead of any array is poor YAML style and hinders the use of the output with common YAML tooling. If an array ("components") was used instead of numbered keys then we could do things like:

lfs getstripe --yaml f0 | yq '.components[] | select(.lcme_flags == "init,stale")'

Issue Links

• is related to: LU-5170 lfs usability
• is related to: LU-8417 setstripe -o does not work on directories
• is related to: LU-14156 mirror split to check for last in-sync early
• is related to: LU-16491 "lfs getdirstripe -y" doesn't print YAML formatted output

Comments

[Andreas Dilger]

The main reason to keep any of the existing output formats is compatibility with existing usage.

 

You are of course welcome to add a "--yaml2" output option that formats the layout differently, or a whole new llapi_* function that reads the file layout and parses directly into a c-yaml data structure, but those shouldn't break the existing usage.

[Nathan Rutman]

yeah, these were just my notes on what was wrong with the format, hadn't posted them to wc. 

#1 fine. just ugly.

#2, #3 were fixed with this patch.

#4 DoM components should indicate e.g. which MDT they live on, and reasonably the output could look like an OST stripe entry, per my example. 

#5 Just saying that proper yaml would represent this list as an array 

#6 lfs seems to output stripes in two forms, the single-line form with {} and the multi-line form. For the single-line form you need the quotes around the fid to demarcate the keys/values. (And it doesn't hurt the multi-line form).

Bigger issue: in my example version, it's trivial to visually see that there are two mirrored layouts. In the current form, with e.g. 2 pfl layouts mirrored, it looks like a list of 6 components, instead of 2 groups of 3. Everything is flat, and you have to search for lcme_mirror_id in each entry in this array to associate which components go together to form a copy of the data. Plus note that you can include per-mirror data (mirror state, component count) in my form.

Bigger issue B: Is there any reason to keep the non-yaml output format? YAML is supposed to be equally human and machine readable, seems dumb to have two different formats for that.

To be clear, I'm not demanding we fix these things, just noting for the future if anyone comes back to this ticket to fix more things, here's some things to think about.

[Andreas Dilger]

nrutman I just double checked the output from "lfs getstripe --yaml" at http://yaml-online-parser.appspot.com/ and https://zhwt.github.io/yaml-to-go/ and neither reported any issues with the current output format, though I'm not a YAML expert myself.

This patch was intended to fix the output format that was definitely not YAML compliant, with the minimal amount of change to avoid breaking other tools that parse this output.

Here was my old note, some of which is still valid.

Sorry, I don't see this comment posted anywhere?

1) sure, but "lcme_extent.e_end" isn't exactly "non-readable", and renaming these keys would be more complex and break other parsing (e.g. many test cases, which is reflective of external user scripts)
2) AFAICS, there are names for the arrays, "components" and "lmm_objects". "mirrors" is a bit of an artifact of how layouts are created, and there are no hard reasons that the components need to be split into RAID-0+1 mirrors vs. RAID-1+0 mirrors
3) I think this is true with the current output format?
4) I'm not sure what you mean? There are no stripes for DoM components
5) this looks like it could be fixed (the processed output looks like "lcme_flags": "init,stale" so it takes the flags as a single value)
6) My understanding is that the ':' only needs to be escaped/quoted if it is at the start/end of a word?

[Nathan Rutman]

Damn, sorry I missed the window to comment on this one. (Of course, the reason I noticed is that my parser of the old "yaml" broke.)

Is there any reason to keep the non-yaml output format? YAML is supposed to be equally human and machine readable, seems dumb to have two different formats for that.

Here was my old note, some of which is still valid.

1. Yaml is supposed to be machine- and human-readable, and things like lcme_extent.e_end are c-code structure elements, not human-readable names.
2. arrays need a array name (like "mirrors" or "components")
3. list elements should be indented, and include a leading "-" before each element (e.g. lcme_id)
4. DoM component also needs a "stripes" entry.
5. Elements with multiple values need array forms (flags: [init, stale])
6. Colons (in FIDs) need to be escaped.

What it should look like (made-up example):

generation: 5
mirror_count: 2
mirror_state: "writing (unsynchronized)"
mirrors:
  - mirror_id: 0
    component_count: 2
    components:
      - component_id: 1
        flags: [init]
        extent: {start: 0, end: 1048576}
        layout:
          generation: 0
          pattern: mdt
          stripe_params: {size: 1048576}
          stripes:
            - 1: {mdt: 2, fid: "0x280000400:0xc59c95:0x0"}
      - component_id: 2
        flags: [init]
        extent: {start: 67108867, end: EOF}
        layout:
          generation: 0
          pattern: raid0
          stripe_params: {size: 4194304, count: 2, offset: 0}
          stripes:
            - 1: {ost: 0, fid: "0x280000400:0xc59c92:0x0"}
            - 2: {ost: 1, fid: "0x2c0000400:0xc583b3:0x0"}
  - mirror_id: 1
    component_count: 1
    components:
      - component_id: 1
        flags: [init, stale]
        extent: {start: 0, end: EOF}
        layout:
          generation: 0
          pattern: raid0
          pool: disk
          stripe_params: {size: 4194304, count: 2, offset: 0}
          stripes:
            - 1: {ost: 10, fid: "0x2c0000400:0xc583b3:0x0"}
 

 

[Andreas Dilger]

Landed for 2.16.0

[Gerrit Updater]

"Oleg Drokin <[e-mail]>" merged in patch https://review.whamcloud.com/c/fs/lustre-release/+/55311/
Subject: LU-15565 utils: updated lfs getstripe yaml format
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 109b5499d3fdc42614497a70a67764f90f00f223

[Gerrit Updater]

"Frederick Dilger <[e-mail]>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/55311
Subject: LU-15565 utils: updating lfs getstripe --yaml format
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 276176ff77593cfd85c6ab6351ef000ad8130945