From 0b60647c0382426e3b4105d82d04862d2e4831cb Mon Sep 17 00:00:00 2001 From: Alexander Boyko Date: Mon, 12 Apr 2021 08:19:47 -0400 Subject: [PATCH] LU-14606 llog: hide ENOENT for cancelling record Llog allows parallel records processing. A record could be cancelled at callback. If two threads processing and cancelling the same record, one thread would get ENOENT. The error was observed during purging changlog records.The patch adds reproducer test sanity 160m. This is a valid case, let's hide ENOENT error from a caller. HPE-bug-id: LUS-9826 Signed-off-by: Alexander Boyko Change-Id: Id00b959e6f329c2ad34966f8a17a52f71680f24c Reviewed-on: https://review.whamcloud.com/43264 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Alexander Zarochentsev Reviewed-by: Andreas Dilger Reviewed-by: Oleg Drokin --- lustre/include/obd_support.h | 1 + lustre/mdd/mdd_device.c | 15 ++++++++------- lustre/obdclass/llog.c | 17 ++++++++++++++++- lustre/tests/sanity.sh | 42 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 67 insertions(+), 8 deletions(-) diff --git a/lustre/include/obd_support.h b/lustre/include/obd_support.h index 2b3902a..9dde5f7 100644 --- a/lustre/include/obd_support.h +++ b/lustre/include/obd_support.h @@ -232,6 +232,7 @@ extern char obd_jobid_var[]; #define OBD_FAIL_MDS_FLD_LOOKUP 0x15c #define OBD_FAIL_MDS_CHANGELOG_REORDER 0x15d #define OBD_FAIL_MDS_LLOG_UMOUNT_RACE 0x15e +#define OBD_FAIL_MDS_CHANGELOG_RACE 0x15f #define OBD_FAIL_MDS_INTENT_DELAY 0x160 #define OBD_FAIL_MDS_XATTR_REP 0x161 #define OBD_FAIL_MDS_TRACK_OVERFLOW 0x162 diff --git a/lustre/mdd/mdd_device.c b/lustre/mdd/mdd_device.c index 54445ef..a782db3 100644 --- a/lustre/mdd/mdd_device.c +++ b/lustre/mdd/mdd_device.c @@ -306,10 +306,8 @@ static int llog_changelog_cancel_cb(const struct lu_env *env, struct llog_rec_hdr *hdr, void *data) { struct llog_changelog_rec *rec = (struct llog_changelog_rec *)hdr; - struct llog_cookie cookie; struct changelog_cancel_cookie *cl_cookie = (struct changelog_cancel_cookie *)data; - int rc; ENTRY; @@ -339,15 +337,18 @@ static int llog_changelog_cancel_cb(const struct lu_env *env, /* records are in order, so we're done */ RETURN(LLOG_PROC_BREAK); - cookie.lgc_lgl = llh->lgh_id; - cookie.lgc_index = hdr->lrh_index; + if (unlikely(OBD_FAIL_PRECHECK(OBD_FAIL_MDS_CHANGELOG_RACE))) { + if (cfs_fail_val == 0) + cfs_fail_val = hdr->lrh_index; + if (cfs_fail_val == hdr->lrh_index) + OBD_RACE(OBD_FAIL_MDS_CHANGELOG_RACE); + } /* cancel them one at a time. I suppose we could store up the cookies * and cancel them all at once; probably more efficient, but this is * done as a user call, so who cares... */ - rc = llog_cat_cancel_records(env, llh->u.phd.phd_cat_handle, 1, - &cookie); - RETURN(rc < 0 ? rc : 0); + + RETURN(LLOG_DEL_RECORD); } static int llog_changelog_cancel(const struct lu_env *env, diff --git a/lustre/obdclass/llog.c b/lustre/obdclass/llog.c index 696a4ea..5580286 100644 --- a/lustre/obdclass/llog.c +++ b/lustre/obdclass/llog.c @@ -312,13 +312,22 @@ int llog_cancel_arr_rec(const struct lu_env *env, struct llog_handle *loghandle, } out_unlock: + if (rc < 0) { + /* restore bitmap while holding a mutex */ + if (subtract_count) { + loghandle->lgh_hdr->llh_count += num; + subtract_count = false; + } + for (i = i - 1; i >= 0; i--) + set_bit_le(index[i], LLOG_HDR_BITMAP(llh)); + } mutex_unlock(&loghandle->lgh_hdr_mutex); up_write(&loghandle->lgh_lock); out_trans: rc1 = dt_trans_stop(env, dt, th); if (rc == 0) rc = rc1; - if (rc < 0) { + if (rc1 < 0) { mutex_lock(&loghandle->lgh_hdr_mutex); if (subtract_count) loghandle->lgh_hdr->llh_count += num; @@ -727,6 +736,12 @@ repeat: rc = llog_cancel_rec(lpi->lpi_env, loghandle, rec->lrh_index); + /* Allow parallel cancelling, ENOENT + * means record was canceled at another + * processing thread or callback + */ + if (rc == -ENOENT) + rc = 0; } if (rc) GOTO(out, rc); diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh index 36c19bd..75b3565 100755 --- a/lustre/tests/sanity.sh +++ b/lustre/tests/sanity.sh @@ -15799,6 +15799,48 @@ test_160l() { } run_test 160l "Verify that MTIME changelog records contain the parent FID" +test_160m() { + remote_mds_nodsh && skip "remote MDS with nodsh" && return + [[ $MDS1_VERSION -ge $(version_code 2.14.51) ]] || + skip "Need MDS version at least 2.14.51" + local cl_users + local cl_user1 + local cl_user2 + local pid1 + + # Create a user + changelog_register || error "first changelog_register failed" + changelog_register || error "second changelog_register failed" + + cl_users=(${CL_USERS[mds1]}) + cl_user1="${cl_users[0]}" + cl_user2="${cl_users[1]}" + # generate some changelog records to accumulate on MDT0 + test_mkdir -p -i0 -c1 $DIR/$tdir || error "test_mkdir $tdir failed" + createmany -m $DIR/$tdir/$tfile 50 || + error "create $DIR/$tdir/$tfile failed" + unlinkmany $DIR/$tdir/$tfile 50 || error "unlinkmany failed" + rm -f $DIR/$tdir + + # check changelogs have been generated + local nbcl=$(changelog_dump | wc -l) + [[ $nbcl -eq 0 ]] && error "no changelogs found" + +#define OBD_FAIL_MDS_CHANGELOG_RACE 0x15f + do_facet mds1 $LCTL set_param fail_loc=0x8000015f fail_val=0 + + __changelog_clear mds1 $cl_user1 +10 + __changelog_clear mds1 $cl_user2 0 & + pid1=$! + sleep 2 + __changelog_clear mds1 $cl_user1 0 || + error "fail to cancel record for $cl_user1" + wait $pid1 + [[ $? -eq 0 ]] || error "fail to cancel record for $cl_user2" +} +run_test 160m "Changelog clear race" + + test_161a() { [ $PARALLEL == "yes" ] && skip "skip parallel run" -- 1.8.3.1