Uploaded image for project: 'Lustre'
  1. Lustre
  2. LU-13031

store JobID of program that created file in inodes at create time

Details

    • 9223372036854775807

    Description

      It would be useful to store the JobID that created a file in an xattr on the MDT inode. This would allow tracking the source of files after the fact, and can help users with the provenance of their file data (e.g. which run the files came from, which can then be used to determine runtime parameters used, whether the run was successful or not, etc.).

      It isn't clear whether this would be needed on the OST inodes as well, but I think initially the MDT inode would be enough, as this would be the only xattr visible to users. The OST xattr would only be accessible for low-level scanning tools.

      Attachments

        Issue Links

          Activity

            [LU-13031] store JobID of program that created file in inodes at create time

            "Oleg Drokin <green@whamcloud.com>" merged in patch https://review.whamcloud.com/c/fs/lustre-release/+/52095/
            Subject: LU-13031 tests: skip sanity/test_205h,205i in interop
            Project: fs/lustre-release
            Branch: master
            Current Patch Set:
            Commit: a6df532162556028c2ab9b974989fc0cca68d4fe

            gerrit Gerrit Updater added a comment - "Oleg Drokin <green@whamcloud.com>" merged in patch https://review.whamcloud.com/c/fs/lustre-release/+/52095/ Subject: LU-13031 tests: skip sanity/test_205h,205i in interop Project: fs/lustre-release Branch: master Current Patch Set: Commit: a6df532162556028c2ab9b974989fc0cca68d4fe

            "Thomas Bertschinger <bertschinger@lanl.gov>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/52095
            Subject: LU-13031 tests: skip sanity/test_205h,205i in interop
            Project: fs/lustre-release
            Branch: master
            Current Patch Set: 1
            Commit: 1532029b43cfdc41b6747ea2d4076f12d28e8be0

            gerrit Gerrit Updater added a comment - "Thomas Bertschinger <bertschinger@lanl.gov>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/52095 Subject: LU-13031 tests: skip sanity/test_205h,205i in interop Project: fs/lustre-release Branch: master Current Patch Set: 1 Commit: 1532029b43cfdc41b6747ea2d4076f12d28e8be0
            adilger Andreas Dilger added a comment - - edited

            The version_code helper supports 4-component version numbers, so since this is being done after the landing it is helpful to include the full version from "git describe" of the commit hash of the patch itself. If it were before landing it could use the 4-component version of the patch before landing, even though it is slightly inaccurate..

            It is also helpful to include a brief description of why the test is being skipped. Please see some examples in patch https://review.whamcloud.com/52009 "LU-16097 tests: skip quota subtests in interop".

            adilger Andreas Dilger added a comment - - edited The version_code helper supports 4-component version numbers, so since this is being done after the landing it is helpful to include the full version from " git describe " of the commit hash of the patch itself. If it were before landing it could use the 4-component version of the patch before landing, even though it is slightly inaccurate.. It is also helpful to include a brief description of why the test is being skipped. Please see some examples in patch https://review.whamcloud.com/52009 " LU-16097 tests: skip quota subtests in interop ".

            Yes, you've got this exactly right.  That's what interop is and that check is what's needed.

            Generally you'd just use 2.15.57, the tag that was live when the patch was merged.  We don't do interop testing between different development versions, just between dev versions and older full versions.  It's also fine if in some narrow scenario we skip a test that would pass.

            So that MDS version check you suggested for 2.15.57 is what's needed.  Oops on the reviewers (ie, including me) for not noting that at the time.

            paf0186 Patrick Farrell added a comment - Yes, you've got this exactly right.  That's what interop is and that check is what's needed. Generally you'd just use 2.15.57, the tag that was live when the patch was merged.  We don't do interop testing between different development versions, just between dev versions and older full versions.  It's also fine if in some narrow scenario we skip a test that would pass. So that MDS version check you suggested for 2.15.57 is what's needed.  Oops on the reviewers (ie, including me) for not noting that at the time.

            Does interop refer to some testing that mixes different client and server versions? Is the failure occurring when the server version is older than the client and the MDS doesn't have this new change?

            If it just needs a version check like:

            (( $MDS1_VERSION >= $(version_code 2.15.??) )) || skip ...

            that should be simple enough, I can submit a fix for that.

            The version I get when I check out this commit and run `lctl get_param version` is 2.15.57. Is that the correct code to put in the version check? Or do I need one higher than that (since I guess you could check out a commit newer than the 2.15.57 tag but older than this change and its version would pass...). If I need one higher than 2.15.57, do I use 2.15.58, 2.16.0, or something else?

            bertschinger Thomas Bertschinger added a comment - Does interop refer to some testing that mixes different client and server versions? Is the failure occurring when the server version is older than the client and the MDS doesn't have this new change? If it just needs a version check like: (( $MDS1_VERSION >= $(version_code 2.15.??) )) || skip ... that should be simple enough, I can submit a fix for that. The version I get when I check out this commit and run `lctl get_param version` is 2.15.57. Is that the correct code to put in the version check? Or do I need one higher than that (since I guess you could check out a commit newer than the 2.15.57 tag but older than this change and its version would pass...). If I need one higher than 2.15.57, do I use 2.15.58, 2.16.0, or something else?

            The new maloo test fail in interop.

            simmonsja James A Simmons added a comment - The new maloo test fail in interop.
            pjones Peter Jones added a comment -

            Landed for 2.16

            pjones Peter Jones added a comment - Landed for 2.16

            "Oleg Drokin <green@whamcloud.com>" merged in patch https://review.whamcloud.com/c/fs/lustre-release/+/50982/
            Subject: LU-13031 jobstats: store jobid in xattr when files are created
            Project: fs/lustre-release
            Branch: master
            Current Patch Set:
            Commit: 23a2db28dcf1422a6a6da575e907fd257106d402

            gerrit Gerrit Updater added a comment - "Oleg Drokin <green@whamcloud.com>" merged in patch https://review.whamcloud.com/c/fs/lustre-release/+/50982/ Subject: LU-13031 jobstats: store jobid in xattr when files are created Project: fs/lustre-release Branch: master Current Patch Set: Commit: 23a2db28dcf1422a6a6da575e907fd257106d402

            For the implementation, any such restrictions/checking would be done in the sysfs write handler when the xattr is first set, and for actual xattr creation this would just copy the job_xattr string during actual file creation with no need for additional sanity checking, since it should always be consistent.

            For the "don't want user.job to be cp.12345" issue, I think that should be handled properly. Initially, the xattr would be set to cp.PID or SLURM_JOBID or whatever by the MDS internally at file creation time. However, if cp, rsync, tar, dcp, dsync ... is also copying xattrs, the initial value would be overwritten later in the copy process, which is what we would want I think.

            adilger Andreas Dilger added a comment - For the implementation, any such restrictions/checking would be done in the sysfs write handler when the xattr is first set, and for actual xattr creation this would just copy the job_xattr string during actual file creation with no need for additional sanity checking, since it should always be consistent. For the "don't want user.job to be cp.12345 " issue, I think that should be handled properly. Initially, the xattr would be set to cp.PID or SLURM_JOBID or whatever by the MDS internally at file creation time. However, if cp , rsync , tar , dcp , dsync ... is also copying xattrs, the initial value would be overwritten later in the copy process, which is what we would want I think.
            adilger Andreas Dilger added a comment - - edited

            I'm inclined toward the user.job xattr, but it is never a good idea to unilaterally make policy decisions in the kernel that cannot be changed.

            As such, it might make sense to have a tunable parameter like "mdt.*.job_xattr=user.job" (or mdd.*.job_xattr is possibly better, though e.g. "mdt.*.job_cleanup_interval" already exists), and then this could be changed in the future if there is some conflict (e.g. some site already uses the "user.job" xattr for some other purpose).

            I don't think the job_xattr should allow totally arbitrary values (e.g. overwriting trusted.lov or trusted.lma or security.* would be bad). One option is to only allow a limited selection of valid xattr namespaces, and possibly names:

            • NONE to turn this feature off
            • user, or trusted or system (if admin wants to restrict the ability of regular users to change this value?), with implicit ".job" added
            • user.* (or trusted.* or system.*) to also allow specifying the xattr name

            If we allow the xattr name portion to be specified (which I'm not sure about, but putting it out for completeness), it should have some reasonable limits:

            • <= 7 characters long to avoid wasting valuable xattr space in the inode
            • only ASCII alpha-numeric characters, plus "." for the name separator, to avoid spaces, control characters, symbols (=, ,, etc.) that might cause problems
            • should not conflict with other known xattrs, which is tricky if we allow the name to be arbitrary. Possibly if in trusted (and system?) it should only allow trusted.job to avoid future conflicts?
            • maybe restrict it to contain "job" (or maybe "pbs", "slurm", ...) to reduce the chance of namespace clashes in user or system? However, I'm reluctant to restrict names in user since this shouldn't have any fatal side effects (e.g. data corruption like in trusted or system), and the admin is supposed to know what they are doing...

            Thoughts?

            adilger Andreas Dilger added a comment - - edited I'm inclined toward the user.job xattr, but it is never a good idea to unilaterally make policy decisions in the kernel that cannot be changed. As such, it might make sense to have a tunable parameter like " mdt.*.job_xattr=user.job " (or mdd.*.job_xattr is possibly better, though e.g. " mdt.*.job_cleanup_interval " already exists), and then this could be changed in the future if there is some conflict (e.g. some site already uses the " user.job " xattr for some other purpose). I don't think the job_xattr should allow totally arbitrary values (e.g. overwriting trusted.lov or trusted.lma or security.* would be bad). One option is to only allow a limited selection of valid xattr namespaces, and possibly names: NONE to turn this feature off user , or trusted or system (if admin wants to restrict the ability of regular users to change this value?), with implicit " .job " added user.* (or trusted.* or system.* ) to also allow specifying the xattr name If we allow the xattr name portion to be specified (which I'm not sure about, but putting it out for completeness), it should have some reasonable limits: <= 7 characters long to avoid wasting valuable xattr space in the inode only ASCII alpha-numeric characters, plus " . " for the name separator, to avoid spaces, control characters, symbols ( = , , , etc.) that might cause problems should not conflict with other known xattrs, which is tricky if we allow the name to be arbitrary. Possibly if in trusted (and system ?) it should only allow trusted.job to avoid future conflicts? maybe restrict it to contain " job " (or maybe " pbs ", " slurm ", ...) to reduce the chance of namespace clashes in user or system ? However, I'm reluctant to restrict names in user since this shouldn't have any fatal side effects (e.g. data corruption like in trusted or system ), and the admin is supposed to know what they are doing... Thoughts?

            Thank you for that; I'm persuaded we should do the user xattr, the issues around interop and user tools look decisive.  If we're forced to change tack in the future, we can deal with that then.  (I think I was the only one actively objecting, Andreas indicated OK at LUG, so you should be good to go.)

            paf0186 Patrick Farrell added a comment - Thank you for that; I'm persuaded we should do the user xattr, the issues around interop and user tools look decisive.  If we're forced to change tack in the future, we can deal with that then.  (I think I was the only one actively objecting, Andreas indicated OK at LUG, so you should be good to go.)

            People

              bertschinger Thomas Bertschinger
              adilger Andreas Dilger
              Votes:
              0 Vote for this issue
              Watchers:
              12 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: