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

Enable Non Forceful Blockstore Data Removal #87

Open
bonedaddy opened this issue May 2, 2020 · 5 comments
Open

Enable Non Forceful Blockstore Data Removal #87

bonedaddy opened this issue May 2, 2020 · 5 comments
Assignees
Labels
enhancement New feature or request needs-discussion the outcome is not clear and needs to be discussed

Comments

@bonedaddy
Copy link
Collaborator

bonedaddy commented May 2, 2020

At the moment the only way to manually remove objects is to to remove their individual blocks, with the Blockstore::Delete(force) RPC. The way data removal is conducted should be slightly adjusted, leaving the forceful delete method for backwards compatability.

The new RPC should perform the following steps when giving a CID to delete:

  • Pull its reference counter entry
  • If the entry has links, iterate over all of its links (and if those links have links, do the same, etc...), calling the DeleteBlock function on each link
  • Finally after doing this on all links, and their descendants, call the DeleteBlock function on the CID given in the RPC request

If any reference count entries have a reference count of >= 1 after this RPC is done being processed, it means that it is referred to by other objects and wont be removed until those objects are removed.

To do this, the reference counter will need to be modified to include the links within the reference count entry. As such this means a migration will need to be ran if the reference counted blockstore has previous items in it. This is a pretty simple thing to do as all we need to do is decode the block into an IPLD type, see if it has links, and add those to the reference counter protocol buffer entry.

To deal with the migration I'm thinking we can use a marker key in the reference counter datastore using a key named something like is-linkable. IF this key isn't present it means that the reference counter isn't yet one that enables link decoding, which means we need to do a migration that goes through all CIDs, and makes sure to add the links to the reference counter entry, after which we add a marker key. Overall the migration should be pretty fast as all we would need to do is run AllKeysChan, for each CID pull the block from the blockstore, decode it into an ipld.Node type, get its Links and add that to the entry.Links field.

This new RPC would also be able to be integrated into s3x and give us the utility to enable removal of objects

@xiegeo thoughts on the proposed solution?

@bonedaddy bonedaddy added enhancement New feature or request needs-discussion the outcome is not clear and needs to be discussed labels May 2, 2020
@xiegeo
Copy link
Contributor

xiegeo commented May 2, 2020

Instead of is-linkable, we could add a recursive counter in addition to our current count.

This allows the existing API to stay as is, but add a recursive version to all adds and deletes. Where the recursive version only allows adding data that contains links.

With two counters, only if both are zero, is an object deleted.

This has the additional property of allowing the user to chose which type of add is appropriate for each call, instead of for each object globally.

For additional safety, I think we should add an is_required_by counted set for each key, to prevent a user from deleting an object more times than calling add (good protection against bugs, and required for public-facing API). The set should contains some application-specific user id for directly added counts or the object hash for recursively added counts. If implemented, this counted set should replace the traditional counters.

@bonedaddy
Copy link
Collaborator Author

Instead of is-linkable, we could add a recursive counter in addition to our current count.
This allows the existing API to stay as is, but add a recursive version to all adds and deletes. Where the recursive version only allows adding data that contains links.

Good idea

This has the additional property of allowing the user to chose which type of add is appropriate for each call, instead of for each object globally.

So the problem with this is it somewhat changes the semantics of the reference counter. The way the reference counter works right now requires no additional activity from the user because it transparently traps blockstore calls and handles the reference count operation.

To avoid having to require the user to correctly call the appropriate reference counter, and to keep the transparent trapping, what we could do is have an additional recursive reference counter type. Whenever adds and deletes are called it would perform a check to see if the object contains links.

If it does contain links we would run a function called countLinkedCID, if the object doesn't contain links then we run the countCID function that we have now. This preserves the transparent trapping effect that we have now, while not requiring the user to call the correct reference count operation, ensuring that there wont ever be user error which would cause a reference count to be incorrect.

For additional safety, I think we should add an is_required_by counted set for each key, to prevent a user from deleting an object more times than calling add (good protection against bugs, and required for public-facing API).

I think we have this already? reference counter entries have a links field, which would indicate other objects that require the CID.

The set should contains some application-specific user id for directly added counts or the object hash for recursively added counts. If implemented, this counted set should replace the traditional counters.

Could you explain this a bit more? I'm not sure i understand

@xiegeo
Copy link
Contributor

xiegeo commented May 10, 2020

I don't think it's implemented already. If I call delete on an object, could I decrement the counter even if I never called add on it, therefor possibly removing someone else's object?

@bonedaddy
Copy link
Collaborator Author

bonedaddy commented May 10, 2020

I don't think it's implemented already. If I call delete on an object, could I decrement the counter even if I never called add on it, therefor possibly removing someone else's object?

Not sure I follow? If an object has never been added before then it can be decreased in reference. If you're adding a file via the upload API then when the file gets chunked and stored by the dagservice, the dagservice forwards the data to the blockstore which gets trapped by the reference counter.

So it's not possible to negatively set the reference count, thus when someone goes to add the object later it would get automatically removed. Essentially if you try this, you'll just waste CPU cycles deleting an object that isn't there.

@xiegeo
Copy link
Contributor

xiegeo commented May 10, 2020

So is this process impossible?

  1. User A uploads an object.
  2. User A shares read to the object by giving it's cid to User B.
  3. User B calls delete on the object using the cid.
  4. User A can no longer read the object.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs-discussion the outcome is not clear and needs to be discussed
Projects
None yet
Development

No branches or pull requests

2 participants