Implement topics for store nodes #120
Conversation
pkg/store/log.go
Outdated
| newLog func(string) (Log, error) | ||
| mtx sync.Mutex | ||
| m map[string]Log | ||
| } |
There was a problem hiding this comment.
This essentially adds coordination across actors, which is not great.
If we remove the lock file from the FileLog to a single one for the top level data dir we can get rid of that again I think.
| func BenchmarkDemux(b *testing.B) { | ||
| for name, newFsys := range map[string]func() fs.Filesystem{ | ||
| "virtual": fs.NewVirtualFilesystem, | ||
| "real": fs.NewRealFilesystem, |
There was a problem hiding this comment.
BenchmarkDemux/virtual-8 2000000 562 ns/op 1079 B/op 3 allocs/op
BenchmarkDemux/real-8 2000000 1015 ns/op 408 B/op 3 allocs/op
PASS
There was a problem hiding this comment.
That seems good enough for the time being.
There was a problem hiding this comment.
The real number actually strongly depends on how big our bufio.Writer buffers are when fanning out to per-topic segments. Cranking it to a higher number makes it look nice, but probably the benchmark doesn't quite capture the right thing here yet?
Should we change it to actually benchmark against small fixed-sized staging segments?
cmd/oklog/ingeststore.go
Outdated
| // Run each compacters' next stage. | ||
| for _, c := range tc { | ||
| c.Next() | ||
| } |
There was a problem hiding this comment.
Using Run/Stop is not really feasible here I think as we may end up with 1000 compactors competing for IO. Some parallelism might be okay though if this cannot keep up.
I generally prefer handling lifecycle entirely in main over blackbox-y Run/Stop methods – which works especially great with run.Group.
| return "", id, nil, err | ||
| } | ||
| // Execution of return arguments is not ordered. We must cast the topic | ||
| // to a string separately. |
There was a problem hiding this comment.
This really surprised me as I assumed left-to-right order.
From https://golang.org/ref/spec#Order_of_evaluation:
when evaluating the operands of an expression, assignment, or return statement, all
function calls, method calls, and communication operations are evaluated in lexical
left-to-right order.
Does the "lexical" bit mean that return string(p[1]), append(b[... would be reordered because str... > app...?
| @@ -54,6 +55,10 @@ func (realFilesystem) Exists(path string) bool { | |||
| return !os.IsNotExist(err) | |||
| } | |||
|
|
|||
| func (realFilesystem) ReadDir(dirname string) ([]os.FileInfo, error) { | |||
| return ioutil.ReadDir(dirname) | |||
| } | |||
There was a problem hiding this comment.
I noticed you used Walk everywhere. Because of the annoying os.File.Readdir{,names} semantics?
Went with the ioutil.ReadDir signature instead unless we strictly want to match os.File.
fabxc
force-pushed
the
topics
branch
4 times, most recently
from
3d1ecd6
to
29f19d3
Compare
|
Moved file locking to main, which allowed things to get cleaned up a bit. |
| } | ||
| m[t] = l | ||
| } | ||
| return m, nil |
There was a problem hiding this comment.
Always instantiating new logs seems wasteful (allocs and syscalls) – but compared to when we read/write logs it probably won't matter.
|
@fabxc: Let us know when this is ready for first review. Perhaps adding a WIP label would be good this sort of PRs. |
|
Right, prefix added. |
fabxc
changed the title
|
@peterbourgon I ran into several issues in the virtual FS implementation, missing features and bugs alike, e.g. When I worked on something similar, I remember that I mostly dropped it because getting actual FS semantics was incredibly tedious and virtually impossible to get entirely right. I wonder whether it would be better to replace the virtual FS to be backed by a chrooted tmpfs for tests and such. |
|
I think this is generally good for review. I wonder how the staging log behaves in practice. If it gets evicted sufficiently fast, I'd hope the staging segments mostly never make it beyond the page cache in a busy server, thus not impacting the overall throughput limit as given by the disk. |
fabxc
changed the title
fs: add ReadDir method store: add topic demuxer store: handle topics in store store: remove per-log lock, refactor store: fix tests for topics Resolved oklog#120
fs: add ReadDir method store: add topic demuxer store: handle topics in store store: remove per-log lock, refactor store: fix tests for topics Resolved oklog#120
fs: add ReadDir method store: add topic demuxer store: handle topics in store store: remove per-log lock, refactor store: fix tests for topics Resolved oklog#120
fs: add ReadDir method store: add topic demuxer store: handle topics in store store: remove per-log lock, refactor store: fix tests for topics Resolved oklog#120
fs: add ReadDir method store: add topic demuxer store: handle topics in store store: remove per-log lock, refactor store: fix tests for topics Resolved oklog#120
This is more of a preview.