Page MenuHomePhabricator

Conpherence - fix a fatal
ClosedPublic

Authored by btrahan on Apr 9 2015, 4:46 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Mar 17, 12:42 AM
Unknown Object (File)
Sat, Mar 9, 4:22 PM
Unknown Object (File)
Feb 10 2024, 11:33 PM
Unknown Object (File)
Feb 6 2024, 10:19 AM
Unknown Object (File)
Jan 31 2024, 3:32 PM
Unknown Object (File)
Jan 16 2024, 7:56 PM
Unknown Object (File)
Dec 27 2023, 4:07 AM
Unknown Object (File)
Dec 19 2023, 12:36 AM

Details

Summary

Ref T7795.

I can't get this to reproduce and its confusing to me how its possible. The trace in T7795 uses the "LOAD" pathway on the update controller. Under the hood, this issues a ThreadQuery with needTransactions to true. With needTransactions to true, the transactions and pertinent handles are all loaded nicely.

So... best guess is there has been some LIMIT of transactions since the offending person participated...? Alternative fix which would probably work is to specify needParticipantCache to true.

More on T7795 - the user report found the "a, b, c..." subtitle thing in the messages dropdown confusing. Yet another fix here would be to change that to be something like "a: snippet of what a said...". I'll discuss that on the task.

Test Plan

iiam

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

btrahan retitled this revision from to Conpherence - fix a fatal.
btrahan updated this object.
btrahan edited the test plan for this revision. (Show Details)
btrahan added a reviewer: epriestley.
epriestley edited edge metadata.

In loadAndRenderUpdates(), we always seem to end up in getDisplayData(), but only the ConpherenceUpdateActions::METADATA pathway actually loads the participant cache.

If one of the recent participants has not said anything in the last 100 transactions, it seems like the other pathways should always break? That is, I'd expect something like this:

  • A and B join.
  • A says something.
  • B says something.
  • A says 105 things.

...to cause issues?

At some point, we can probably simplify this by using the newer Handle logic (which generally requires less up-front loading). This fix doesn't feel great but obviously won't hurt anything...

This revision is now accepted and ready to land.Apr 9 2015, 5:03 PM

Yeah, I'll probably hold this until T7795 product discussion is figured out and then fix this based on that.

I updated to HEAD and then applied this patch to the test server I created which had a repro based on T7795#106182, and this does appear to fix the server 500 error

btrahan edited edge metadata.

Fix the data loading directly

  • always load participant cache
  • change the query code slightly so you can load participant cache and participants (changed "or" to "and")
  • stop loading participant cache up front and only load participants in the narrow case we need it for settings
This revision was automatically updated to reflect the committed changes.

@nevogd - thanks for the testing. ended up fixing this in a slightly different way but if you update to HEAD it should still be resolved.