flumedb / flumelog-offset

MIT License
10 stars 8 forks source link

Add simple deletion #16

Closed christianbundy closed 5 years ago

christianbundy commented 5 years ago
christianbundy commented 5 years ago

Backlink: depends on https://github.com/flumedb/aligned-block-file/pull/5

arj03 commented 5 years ago

I read the commit, how does this affect performance? I see that stream now filters all elements, so I'm wondering what kind of overhead this might or might not have?

christianbundy commented 5 years ago

Thanks for the review! I took a look at the performance and noticed that isDeleted() was super slow because it was allocating an empty buffer and checking for equality. I've pushed a commit that resolves the performance regression, so the benchmarks look like:

master

validate 100000 974 8695.652173913044 11.5
flume 100000 10000 34352.45620061834 2.911
minimal 100000 2446 7200.979333189313 13.887
legacy 100000 436 4760.544606302961 21.006
random-read 100000 10000 59453.032104637336 1.682
random-read 100000 0 59453.032104637336 1.682

1351d03 (slow)

validate 100000 1541 8167.932696234583 12.243
flume 100000 10000 31735.9568390987 3.151
minimal 100000 2889 6796.71039217019 14.713
legacy 100000 851 4409.171075837742 22.68
random-read 100000 10000 49726.504226752855 2.011
random-read 100000 0 49726.504226752855 2.011

32a1392

validate 100000 969 8604.371020478404 11.622
flume 100000 10000 34094.78349812479 2.933
minimal 100000 2779 7303.534910896874 13.692
legacy 100000 600 4753.981459472308 21.035
random-read 100000 10000 56338.02816901409 1.775
random-read 100000 0 56338.02816901409 1.775
arj03 commented 5 years ago

Excellent @christianbundy! The commit looks good, I'm not sure if blanking is the best option, havn't thought deeply enough about that, but I trust you on that.

I really appreciate you driving deletion forward!

christianbundy commented 5 years ago

Thanks @arj03! If you think of any other techniques I'd be happy to hear them, this shouldn't require a migration or anything so we can always change the implementation if we come up with something clever.

I've got a call with Dominic in a bit, I'm going to check in with him before merging just in case. :+1:

christianbundy commented 5 years ago

Oops, this slipped my mind on the call -- any changes you'd like to see @dominictarr?