VWR-24889: When a bake texture upload fails, retry instead of giving up.
Review Request #152 - Created Feb. 17, 2011 and submitted
| Thickbrick Sleaford | Reviewers | ||
| viewer | |||
| VWR-24889 | |||
| None | viewer-development | ||
When a bake upload fails, the viewer doesn't retry it, and subsequently doesn't send a AgentSetAppearance message. This can happen without the user being aware, leaving the avatar looking good on their screen, but not updated to the same outfit on other people's screens. The avatar will remain in that state until the user does something that causes a rebake (manually rebake or change outfit.) The solution here is to retry the upload after a small delay.
What this diff changes: when a full-res upload fails, retry to upload it after a 5s delay, up to 5 times (in case the cap is available, last attempt is via the old asset store.) Also, some clearer log messages. This implements an old *FIX: comment:
// *FIX: retry upload after n seconds, asset server could be busy
This isn't needed for low res uploads, because they don't block subsequent full-res uploads (mNeedsUpload isn't set to FALSE in LLTexLayerSetBuffer::doUpload in low-res uploads.)
Attempted outfit changes using a problematic connection (not recently used outfits to avoid using cached bakes). Looked for "Baked full res texture upload for <region name> failed" log messages, observed the subsequent retries and successful upload for that region. Observed that eventually the fully-baked avatar is visible to other users.
Expand AllPosted (Feb. 17, 2011, 1:59 p.m.)
-
indra/newview/lltexlayer.h (Diff revision 1) -
Is there any reason why you use BOOL instead of bool for this new data member? If not, prefer bool. (See https://wiki.secondlife.com/wiki/Coding_standard#Linden_Variable_Types )
-
indra/newview/lltexlayer.cpp (Diff revision 1) -
Is "res" for "resolution" or "result"?
Review request changed
Updated (Feb. 19, 2011, 9:08 a.m.)
-
- added Diff r2
- changed from When a bake texture upload fails, retry instead of giving up. to VWR-24889: When a bake texture upload fails, retry instead of giving up.
Posted (Feb. 25, 2011, 7:56 p.m.)
Apart from what Thickbrick already pointed out and the typo here under, I don't have anything against that patch. We should test live though and make sure that 5 times 5 sec is not too much for a retry loop (seems a bit long to me).
-
indra/newview/lltexlayer.h (Diff revision 2) -
typos in comment: "whether it is a final bake or intermediary low res"
Review request changed
Updated (March 1, 2011, 8:28 a.m.)
-
- added Diff r3
New diff, fixes the typo in the comment pointed out by Merov.
Posted (March 1, 2011, 8:52 a.m.)
-
indra/newview/lltexlayer.h (Diff revisions 2 - 3) -
"intermediary" is still misspelled
Review request changed
Updated (March 1, 2011, 8:58 a.m.)
-
- added Diff r4
More comment spelling.
Posted (June 10, 2011, 4:38 a.m.)
-
indra/newview/lltexlayer.h (Diff revision 4) -
better to define an enum here rather than use a bool: "{ finalBake, lowresBake }"
Posted (June 13, 2011, 8:20 a.m.)
-
indra/newview/lltexlayer.h (Diff revision 4) -
This would be more clear as an enum: when looking at a call to this method, a value 'true' or 'false' is much less informative that 'isHighestRes' or 'isLowRes'
-
indra/newview/lltexlayer.cpp (Diff revision 4) -
These should be 'static'; they don't need to be visible to the linker.
-
indra/newview/lltexlayer.cpp (Diff revision 4) -
what's with the minus one here?
Review request changed
Updated (June 13, 2011, 2:39 p.m.)
-
- added Diff r5
- Fixed typo in comment ("intermediary").
- Changed file-scope constants to static const.
- At Oz's suggestion (in email), delays are now exponential. Oz suggested leaving this to hammer the server indefinitely, but I feel more comfortable with a limit of 7 attempts. The first attempt is immediate, and 6 retries are done with delays: 2s, 4s, ... 64s. The delays sum up to 126 seconds.
Other reviews