Improve commitlog bootstrapping reliability in the face of sudden failures #1065
Conversation
richardartoul
commented
Oct 10, 2018
•
richardartoul
commented
- Prevent commit log reader from deadlocking when faced with certain types of commit log file corruption
- Add regression property test to verify that the commit log reader never deadlocks or gets stuck regardless of any kind of random corruption
richardartoul
requested review from
prateek and
robskillington
and removed request for
prateek
Codecov Report
@@ Coverage Diff @@
## master #1065 +/- ##
==========================================
+ Coverage 77.35% 77.37% +0.02%
==========================================
Files 541 541
Lines 45917 45924 +7
==========================================
+ Hits 35518 35534 +16
+ Misses 8142 8137 -5
+ Partials 2257 2253 -4
Continue to review full report at Codecov.
|
src/dbnode/persist/fs/types.go
Outdated
| @@ -460,3 +460,13 @@ type BlockRetrieverOptions interface { | |||
| // IdentifierPool returns the identifierPool | |||
| IdentifierPool() ident.Pool | |||
| } | |||
|
|
|||
| // FD is the interface implemented by *os.File. | |||
| type FD interface { | |||
There was a problem hiding this comment.
nit: move this to m3/src/x/os and call it File instead of FD
| @@ -80,6 +80,13 @@ type commitLogWriter interface { | |||
| Close() error | |||
| } | |||
|
|
|||
| type chunkWriterIface interface { | |||
| reset(f fs.FD) | |||
There was a problem hiding this comment.
nit: use uniform casing in method names
There was a problem hiding this comment.
Some have to be public to implement other interfaces, other methods should probably just be private...but I guess its a private interface so I guess it doesnt matter
There was a problem hiding this comment.
embed the interface's rather than copying methods if you can help it.
There was a problem hiding this comment.
that worked actually, cheers
| @@ -80,6 +80,13 @@ type commitLogWriter interface { | |||
| Close() error | |||
| } | |||
|
|
|||
| type chunkWriterIface interface { | |||
There was a problem hiding this comment.
naming nit: not a fan of the Iface - how about calling the interface chunkWriter and the usual struct fsChunkWriter?
There was a problem hiding this comment.
yeah sure, I didn't like this either
richardartoul
changed the title
| @@ -122,12 +124,25 @@ func TestCommitLogReadWrite(t *testing.T) { | |||
| } | |||
|
|
|||
| func TestCommitLogPropTest(t *testing.T) { | |||
| // Temporarily reduce size of buffered channels to increase change of | |||
