maxBytesPerFile read check off-by-one #15
Conversation
|
Please see the newest pr code. use curReadFileSize instead of maxBytesPerFile for move to next file.
|
diskqueue.go
Outdated
| @@ -625,7 +644,16 @@ func (d *diskQueue) ioLoop() { | |||
| count = 0 | |||
| } | |||
|
|
|||
| if (d.readFileNum < d.writeFileNum) || (d.readPos < d.writePos) { | |||
There was a problem hiding this comment.
this expression did not need to change (and the original is more concise)
There was a problem hiding this comment.
Yes, the change just for my debug, and forget change back.
I will change it back, and submit another pr.
There was a problem hiding this comment.
no need for another PR: you can do a git interactive-rebase, and force-push this branch, to make any change to this PR
| // the value can change without affecting runtime | ||
| if d.nextReadPos > d.maxBytesPerFile { | ||
| // only readFileNum less than writeFileNum need move next file | ||
| if d.readFileNum < d.writeFileNum && d.nextReadPos >= d.curReadFileSize { |
There was a problem hiding this comment.
I'm not sure if d.readFileNum < d.writeFileNum needed to be added here, I'll think on it a bit...
Adding a test or two would help :) In particular, one that fails with current master, and passes with this branch. Note that Travis-CI currently fails to post "commit status" for the "legacy travis-ci.org integration". I've updated a couple of other repos to use GitHub Actions instead, I'll get around to this one eventually. In the meantime, you can view test results at https://travis-ci.org/github/nsqio/go-diskqueue/pull_requests or run them locally.
There was a problem hiding this comment.
when d.readFileNum == d.writeFileNum means reader and writer is same file, so when d.nextReadPos >= d.curReadFileSize the readFile can't be closed, the program should be waiting writeFile update d.curReadFileSize
And I think just pass now unit test will be ok.
|
The new |
|
I use the I use |
|
The logic here looks good and addresses an outstanding TODO, I've cleaned things up in #23, so let's wrap this up there. Thanks! |
No description provided.