fix: blob/allocate response for already allocated or received blob #20
Conversation
alanshaw
force-pushed
the
fix/response-for-allocated-or-received-blob
branch
from
20c3f07 to
5c265c2
Compare
| // the size reported in the receipt is the number of bytes allocated | ||
| // in the space - if a previous allocation already exists, this has | ||
| // already been done, so the allocation size is 0 |
There was a problem hiding this comment.
q: does that imply that only the first space where a given blob was allocated is billed for that storage or this has nothing to do with charging users?
There was a problem hiding this comment.
Yes originally this was for billing purposes. In the case of failed uploads the user may try to upload the same data again, or sometimes they just uplaod the same thing multiple times. However we don't want to change them multiple times so the size value accounts for this. We need to revisit what this means in a decentralized network, as this particular node does not have a view over the whole network and there is no guarantee a second allocation will come to the same node as before (would be useful though).
There was a problem hiding this comment.
thanks for the context. I thought we were looking for previous allocations of the same blob in other spaces too, but I see now the check is scoped only to the space the invocation refers to.
about the issue in the decentralized scenario, would it be a possibility to store the DIDs of the spaces where a CID lives as part of the metadata when indexing? Storage nodes could then check with the indexing-service whether a given blob has already been allocated in that same space. I'm still missing a lot of context, so please let me know if that's just nonsensical 😄.
There was a problem hiding this comment.
Yes sounds like a reasonable suggestion!
There was a problem hiding this comment.
I've opened an issue with more details. #23 - would you mind adding your thoughts?
| // even if a previous allocation was made in this space, we create | ||
| // another for the new invocation. |
There was a problem hiding this comment.
q: this is for the case that a previous allocation invocation expired, right?
| fmt.Println(*f.Stack) | ||
| require.Nil(t, f) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
nit: it may be interesting to add a test for the missing case, the one where the blob has been received but is allocated in a different space, for the sake of completeness
There was a problem hiding this comment.
Oh yes good shout ab8c03c
| @@ -17,7 +17,7 @@ import ( | |||
|
|
|||
| var log = logging.Logger("server") | |||
|
|
|||
| // ListenAndServe creates a new indexing service HTTP server, and starts it up. | |||
| // ListenAndServe creates a new storage node HTTP server, and starts it up. | |||
Co-authored-by: Vicente Olmedo <[email protected]>
This PR fixes the response for a repeated
blob/allocatefor an existing blob.Previously:
Now: