Skip to content

Change to avoid overwriting data length bytes#22

Open
christianbundy wants to merge 1 commit into
masterfrom
no-bookend
Open

Change to avoid overwriting data length bytes#22
christianbundy wants to merge 1 commit into
masterfrom
no-bookend

Conversation

@christianbundy

Copy link
Copy Markdown
Member

As mentioned to me in a comment by @arj03, there's really no reason to
be overwriting the length of the data since that doesn't change.

As mentioned to me in a comment by @arj03, there's really no reason to
be overwriting the length of the data since that doesn't change.
@arj03

arj03 commented Dec 28, 2019

Copy link
Copy Markdown
Contributor

That looks right.

The reason why I found this was that I was looking at implementing delete in browser and then realized that flumelog-aligned-offset doesn't have a del method, so I was looking at what you did here and porting that over. One thing that I noticed was that it slows down stream. How did you go about benchmarking this back then?

@christianbundy

Copy link
Copy Markdown
Member Author

One thing that I noticed was that it slows down stream. How did you go about benchmarking this back then?

I think I was just running bench-ssb in this comment, does that look right to you? I think the slightly slower streaming is probably because it has to check whether the item is all null bytes. For most messages it only looks at the first byte, but if a messages has been deleted it looks at every byte to see whether the item has been deleted.

@dominictarr

Copy link
Copy Markdown
Collaborator

I suggested using this filtering approach. Also the views work like this. flumeview-level still has indexes for deleted records, but they are filtered at read time. If nothing has been deleted, then it will still be just as fast. If a few things have been deleted then it will be slightly slower... To make it full speed you'd need to recompact the log file, and rebuild the indexes, which will take a while, that would make sense if you really deleted a LOT of stuff though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants