nsqd: enforce minimum msg size when reading from the diskqueue #600
Conversation
mreiferson
changed the title
| // TODO: this is off by one, but we cannot fix this without also fixing | ||
| // readOne above. We cannot fix readOne above until we guarantee that | ||
| // every old diskqueue format has been fully processed, otherwise we | ||
| // will miss one message from old formatted files. |
There was a problem hiding this comment.
what do you mean by this comment?
There was a problem hiding this comment.
This is meant to rotate files when we reach maxBytesPerFile, but will only
rotate when we exceed that number. If max bytes is 1024 and we are at bytes
1024, we will write one more message before rotating files.
There was a problem hiding this comment.
Fair enough, however for all practical purposes I don't think this is something that needs to be addressed other than perhaps the documentation around the config param.
Can we remove this comment?
|
nice work on this @twmb - thanks for adding all those comments to the tests! |
… diskqueue Prevents panics in the face of corrupt messages.
| @@ -261,6 +266,14 @@ func (d *diskQueue) readOne() ([]byte, error) { | |||
| return nil, err | |||
| } | |||
|
|
|||
| if msgSize < d.minMsgSize || msgSize > d.maxMsgSize { | |||
There was a problem hiding this comment.
I don't know if msgSize > d.maxMsgSize means it's corrupt? It should skip it, but does it need to discard the whole segment?
There was a problem hiding this comment.
msgSize could be > d.maxMsgSize because of either
a) a corrupt message, or
b) a person changed the flag of their maxMsgSize between nsqd restarts
If it's a corrupt message, then we'd skip forward in the file and then have no guarantee of what we're reading. If a person changed their maxMsgSize, they'd miss this message and be fine reading the next. I don't think it's too common to reduce MaxMsgSize between nsqd restarts other than when a person is trying to figure out parameters, and if they're fine with losing any large messages when tuning, I can't see why they wouldn't be fine losing an entire segment.
I favor discarding the file but I could go either way.
There was a problem hiding this comment.
Alright, you've sold me - all of this falls under "reasonable behavior" without actually knowing if it's corrupt.
nsqd: enforce minimum msg size when reading from the diskqueue
See #598