mgravell / FASTERCache

IDistributedCache implementation on FASTER
MIT License
30 stars 2 forks source link

Async path not working properly for transition to disk based files #7

Closed Tornhoof closed 5 months ago

Tornhoof commented 5 months ago

I tried to add proper tests for the async path, by reducing the actual log sitze. The page size is now 4096 byte, the memory size is 16kb, the disk segment size is 32kb. The test writes less than pagesize data (multiple times) into the cache and tries to read them again. As soon as the cache hits the async path (actually writing data to disk) the test fails, you can see that the file is created (it's not created before that) and you can put in a breakpoint in `AsyncTransitionGet.

The test reads the keys in reverse, so the last 8 keys work for a size of 1kb, which is exactly half of the memory size, after which faster pushes the data to disk.

I found one bug (you never incremented the count for your CompleteSinglePending), but that's it.

I couldn't find the part why it fails, might take another look tomorrow.

mgravell commented 5 months ago

that count++; should be merged ASAP; is the other stuff OK to merge, even if it is a failing test (that demonstrates a problem)?

Tornhoof commented 5 months ago

Yeah it's a failing test, all the other tests still work as the data is too small for the transition to disk.

mgravell commented 5 months ago

thanks; I've run the test - pretty sure it is a "mutable struct / async result box" problem; I know some tricks, will look tomorrow

Tornhoof commented 5 months ago

At some later stage, we probably need to reduce some of the sizes in the default settings anyway, they are at the moment:

        /// <summary>
        /// Size of a segment (group of pages), in bits
        /// </summary>
        public int PageSizeBits = 25;

        /// <summary>
        /// Size of a segment (group of pages), in bits
        /// </summary>
        public int SegmentSizeBits = 30;

        /// <summary>
        /// Total size of in-memory part of log, in bits
        /// </summary>
        public int MemorySizeBits = 34;

34bit for the in memory cache, meaning 33bit until it flushes to disk is 16Gb, which is probably way too large for any sane default value. In my impl, I have smaller values, the page size is still 32mb, the segment size is 128mb and the memory size is 1Gb, which fits the access pattern in the app I use it for fairly well.

mgravell commented 5 months ago

I think the problem was copy update; I don't understand what that means, but this seems to fix it - if you know whether this looks right, I'd love your input

Tornhoof commented 5 months ago

Ah the functions, I kinda expected something like that, because that's the area in Faster i have no experience with.

So great it works and no I have no idea why.