Skip to content
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

feat: atomic object update #570

Merged
merged 6 commits into from Feb 21, 2024

Conversation

alexgao001
Copy link
Collaborator

@alexgao001 alexgao001 commented Jan 24, 2024

Description

Enable atomic object update.

  1. Introduces a new Msg MsgUpdateObjectContent, added fields to current ObjectInfo model.
message ObjectInfo {
  ..... 
  // is_updating indicates whether a object is being updated.
  bool is_updating = 16;
  // updated_at define the block timestamp when the object is updated. Will not be visible until object is re-sealed.
  int64 updated_at = 17;
  // updated_by defined the account address of updater(if there is). Will not be visible until object is re-sealed.
  string updated_by = 18 [(cosmos_proto.scalar) = "cosmos.AddressString"];
  // version define the version of object
  int64 version = 19;
}
  1. Modify current seal object process so that a object under updating can be sealed.
  2. Lock Updated Object while user submit the request.
  3. Once object is updated successfully, new bill is base on the updated object, previous payment will be deducted.

More details please refer to bnb-chain/BEPs#346

Rationale

tell us why we need these changes...

Example

add an example CLI or API response...

Changes

Notable changes:

  • add each change in a bullet point here
  • ...

Potential Impacts

  • add potential impacts for other components here
  • ...

@alexgao001 alexgao001 added the wip label Jan 24, 2024
@alexgao001 alexgao001 force-pushed the feat-atomic-file-update branch 2 times, most recently from 56e56a4 to c6860c2 Compare January 24, 2024 12:24
@alexgao001 alexgao001 force-pushed the feat-atomic-file-update branch 2 times, most recently from f956aac to bc85d0f Compare February 2, 2024 07:28
@alexgao001 alexgao001 added r4r and removed wip labels Feb 2, 2024
return err
}
}
_ = k.MustGetPrimarySPForBucket(ctx, bucketInfo)
Copy link
Contributor

Choose a reason for hiding this comment

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

This line _ = k.MustGetPrimarySPForBucket(ctx, bucketInfo) is not needed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

removed

@@ -101,6 +112,12 @@ func GetObjectByIDKey(objectId math.Uint) []byte {
return append(ObjectByIDPrefix, seq.EncodeSequence(objectId)...)
}

// GetShadowObjectByIDKey return the shadowObjectId store key
func GetShadowObjectByIDKey(objectId math.Uint) []byte {
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need bucketName + objectName -> id -> shadow object.
We can just bucketName + objectName -> shadow object, to save storage and make code easier.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep, change is applied.

return err
}
} else {
objectInfo.IsUpdating = true
Copy link
Contributor

Choose a reason for hiding this comment

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

IsUpdating is a temp field. It seems better we just put into ShadowObjectInfo - so it will be deleted with shadow object to save storage.

}

bbz := k.cdc.MustMarshal(bucketInfo)
store.Set(types.GetBucketByIDKey(bucketInfo.Id), bbz)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we change bucketInfo here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no change made, removed.

return errors.Wrapf(types.ErrAccessDenied, "Only allowed owner/updater to do cancel update object")
}

objectInfo.IsUpdating = false
Copy link
Contributor

Choose a reason for hiding this comment

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

Put IsUpdating into shadowObject will reduce such codes.

@alexgao001 alexgao001 added this pull request to the merge queue Feb 21, 2024
Merged via the queue into bnb-chain:develop with commit a6c913a Feb 21, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants