wal, ioutil: set page offset for encoder #6544
Conversation
| @@ -38,9 +36,10 @@ type PageWriter struct { | |||
| bufWatermarkBytes int | |||
| } | |||
|
|
|||
| func NewPageWriter(w io.Writer, pageBytes int) *PageWriter { | |||
| func NewPageWriter(w io.Writer, pageOffset, pageBytes int) *PageWriter { | |||
There was a problem hiding this comment.
NewPageWriter(w io.Writer, pageBytes, pageOffset int) *PageWriter -- order the arguments by importance; the page size is more important to the operation of the writer than the offset since the offset can be dropped in the default cases (i.e., a new file) and the writer will work, but the page size must always be provided.
| @@ -377,7 +384,7 @@ func (w *WAL) cut() error { | |||
| // update writer and save the previous crc | |||
| w.locks = append(w.locks, newTail) | |||
| prevCrc := w.encoder.crc.Sum32() | |||
| w.encoder = newEncoder(w.tail(), prevCrc) | |||
| w.encoder = newEncoder(w.tail(), int(off), prevCrc) | |||
There was a problem hiding this comment.
this is writing to a new file, the offset should be 0
| @@ -39,9 +39,9 @@ type encoder struct { | |||
| uint64buf []byte | |||
| } | |||
|
|
|||
| func newEncoder(w io.Writer, prevCrc uint32) *encoder { | |||
| func newEncoder(w io.Writer, offset int, prevCrc uint32) *encoder { | |||
There was a problem hiding this comment.
Passing the offset into this is evidently error-prone. Have a second function newFileEncoder(f *os.File, prevCrc uint32) *encoder instead of adding the offset to this function?
There was a problem hiding this comment.
Right will fix. Thanks.
|
@heyitsanthony PTAL. Thanks. |
|
lgtm |
Fix #6515.
Previously page writer was not aware of page offset of the given writer. Then mis-computed actual offset of unaligned pages. This passes the starting page offset to page writer to position the right offset for next flushing.
/cc @heyitsanthony Please review.
Do you think tests in wal is needed? The offset calculation is not exported though.