celestiaorg / go-header

Go library with all the services needed to request, sync and store blockchain headers.
Apache License 2.0
17 stars 16 forks source link

store: use less caches #189

Closed cristaloleg closed 1 month ago

cristaloleg commented 1 month ago

Currently store.Store contains a few caches:

At the same time we access datastore (ds) directly BUT we also have a cache on top of it (see heightIndex above). Both heightSub and pending can be also treated as caches but let's omit this for a second.

I'm proposing to refactor store.Store internals to keep only caching datastore that can return header by hash or height from cache and if it doesn't exist - fetch from real datastore (ds).

Wondertan commented 1 month ago

So do you propose instead of caching deserialized headers to cache serialized headers on datastore level? Whats the benefit of that?

Also,heightSub is not a cache and pending will be removed during backward syjc work.

cristaloleg commented 1 month ago

caching deserialized headers to cache serialized

No, I'm not saying that. I'm proposing to refactor store.Store internals to have 1 cache over datastore. But with a new day and a fresh head I see that will not work due to different patterns of access (height and hash).

heightSub is not a cache

I mentioned that 😉

pending will be removed

I don't recall that we've discussed that. Why pending will be removed? It's a batch of what we're going to write, we haven't discussed that we change the part how we collect data before writing it to the underlying datastore 🤔

Gonna close one but the question above is still open, waiting for you answer Hlib

Wondertan commented 1 month ago

As you correctly stated, the pendingCache is the cache of writes that needs to happen to the store. It exists in first place because of the adjacency issue. If we can start writing things in the store in any order the need for the pending cache disappears and we can remove it to simplify