changes on read path for supporting per user index with boltdb-shipper #5073
Conversation
| return err | ||
| } | ||
|
|
||
| downloadedFilesMtx.Lock() |
There was a problem hiding this comment.
not sure you actually need a lock if you use the index instead of appending.
There was a problem hiding this comment.
yeah, that should work, but we skip adding names of the files which went missing due to compaction. This change would leave some of the elements, and the caller will have to skip the empty file names, which would not look good.
|
|
||
| return nil | ||
| } | ||
| t.indexSetsMtx.Lock() |
There was a problem hiding this comment.
Since this lock is taken with the sync should we move that to another go routine, or make that index to be remove later ?
There was a problem hiding this comment.
Sorry, didn't get you here. Do you mean removing broken indexSet somewhere else and not at query time?
There was a problem hiding this comment.
Yeah, I think it's fine to set them aside or ask a worker pool to do this. I don't like the lock basically.
There was a problem hiding this comment.
The only reason I did this here is that I wanted to detect and recover from it asap. A worker pool would have to check it periodically and until then the read path would have to keep returning an error. If we clear it up sooner then the next read request could try to initialize it again. What do you think?
There was a problem hiding this comment.
Sure you don't have to do it now let's see how it goes
What this PR does / why we need it:
Follow up for adding support for per user index in boltdb-shipper. This PR only includes changes on the read path.
The first PR was #5026.
Checklist