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

• Status: Resolved
• Project: Lustre
• Fix Version/s: Lustre 2.14.0
• Type: Bug
• Priority: Minor
• Reporter: Maloo
• Assignee: Andreas Dilger
• Resolution: Fixed
• Severity: 3

Description

This issue was created by maloo for Andreas Dilger <[e-mail]>

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

Issue Links

• is related to: LU-12027 Allow "lfs find" atime/mtime/ctime wi...

Comments

[Andreas Dilger]

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".

[Andreas Dilger]

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?

[Gerrit Updater]

Wang Shilong ([e-mail]) 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 Updater]

Andreas Dilger ([e-mail]) 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

[Mikhail Pershin]

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

[Gerrit Updater]

Andreas Dilger ([e-mail]) 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

[Andreas Dilger]

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.

[Gerrit Updater]

Andreas Dilger ([e-mail]) 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 Updater]

Oleg Drokin ([e-mail]) 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

[Peter Jones]

Landed for 2.14