Skip to content

jgm/daos-4698-1: Rebuild object migration should execute applicable object punch for each migration. #4895

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Mar 11, 2021

Conversation

jgmoore-or
Copy link
Contributor

Added object punch epoch to object enumeration. Execute the punch during migrate.

jolivier23 and others added 4 commits March 3, 2021 14:32
Add future object punch epoch to key iterator.  Needed for
rebuild migration so we can rebuild punched objects that
are visible in a prior snapshot.

Signed-off-by: Jeff Olivier <jeffrey.v.olivier@intel.com>
Signed-off-by: Joseph Moore <joseph.moore@intel.com>
Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. No errors found by checkpatch.

@jgmoore-or jgmoore-or requested a review from jolivier23 March 5, 2021 22:26
…d targets.

Signed-off-by: Joseph Moore <joseph.moore@intel.com>
Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. No errors found by checkpatch.

… object punch for each migration.

Signed-off-by: Joseph Moore <joseph.moore@intel.com>
Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. No errors found by checkpatch.

…object punch for each migration.

Signed-off-by: Joseph Moore <joseph.moore@intel.com>
… object punch for each migration.

Signed-off-by: Joseph Moore <joseph.moore@intel.com>
Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. No errors found by checkpatch.

iov->iov_len += pi_size;
arg->obj_punched = true;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we will get object punch epoch by scan. see rebuild_obj_scan_cb() L601. So you do not need change enumerate packing.
Anything concern?

Copy link
Contributor Author

@jgmoore-or jgmoore-or Mar 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I discussed this option with Jeff. He says the scan only picks up the latest punch. For the incarnation log problem we're trying to solve, the subsequent punch closest to the update epoch must be replayed. There can be multiple object punches at differing epochs for an object, and recording them in the enumeration packing is required.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The latest isn't sufficient. You could have many snapshots you are rebuilding. As such, you could have something like this

update object at time 1
take snapshot at 2
punch at 3
update a at 4
snapshot at 5
punch at 6

If you rebuild the object at 7, you will only see the punch at 6 in the scan phase. You only see the intermediate punch 3 while you are doing migration for the snapshot at 2. So you can't do this at the scan phase.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, right. Thanks for explanation.

@jgmoore-or jgmoore-or requested review from daosbuild1 and wangdi1 March 8, 2021 15:26
jolivier23
jolivier23 previously approved these changes Mar 8, 2021
Copy link
Contributor

@wangdi1 wangdi1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably add a few more snapshots and punch in rebuild_snap_punch_empty() test. Thanks.

rc = vos_obj_punch(cont->sc_hdl, mrone->mo_oid,
mrone->mo_obj_punch_eph,
tls->mpt_version, VOS_OF_REPLAY_PC,
NULL, 0, NULL, NULL);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jolivier23 is it ok for VOS to punch the same object with same epoch multiple times?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be ok with the VOS_OF_REPLAY_PC flag. If it doesn't work, it's a bug

if (rc) {
D_ERROR(DF_UOID" punch obj failed: rc %d\n",
DP_UOID(mrone->mo_oid), rc);
return rc;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

D_GOTO(obj_close, rc) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been fixed.

…t objects.

Signed-off-by: Joseph Moore <joseph.moore@intel.com>
Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. No errors found by checkpatch.

@jgmoore-or jgmoore-or requested review from jolivier23 and wangdi1 March 9, 2021 14:57
Copy link
Contributor

@jolivier23 jolivier23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to revert the submodule update

Signed-off-by: Joseph Moore <joseph.moore@intel.com>
Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. No errors found by checkpatch.

@jgmoore-or jgmoore-or requested a review from jolivier23 March 10, 2021 14:01
Copy link
Contributor

@wangdi1 wangdi1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Joe

@jgmoore-or jgmoore-or requested a review from a team March 11, 2021 15:51
@jolivier23 jolivier23 merged commit cf17bc5 into master Mar 11, 2021
@jolivier23 jolivier23 deleted the jgm/DAOS-4698-1 branch March 11, 2021 17:15
@johannlombardi
Copy link
Contributor

Should be ported to 1.2 since this is a 1.2 blocker.

jgmoore-or added a commit that referenced this pull request Mar 25, 2021
… object punch for each migration. (#4895)

Added object punch epoch to object enumeration. Execute the punch during migrate.

Signed-off-by: Joseph Moore <joseph.moore@intel.com>
johannlombardi pushed a commit that referenced this pull request Mar 26, 2021
… object punch for each migration. (#4895) (#5188)

Added object punch epoch to object enumeration.
Execute the punch during migrate.

Signed-off-by: Joseph Moore <joseph.moore@intel.com>
@ashleypittman ashleypittman mentioned this pull request Apr 28, 2021
@ashleypittman ashleypittman mentioned this pull request May 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants