microsoft / microsoft/FASTER

Deserializing page content despite errorCode != 0?

Open
#883 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C#
Stars
6.6k
Forks
595
PR merge metrics
No merged PRs in 30d

Description

While investigating failures of Netherite in customer code (see here) I noticed a stack trace where OOM exceptions were thrown from FASTER at a time when shutting down, which is surprising because at that point all outstanding memory operations were just being cancelled - so I was not expecting any OOMs to be thrown.

System.OutOfMemoryException: Exception of type 'System.OutOfMemoryException' was thrown.
at System.IO.BinaryReader.ReadBytes(Int32 count)
at DurableTask.Netherite.Faster.FasterKV.Value.Serializer.Deserialize(Value& obj) in //src/DurableTask.Netherite/StorageLayer/Faster/FasterKV.cs:line 1594
at FASTER.core.GenericAllocator2.Deserialize(Byte* raw, Int64 ptr, Int64 untilptr, Record2[] src, Stream stream)
at FASTER.core.GenericAllocator`2.AsyncReadPageWithObjectsCallback[TContext](UInt32 errorCode, UInt32 numBytes, Object context)
at DurableTask.Netherite.Faster.AzureStorageDevice.CancelAllRequests() in /
/src/DurableTask.Netherite/StorageLayer/Faster/AzureBlobs/AzureStorageDevice.cs:line 246
at System.Threading.CancellationToken.<>c.b__12_0(Object obj)
at System.Threading.CancellationTokenSource.Invoke(Delegate d, Object state, CancellationTokenSource source)
at System.Threading.CancellationTokenSource.CallbackNode.<>c.b__9_0(Object s)
at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
--- End of stack trace from previous location ---
at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
at System.Threading.CancellationTokenSource.CallbackNode.ExecuteCallback()
at System.Threading.CancellationTokenSource.ExecuteCallbackHandlers(Boolean throwOnFirstException)

Taking a closer look at AsyncReadPageWithObjectsCallback, I can see that the errorCode is being basically ignored (other than for logging). I don't understand why it is o.k. for this code to read and deserialize the results even though this callback is a cancellation, i.e. the read was never completed?

private void AsyncReadPageWithObjectsCallback<TContext>(uint errorCode, uint numBytes, object context)
{
    if (errorCode != 0)
    {
        logger?.LogError($"AsyncReadPageWithObjectsCallback error: {errorCode}");
    }

    PageAsyncReadResult<TContext> result = (PageAsyncReadResult<TContext>)context;

    Record<Key, Value>[] src;

    // We are reading into a frame
    if (result.frame != null)
    {
        var frame = (GenericFrame<Key, Value>)result.frame;
        src = frame.GetPage(result.page % frame.frameSize);
    }
    else
        src = values[result.page % BufferSize];


    // Deserialize all objects until untilptr
    if (result.resumePtr < result.untilPtr)
    {
        MemoryStream ms = new(result.freeBuffer2.buffer);
        ms.Seek(result.freeBuffer2.offset, SeekOrigin.Begin);
        Deserialize(result.freeBuffer1.GetValidPointer(), result.resumePtr, result.untilPtr, src, ms);
        ms.Dispose();

        result.freeBuffer2.Return();
        result.freeBuffer2 = null;
        result.resumePtr = result.untilPtr;
    }

    // If we have processed entire page, return
    if (result.untilPtr >= result.maxPtr)
    {
        result.Free();

        // Call the "real" page read callback
        result.callback(errorCode, numBytes, context);
        return;
    }

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with GenericAllocator.AsyncReadPageWithObjectsCallback and the cancellation path in AzureStorageDevice.CancelAllRequests(), then inspect the deserialization call shown in the issue. Reproduce or trace shutdown with a nonzero errorCode and determine the expected callback behavior. Done means the failure path is understood and covered by an appropriate regression test without introducing out-of-memory deserialization.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.