[LU-11426] 2/2 Olafs agree: changelog entries are emitted out of order

• Type: Bug
• Resolution: Fixed
• Priority: Minor
• Fix Version/s: Lustre 2.13.0, Lustre 2.12.3
• Labels: ORNL changelog hsm llnl
• Severity: 3
• Assignee: Qian Yingjin
• Reporter: John Hammond

Description

When storing changelogs we do not strictly order the entries with respect to index.

int mdd_changelog_store(const struct lu_env *env, struct mdd_device *mdd,
                        struct llog_changelog_rec *rec, struct thandle *th)
{
        ...

        /* llog_lvfs_write_rec sets the llog tail len */
        rec->cr_hdr.lrh_type = CHANGELOG_REC;
        rec->cr.cr_time = cl_time();

        spin_lock(&mdd->mdd_cl.mc_lock);
        /* NB: I suppose it's possible llog_add adds out of order wrt cr_index,
         * but as long as the MDD transactions are ordered correctly for e.g.
         * rename conflicts, I don't think this should matter. */
        rec->cr.cr_index = ++mdd->mdd_cl.mc_index;
        spin_unlock(&mdd->mdd_cl.mc_lock);

        ctxt = llog_get_context(obd, LLOG_CHANGELOG_ORIG_CTXT);
        if (ctxt == NULL)
                return -ENXIO;

        llog_th = thandle_get_sub(env, th, ctxt->loc_handle->lgh_obj);
        if (IS_ERR(llog_th))
                GOTO(out_put, rc = PTR_ERR(llog_th));

        /* nested journal transaction */
        rc = llog_add(env, ctxt->loc_handle, &rec->cr_hdr, NULL, llog_th);

This may not be a bug in itself but it means that a changelog consumer must account for gaps in the sequence when clearing changelog entries.

Issue Links

• is duplicated by: LU-12869 interop: lustre-rsync-test test 2a fails with 'Failure in replication; differences found.'
• is related to: LU-11581 Not all changelog entries are returned to userspace
• is related to: LU-11205 Failure to clear the changelog for user 1 on MDT

Comments

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/36316/
Subject: LU-11426 llog: changelog records reordering
Project: fs/lustre-release
Branch: b2_12
Current Patch Set:
Commit: 5c5a1e9b4839c3d6a70b3b7e768944f6dc237c2e

[James A Simmons]

I'm also applying LU-11617 which is a potential bug as well as the above patches. The reason for also including LU-12533 was so the patches applied cleanly to 2.12 LTS. If its not needed then thats fine

[Peter Jones]

Thanks James. The second patch looks like a nice performance improvement but I am not sure how this would relate to changelog correctness. Are you just listing all the patches that you are currently applying to b2_12 regardless of their relevance to this particular issue?

[James A Simmons]

We have also

Lustre commit d7bb6647cd4dd26949bceb6a099cd606623aff2b ("LU-11626 mdc: hold obd while processing changelog")

The other patch is LU-12533 but I don't know if that is more a feature or a fix so its not clear in that case.

 

[Peter Jones]

simmonsja could you please be more specific as to what is missing from b2_12 before the benefits of this patch could be realized?

[Gerrit Updater]

Minh Diep ([e-mail]) uploaded a new patch: https://review.whamcloud.com/36316
Subject: LU-11426 llog: changelog records reordering
Project: fs/lustre-release
Branch: b2_12
Current Patch Set: 1
Commit: 2b7ea4e3cf43f18b1e1354044d024e085701946e

[James A Simmons]

To back port this fix will require a bunch of other change log fixes to land first. One of them is the add polling patch. Is that patch needed for 2.12 LTS?

[Peter Jones]

Yes absolutely - a number of sites have hit this

[Bruno Travouillon (Inactive)]

Hi Peter,
Can we consider a backport to LTS12? (2.12.4?)

[Peter Jones]

Landed for 2.13