spirit-labs / tektite

Tektite DB
http://www.tektitedb.com
Other
180 stars 26 forks source link

Do not unnecessarily merge non overlapping L0 groups in query results #242

Closed watson28 closed 2 months ago

watson28 commented 2 months ago

solves: https://github.com/spirit-labs/tektite/issues/187

watson28 commented 2 months ago

This doesn't work, as it assembles query results based on a combination of the readable state, and the current writable state, protected by a RW lock. Using a lock also defeats the purpose of readable state which is to avoid locking. If a compaction or registration occurs, it will get a write lock which will block all readers causing queries to stall while that is happening. I think what we need to do is to add everything that we need to create the query results to the readable state.

@purplefox Thanks for the explanation. I have removed the lock and added sstProcessorMap to the readable state.

I think it could be a good idea to extract the code of query/reading from the level_manager.go into its own module so it's clear the dependencies used for generating query results and thus easily verify any lock contention. For instance, the current QueryTablesInRange function depends on segmentCache ( QueryTablesInRange -> getOverlappingTables -> getSegment -> segmentCache.get. And segmentCache control access with a mutex lock so I assume there is contention when registering new tables applyRegistrations().

purplefox commented 2 months ago

Hey @watson28 sorry for the slow reply to this, and thank you for your work on it! Over the last few days I've been thinking of some changes to the level manager which simplifies things and make its easier to partition our write load to the LSM. One result of that is that processor groups will disappear, which sadly makes this work redundant.