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

sanity test_56ob: lfs find /mnt/lustre/d56ob.sanity -mtime 1y' wrong: found 0, expected 1

Details

    • Bug
    • Resolution: Fixed
    • Minor
    • Lustre 2.14.0
    • None
    • None
    • 3
    • 9223372036854775807

    Description

      This issue was created by maloo for Andreas Dilger <adilger@whamcloud.com>

      This issue relates to the following test suite run: https://testing.whamcloud.com/test_sets/0e2fc3d7-0b4f-469a-8b96-7b6fa53e6261

      test_56ob failed with the following error:

      '/usr/bin/lfs find /mnt/lustre/d56ob.sanity -mtime 1y' wrong: found 0, expected 1
      

      This is failing 100% since last night after the most recent master-next landing.

      VVVVVVV DO NOT REMOVE LINES BELOW, Added by Maloo for auto-association VVVVVVV
      sanity test_56ob - '/usr/bin/lfs find /mnt/lustre/d56ob.sanity -mtime 1y' wrong: found 0, expected 1

      Attachments

        Issue Links

          Activity

            [LU-13314] sanity test_56ob: lfs find /mnt/lustre/d56ob.sanity -mtime 1y' wrong: found 0, expected 1
            pjones Peter Jones added a comment -

            Landed for 2.14

            pjones Peter Jones added a comment - Landed for 2.14

            Oleg Drokin (green@whamcloud.com) merged in patch https://review.whamcloud.com/39433/
            Subject: LU-13314 utils: fix lfs find time calculation margin
            Project: fs/lustre-release
            Branch: master
            Current Patch Set:
            Commit: 7e0208da21f9358d96feebce5124f8587778c318

            gerrit Gerrit Updater added a comment - Oleg Drokin (green@whamcloud.com) merged in patch https://review.whamcloud.com/39433/ Subject: LU-13314 utils: fix lfs find time calculation margin Project: fs/lustre-release Branch: master Current Patch Set: Commit: 7e0208da21f9358d96feebce5124f8587778c318

            Andreas Dilger (adilger@whamcloud.com) uploaded a new patch: https://review.whamcloud.com/39657
            Subject: LU-13314 utils: allow lfs find time within a range
            Project: fs/lustre-release
            Branch: master
            Current Patch Set: 1
            Commit: 3d3cc0092a2b86907252541ea0b8759bb987a315

            gerrit Gerrit Updater added a comment - Andreas Dilger (adilger@whamcloud.com) uploaded a new patch: https://review.whamcloud.com/39657 Subject: LU-13314 utils: allow lfs find time within a range Project: fs/lustre-release Branch: master Current Patch Set: 1 Commit: 3d3cc0092a2b86907252541ea0b8759bb987a315

            I fixed this by increasing the margin of error by a day per year. I have a follow-on patch that will allow searching between precise intervals (e.g. "-atime +30 -atime -60" that needs to be finished, but at least this first patch/test is functional again.

            adilger Andreas Dilger added a comment - I fixed this by increasing the margin of error by a day per year. I have a follow-on patch that will allow searching between precise intervals (e.g. " -atime +30 -atime -60 " that needs to be finished, but at least this first patch/test is functional again.

            Andreas Dilger (adilger@whamcloud.com) uploaded a new patch: https://review.whamcloud.com/39433
            Subject: LU-13314 utils: fix lfs find time calculation margin
            Project: fs/lustre-release
            Branch: master
            Current Patch Set: 1
            Commit: a673bda1ba892d302f6638d112ea75b07b24f7c1

            gerrit Gerrit Updater added a comment - Andreas Dilger (adilger@whamcloud.com) uploaded a new patch: https://review.whamcloud.com/39433 Subject: LU-13314 utils: fix lfs find time calculation margin Project: fs/lustre-release Branch: master Current Patch Set: 1 Commit: a673bda1ba892d302f6638d112ea75b07b24f7c1
            tappro Mikhail Pershin added a comment - - edited

            this is lfs calc error in set_time(). It uses 365 days in year always, so to work correctly is should takes leap day into account while doing maths for +/- year/week

            tappro Mikhail Pershin added a comment - - edited this is lfs calc error in set_time() . It uses 365 days in year always, so to work correctly is should takes leap day into account while doing maths for +/- year/week

            Andreas Dilger (adilger@whamcloud.com) merged in patch https://review.whamcloud.com/37767/
            Subject: LU-13314 test: add 56ob to ALWAYS_EXCEPT for now
            Project: fs/lustre-release
            Branch: master
            Current Patch Set:
            Commit: bfaa8a0686f95802fda79fbc8cf9001fcf1b5192

            gerrit Gerrit Updater added a comment - Andreas Dilger (adilger@whamcloud.com) merged in patch https://review.whamcloud.com/37767/ Subject: LU-13314 test: add 56ob to ALWAYS_EXCEPT for now Project: fs/lustre-release Branch: master Current Patch Set: Commit: bfaa8a0686f95802fda79fbc8cf9001fcf1b5192
            gerrit Gerrit Updater added a comment - - edited

            Wang Shilong (wshilong@ddn.com) uploaded a new patch: https://review.whamcloud.com/37767
            Subject: LU-13314 test: add 56ob to ALWAYS_EXCEPT for now
            Project: fs/lustre-release
            Branch: master
            Current Patch Set: 1
            Commit: 2d82cb1ae482e9d9ceae8d8d95540bcfdcc4d19f

            gerrit Gerrit Updater added a comment - - edited Wang Shilong (wshilong@ddn.com) uploaded a new patch: https://review.whamcloud.com/37767 Subject: LU-13314 test: add 56ob to ALWAYS_EXCEPT for now Project: fs/lustre-release Branch: master Current Patch Set: 1 Commit: 2d82cb1ae482e9d9ceae8d8d95540bcfdcc4d19f

            It looks like time_margin is exactly 24h, so the leap day plus a few seconds for running the test moves the "touch" file out of range.

            I think it makes sense to have more than a 1-day margin for files 1 year old. I'm not sure what a sensible default would be, but maybe 1d + 1d/year? That would be enough to fix the leap-day issue, and for files 7 years old that would give a margin of 8d, which I think would be totally acceptable margin of error?

            adilger Andreas Dilger added a comment - It looks like time_margin is exactly 24h, so the leap day plus a few seconds for running the test moves the " touch " file out of range. I think it makes sense to have more than a 1-day margin for files 1 year old. I'm not sure what a sensible default would be, but maybe 1d + 1d/year? That would be enough to fix the leap-day issue, and for files 7 years old that would give a margin of 8d, which I think would be totally acceptable margin of error?

            Wang Shilong wrote:

            I guess this is becuase:

            set_time
            
             switch (*endptr) {
             case 'y': 
             unit *= 52; /* 52 weeks + 1 day below */
             case 'w': /* fallthrough */
             unit *= 7; 
             case '\0': /* days are default unit if none used */
             case 'd': /* fallthrough */
             unit = (unit + (*endptr == 'y')) * 24; 
             case 'h': /* fallthrough */
             unit *= 60;
             case 'm': /* fallthrough */
             unit *= 60;
             case 's': /* fallthrough */
             break;
             /* don't need to multiply by 1 for seconds */
             default:
            

            This is not always exact for one year calculations? i guess codes might be fine, but need allow tests to be ware of such case?

            This seems to be correct. It looks like the bug was introduced in patch https://review.whamcloud.com/34367 "LU-12027 utils: add units to 'lfs find -amctime'", or possibly patch https://review.whamcloud.com/34658 "LU-12027 utils: fix 'lfs find -amctime' comparison".

            adilger Andreas Dilger added a comment - Wang Shilong wrote: I guess this is becuase: set_time switch (*endptr) { case 'y': unit *= 52; /* 52 weeks + 1 day below */ case 'w': /* fallthrough */ unit *= 7; case '\0': /* days are default unit if none used */ case 'd': /* fallthrough */ unit = (unit + (*endptr == 'y')) * 24; case 'h': /* fallthrough */ unit *= 60; case 'm': /* fallthrough */ unit *= 60; case 's': /* fallthrough */ break; /* don't need to multiply by 1 for seconds */ default: This is not always exact for one year calculations? i guess codes might be fine, but need allow tests to be ware of such case? This seems to be correct. It looks like the bug was introduced in patch https://review.whamcloud.com/34367 " LU-12027 utils: add units to 'lfs find -amctime' ", or possibly patch https://review.whamcloud.com/34658 " LU-12027 utils: fix 'lfs find -amctime' comparison ".

            People

              adilger Andreas Dilger
              maloo Maloo
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: