swarm/chunk: add tags #1341
swarm/chunk: add tags #1341
Conversation
janos
left a comment
janos
left a comment
There was a problem hiding this comment.
It is a nice start Elad. I have a few concerns about data consistency in SetTotal and WaitTill functions and other things are minor.
|
@zelig addressed your PR comments. That linter error for some reason won't go away. tried linting and |
janos
left a comment
janos
left a comment
There was a problem hiding this comment.
I have only one concern about DoneSplit function.
| type State = uint32 | ||
|
|
||
| const ( | ||
| SPLIT State = iota // chunk has been processed by filehasher/swarm safe call |
There was a problem hiding this comment.
Have you considered maybe to call this constants StateSplit, StateStored, etc? Something like http.StatusOK, http.StatusNotFound...
There was a problem hiding this comment.
It's a good idea Janos. I'll do that in the integration PR as I'd just like to get this merged more or less as-is so I can continue work on a rebased rc
| // DoneSplit sets total count to SPLIT count and sets the associated swarm hash for this tag | ||
| // is meant to be called when splitter finishes for input streams of unknown size | ||
| func (t *Tag) DoneSplit(address Address) int { | ||
| total := atomic.LoadUint32(&t.split) |
There was a problem hiding this comment.
There is a data consistency problem since DoneSplit is reading and writing two related values. Two calls to DoneSplit function at the same time can be scheduled in between two atomic calls in that function, resulting incorrect value stored into t.total.
There was a problem hiding this comment.
well it is supposed to be called only once, should be commented
There was a problem hiding this comment.
the data consistency problem here, IMO, is a non-issue as @zelig says it should be called only once. I'll comment that in the integration PR. There is, however, another problem which has to do with serializing the struct. There are multiple fields being read and we should have some synchronization around that.
* swarm/chunk: add tags backend to chunk package
* swarm/chunk: add tags backend to chunk package
This PR adds the tag data structure to the
chunkpackage.This is necessary for implementing push sync tags.
https://hackmd.io/9eWxJ_MJS8i04onWg49UBA?both
closes #1019