Sync HTTP requests should completely avoid the Necko cache

RESOLVED WORKSFORME

Status

Cloud Services
Firefox Sync: Backend
RESOLVED WORKSFORME
6 years ago
6 years ago

People

(Reporter: briansmith, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

+++ This bug was initially created as a clone of Bug #752243 +++

According to Bug 752243, some part of these tests fail due to a problem with the Necko cache as part of running the tests.

But, are HTTP caching semantics suitable for the HTTP requests that Sync makes? AFAICT, they are not, and any cache I/O we are doing for Sync requests is just a waste of time.

If Sync doesn't make any such requests, then Sync should use LOAD_BYPASS_LOCAL_CACHE | INHIBIT_CACHING to avoid reading/write the cache entry from/to the cache, for performance reasons.

INHIBIT_CACHING is especially important to prevent responses from the Sync server from filling up our HTTP cache and pushing out more useful entries.
This is how we create a request:

  _createRequest: function Res__createRequest(method) {
    let channel = Services.io.newChannel(this.spec, null, null)
                          .QueryInterface(Ci.nsIRequest)
                          .QueryInterface(Ci.nsIHttpChannel);

    // Always validate the cache:
    channel.loadFlags |= Ci.nsIRequest.LOAD_BYPASS_CACHE;
    channel.loadFlags |= Ci.nsIRequest.INHIBIT_CACHING;

Should we be setting LOAD_BYPASS_LOCAL_CACHE as well?
That should do the trick already, since we have:

   #define BYPASS_LOCAL_CACHE(loadFlags) \
        (loadFlags & (nsIRequest::LOAD_BYPASS_CACHE | \
                      nsICachingChannel::LOAD_BYPASS_LOCAL_CACHE))

and we always use BYPASS_LOCAL_CACHE(loadFlags) for testing this condition in nsHttpChannel.

Looking again at bug 752243 comment 1, is it possible that we're using these flags for the Sync record data transfers, but not for other Sync requests?

/Users/cltbld/talos-slave/test/build/xpcshell/tests/services/sync/tests/unit/test_service_createAccount.js

Otherwise, perhaps the fact that the other bug is witnessed during a Sync test is a red hering, and perhaps this bug is just INVALID.

Comment 3

6 years ago
*All* Sync HTTP requests go through one of 2 HTTP request libraries:

resource.js:
https://hg.mozilla.org/services/services-central/file/default/services/sync/modules/resource.js#l173

rest.js:
https://hg.mozilla.org/services/services-central/file/default/services/common/rest.js#l122
https://hg.mozilla.org/services/services-central/file/default/services/common/rest.js#l271

As you can see, we use LOAD_BYPASS_CACHE and INHIBIT_CACHING for both.

Per comment #2, we already fill the requirement of disabling the cache, so WORKSFORME.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.