RTradeLtd / TxPB

TemporalX gRPC and Documentation Resources
https://docsx.temporal.cloud
GNU Affero General Public License v3.0
14 stars 1 forks source link

Enable Non Forceful Blockstore Data Removal #87

Open bonedaddy opened 4 years ago

bonedaddy commented 4 years ago

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:

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?

xiegeo commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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.