search

ssbc / ssb-db2

A new database for secure-scuttlebutt
47 stars 8 forks source link

Fix restart update indexes #350

Closed arj03 closed 2 years ago

arj03 commented 2 years ago

While debugging https://github.com/ssbc/ssb-db2/pull/349 I noticed that restartUpdateIndexes was behaving a bit unexpectedly. Compaction added a call to restartUpdateIndexes on startup even if there was nothing to do. This means that if you have an empty database both the normal updateIndexes call, but also the updateIndexes call from restartUpdateIndexes would be queued up for when indexesStateLoaded is ready. This means that you will get 2 calls right after another one calling the other. What this change does it to only call updateIndexes if abortLogStreamForIndexes is set. Meaning that updateIndexes are already running and it will be stopped because we abort those log streams.

staltz commented 2 years ago

I see the problem! Thanks for spotting it.

I'm confused about this whole thing, though. I probably was lucid when I wrote that code, but it doesn't seem now. If I remove the whole restartUpdateIndexes(), all tests still pass (especially compaction.js and compaction-resume.js).

Let me take some time to revise the post-compaction logic.

arj03 commented 2 years ago

Yes, was not 100% sure this is the correct fix, but anyway all tests are still passing. And there is def. something there so take your take to go through the logic :)

staltz commented 2 years ago

@arj03 I made this commit on the formats-split branch https://github.com/ssbc/ssb-db2/pull/347/commits/cdb000d7448b6d03ee0fbee680a9f31758cd69f9, I'm afraid of merge conflicts with master, so is it fine that we close #350 and use that commit instead?

arj03 commented 2 years ago

Sure