Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unexpected missing key on 3rd attempt #463

Closed
timothycoleman opened this issue May 2, 2021 · 2 comments · Fixed by #465
Closed

Unexpected missing key on 3rd attempt #463

timothycoleman opened this issue May 2, 2021 · 2 comments · Fixed by #465

Comments

@timothycoleman
Copy link
Contributor

timothycoleman commented May 2, 2021

I've found some surprising (at least to me) behaviour where I can successfully read a key twice, but it fails to find it the third time. My key is variable length, but my value is fixed length. Repro below, this test fails for me against current master.

[TestFixture]
internal class ReproSpanByteKey
{
    class Functions : FunctionsBase<SpanByte, long, long, long, Empty>
    {
        public override void SingleReader(ref SpanByte key, ref long input, ref long value, ref long dst) => dst = value;

        public override void ConcurrentReader(ref SpanByte key, ref long input, ref long value, ref long dst) => dst = value;
        public override void ReadCompletionCallback(ref SpanByte key, ref long input, ref long output, Empty ctx, Status status) { }
    }

    [Test]
    public void Repro()
    {
        var log = Devices.CreateLogDevice(TestContext.CurrentContext.TestDirectory + "/BasicFasterTests.log", deleteOnClose: true);
        var fht = new FasterKV<SpanByte, long>(
            size: 1L << 27,
            new LogSettings
            {
                LogDevice = log,
                MemorySizeBits = 15,
                PageSizeBits = 12,
            });
        var session = fht.For(new Functions()).NewSession<Functions>();

        // write enough records to be flushed to disk
        for (int i = 0; i < 5000; i++)
        {
            var keyString = $"{i}";
            var key = MemoryMarshal.Cast<char, byte>(keyString.AsSpan());
            session.Upsert(SpanByte.FromFixedSpan(key), i);
        }
        session.CompletePending(wait: true);

        // read "5" twice successfully
        Assert.AreEqual(5, ReadKey5());
        Assert.AreEqual(5, ReadKey5());

        // BUT not found on the third time
        Assert.AreEqual(5, ReadKey5());

        long ReadKey5()
        {
            var keyString = $"5";
            var key = MemoryMarshal.Cast<char, byte>(keyString.AsSpan());

            var status = session.Read(key: SpanByte.FromFixedSpan(key), out var unused);

            // key low enough to need to be fetched from disk
            Assert.AreEqual(Status.PENDING, status);

            session.CompletePendingWithOutputs(out var completedOutputs, wait: true);

            var count = 0;
            var value = 0L;
            while (completedOutputs.Next())
            {
                count++;
                ref var completedKey = ref completedOutputs.Current.Key;
                var completedStringKey = new string(MemoryMarshal.Cast<byte, char>(completedKey.AsReadOnlySpan()));
                Assert.AreEqual(Status.OK, completedOutputs.Current.Status);
                Assert.AreEqual(5, completedOutputs.Current.Output);
                value = completedOutputs.Current.Output;
            }
            completedOutputs.Dispose();

            Assert.AreEqual(1, count);
            return value;
        }
    }
}
@badrishc
Copy link
Collaborator

badrishc commented May 2, 2021

This was a dispose bug with the new API (CompletePendingWithOutputs) we added last week. Thanks for the catch. Using the older stable API of CompletePending will work just fine as workaround (getting result via the Read completion callback in IFunctions).

The linked PR fixes the issue as well, adding this as a testcase.

Also, note that for the SpanByte based calls to FASTER, as warned in the tooltip for FromFixedSpan, you need to fix the Span for the duration of the synchronous operation, since it points to heap memory (strings) that may shift around due to GC:

                fixed (byte* f = key)
                    session.Upsert(SpanByte.FromFixedSpan(key), i);

                fixed (byte* f = key)
                    var status = session.Read(key: SpanByte.FromFixedSpan(key), out var unused);

@timothycoleman
Copy link
Contributor Author

Brilliant, thanks!

Good point re fixing, my original key was stackalloc'ed but I seem to have changed that without adding the fixing 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants