Fix db_wal_test::TruncateLastLogAfterRecoverWithoutFlush failure#6437
Fix db_wal_test::TruncateLastLogAfterRecoverWithoutFlush failure#6437guyuqi wants to merge 2 commits intofacebook:masterfrom
Conversation
TruncateLastLogAfterRecoverWithoutFlush case depends on fallocate support of underlying file system. On a file system which lacks of this feature, like zfs, it will fail to allocate predefined file size as this test case intends to do; So a check block is added to detect fallocate support and skip test if not. Change-Id: I731f89ef39b062bf65e8e495a0018c32a4ecb76a Signed-off-by: Yuqi Gu <yuqi.gu@arm.com>
|
@guyuqi So we have a compile time flag The code that you patch is only enabled when that flag is true. So it would seem to me that your patch indicates a problem with detecting a working fallocate (or some feature of it - Is this some combination of fallocate and truncate?). Either way, it would seem prudent to fix the detection (so that it can be used across the codebase), either:
What do you think? |
|
@adamretter So this patch is needed when binary code run on a node which doesn't support fallocate though the code was compiled properly on another node where |
@adamretter |
|
@adamretter as you mentioned in build_detect_platform, this flag is validated in compile time by checking if the test prog can be built or not, which means as long as |
|
@siying , could you please have a look into this PR? |
facebook-github-bot
left a comment
There was a problem hiding this comment.
@siying has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
db/db_wal_test.cc
Outdated
| close(fd); | ||
| ASSERT_OK(options.env->DeleteFile(fname_test_fallocate)); | ||
| if (alloc_status != 0) { | ||
| fprintf(stderr, " Skipped preallocated space check: %s\n", strerror(err_number)); |
There was a problem hiding this comment.
I would be more comfortable if we only skip when err_number == ENOSYS || err_number == EOPNOTSUPP and otherwise fail the test on alloc_status != 0.
There was a problem hiding this comment.
Updated.
Thanks for your comments.
Change-Id: I95d7326e18939d5bdfbc0ed064f1b3d5127f5f66 Signed-off-by: Yuqi Gu <yuqi.gu@arm.com>
|
@guyuqi has updated the pull request. Re-import the pull request |
facebook-github-bot
left a comment
There was a problem hiding this comment.
@pdillinger has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
pdillinger
left a comment
There was a problem hiding this comment.
LGTM, thanks! I think we now have adequate protection against the test accidentally, quietly being broadly skipped.
|
@pdillinger merged this pull request in e171a21. |
Fix db_wal_test::TruncateLastLogAfterRecoverWithoutFlush failure (facebook#6437)
TruncateLastLogAfterRecoverWithoutFlushcase depends on fallocate supportof underlying file system.
On a file system which lacks of this feature, like zfs, it will fail to allocate predefined file size as this test case intends to do;
So a check block is added to detect fallocate support and skip test if not.
The related work is done by @JunHe77. Thanks!
Signed-off-by: Yuqi Gu yuqi.gu@arm.com