Page MenuHomePhabricator

Straighten out the fetch plan for feed stories
Closed, ResolvedPublic

Description

See T8575 for a brief mention. Feed/notifications have a tangled mess of a data fetch plan right now:

  • We load all the stories.
  • We load all the primary objects + transactions.
    • I believe ObjectQuery sometimes misses transactions (incorrectly discards them in a way that seems "at random", likely causing T8525).
    • Loading the transactions reloads many objects:
      • Transactions load their primary object, first.
      • I believe ObjectQuery frequently misses the workspace cache.
      • Transactions then load all their handles.
        • This should be off by default.
        • This should use handle pools.
        • I think this misses the workspace cache, too.
  • Now, we load all the required handles.
    • We already did a lot of this work.
    • This misses all the caches.
    • The earlier handle loading should happen here instead.
    • All of this should use handle pools.

In some cases, this ends up looking like: "Load transaction > load object, return; load handle > load object, return; load object; return; Load handle > load object, return". And I caught 6 loads on one profile instead of "only" 5, so that's not even the entire story.

Some of this is relatively easy to disentangle, some of it is a bit messier. Particularly, making transaction query default to not loading handles may be a bit of a mess.

Revisions and Commits

Restricted Differential Revision
Restricted Differential Revision
Restricted Differential Revision

Event Timeline

epriestley raised the priority of this task from to Normal.
epriestley updated the task description. (Show Details)
epriestley added a subscriber: epriestley.
epriestley added a revision: Restricted Differential Revision.Jun 29 2015, 9:52 PM
epriestley added a revision: Restricted Differential Revision.Jun 29 2015, 10:16 PM
epriestley added a revision: Restricted Differential Revision.Jun 29 2015, 10:18 PM
epriestley added a commit: Restricted Diffusion Commit.Jun 30 2015, 6:19 PM
epriestley added a commit: Restricted Diffusion Commit.
epriestley added a commit: Restricted Diffusion Commit.

This seems to be stable now, I don't immediately see any more ridiculously dumb slow stuff in the /feed/ profile. Remarkup handle fetches are still a bit loopy but that's a topic for another task.