Strong consistency is not guaranteed

[ishowta]

[REQUIRED] Describe your environment

• Operating System version: WSL Ubuntu 20.04 on Windows 20H2
• Browser version: Chrome 97.0.4692.71
• Firebase SDK version: master branch (4bd3a888353c26854142b1861f12e0222151af96)
• Firebase Product: firestore

[REQUIRED] Describe the problem

When I call DocumentReference.get() asynchronously, update the document synchronously, and then do DocumentReference.get() again, I expect to get the last updated document, however, sometimes the old data may be retrieved. (See the code below.)

DocumentReference.get() looks like it's internally treating the request as a query and passing it to the EventManager, but EventManagerImpl manages the same query in one place, so this probably causes the new request to be lumped in with the old one.

https://github.com/firebase/firebase-js-sdk/blob/4bd3a888353c26854142b1861f12e0222151af96/packages/firestore/src/core/event_manager.ts#L64

Steps to reproduce:

The code below (Occasional failures)

Relevant Code:

it('get document while online with default get options keep strong consistency', () => {
    const initialData = { key: 'initial' };
    const updatedData = { key: 'updated' };
    return withTestDocAndInitialData(persistence, initialData, async docRef => {
        // 1. async get
        void getDoc(docRef);

        // 2. sync update (Or the same with calling functions that update data)
        await updateDoc(docRef, updatedData);

        // 3. get
        // Maybe if request 1 is resolved late, the same data as request 1 will be returned
        // I think this behavior is breaking strong consistency.
        const dataGotAfterUpdate = await getDoc(docRef);

        expect(dataGotAfterUpdate.data()).to.deep.equal(updatedData);
    });
});

[schmidt-sebastian]

Thanks for providing this test case. The test mostly passes for me but let's look at one of the failed runs. Note that the test only fails with persistence disabled. If persistence is enabled, the test always passes as the document is always in the cache.

Logs are here: https://gist.github.com/schmidt-sebastian/6979dc61b8acf35b020aa045856cfb1a

• The initial document is written to the backend. As there are no active listeners, it is immediately cleared from cache.
• The first getDoc is issued, but nothing happens yet.
• updateDoc is sent to the backend. The backend does not yet sent a response.
• The first getDoc is sent to the backend.
• The backend acknowledges the update at time 2. We can't persist anything in the cache since we do not have a base document.
• The backend sends a the response to getDoc at time 1. Note that this is according to our specification as the read for the document occurred before the update was written (update at nanos: 969724000 versus read at nanos: 954131000)
• The second getDoc then runs. We are now in the short window were we have the result of the first getDoc call in the cache and so we return it. If the first getDoc had already returned its result to your code, the document would no longer be in the cache and we would issue another backend call and you would see the updated data.

Since memory persistence does not store any information when there are no active listeners, there is likely not a realistic way for us to prevent this scenario. This problem will not exist with memory persistence or if you keep your listeners active. I will discuss with my team if we should treat this case specifically.

Thank you for this exercise!

[ishowta]

Thank you for your detailed investigation!

By the way, I didn't include it because it's hard to reproduce, but what I actually encountered was a problem via the functions call like below.

I don't understand about the behavior of memory persistence yet, but in this case, I have a feeling that memory persistence won't work since doc is being changed externally.

(I'm using a deepl translator for most of the text, so it may be hard to read. I'm sorry.)

// 1. async get (Somewhere.)
void getDoc(docRef);

// 2. sync function call
await functions.httpsCallable('updateData')();

// 3. get (I'm actually opening another page after calling the function and getting the doc there.)
const dataGotAfterUpdate = await getDoc(docRef);