Skip to content

[Colossus] Accept pending data objects enhancements#4971

Merged
mnaamani merged 15 commits intoJoystream:masterfrom
zeeshanakram3:colossus_accept_pending_data_objects_enhancements
Nov 30, 2023
Merged

[Colossus] Accept pending data objects enhancements#4971
mnaamani merged 15 commits intoJoystream:masterfrom
zeeshanakram3:colossus_accept_pending_data_objects_enhancements

Conversation

@zeeshanakram3
Copy link
Copy Markdown
Contributor

@zeeshanakram3 zeeshanakram3 commented Nov 21, 2023

fixes #4968

This PR:

  • Increase the default timeout value in the extrinsic wrapper to match the transaction validity in the transaction pool
  • Adds new AcceptPendingObjectsService that is responsible for periodically sending batch accept_pending_data_objects for all the pending data objects
  • The POST /files endpoint now no longer calls the accept_pending_data_objects extrinsic for individual uploads, instead, it registers all the pending objects with AcceptPendingObjectsService
  • Updates '/state/data' endpoint response headers to return data objects status too i.e. (pending or accepted)

Copy link
Copy Markdown
Member

@mnaamani mnaamani left a comment

Choose a reason for hiding this comment

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

Left a few suggestions.

I think we need to start thinking about add a better way to manage the state of the node between runs. Depending on presence of files in directories is brittle and many things can go wrong as the file system is easy to corrupt my moving files around accidentally.

this.push(dataObject.id, storageBucket.id, dataObject.storageBag.id)
} else {
logger.warn(
`Data object ${dataObject.id} in pending directory is not assigned to any of the upload buckets: ${this.uploadBuckets}.`
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We would probably want to remove these objects at some point, probably in the "pruning" worker once we merge.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, wasn't sure where that should be done, here OR in the pruning service, that's why I left it as it is.

This scenario would mostly happen if the data object is moved (i.e. object's bag is moved). And, I think removing objects due to this action is best suited for the Pruning service (which extensively ensures that moved data objects at least exist in N different buckets before deleting it). WDYT?

Maybe I should create an issue for that?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes lets leave it to the pruning service, and create an issue for it.
Just fyi the version of colossus with the pruning service is merged into colossus-beta branch.

@mnaamani mnaamani self-requested a review November 30, 2023 05:43
Copy link
Copy Markdown
Member

@mnaamani mnaamani left a comment

Choose a reason for hiding this comment

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

Minor fix in runWithInterval()

this.push(dataObject.id, storageBucket.id, dataObject.storageBag.id)
} else {
logger.warn(
`Data object ${dataObject.id} in pending directory is not assigned to any of the upload buckets: ${this.uploadBuckets}.`
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes lets leave it to the pruning service, and create an issue for it.
Just fyi the version of colossus with the pruning service is merged into colossus-beta branch.

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.

[Bug] Object upload request to the storage-node fail even though the call to accept_pending_data_objects in the same request succeeds

2 participants