Closed
Bug 1030961
Opened 11 years ago
Closed 11 years ago
Consider optimizing url load strategy for the Loop panel on or before open
Categories
(Hello (Loop) :: Client, defect, P1)
Hello (Loop)
Client
Tracking
(Not tracked)
RESOLVED
WONTFIX
| backlog | - |
People
(Reporter: standard8, Unassigned)
References
Details
Attachments
(1 file)
|
469 bytes,
text/x-python
|
Details |
Splitting this out from bug 1000770. We should consider whether or not we need to optimize loading of the url, maybe saving previously generated, unused urls.
Comments 11 to 20 of bug 1000770 are the main discussion. The main suggestion so far is given below.
IMO we should test user feedback and get stats on the UI implemented in that bug before considering this further, as this will require additional work on the server side.
Suggestion from bug 1000770 comment 20:
- When the panel opens, we check to see whether an "unused" URL is cached.
- If so, we (a) present it to the user; and (b) send off a request to update
expiration of the URL via |POST /call-url/{token}|.
- If not, we get a new token from the server and, when it returns, render it to the user.
This URL is written to disk so that it survives browser restarts.
- When the user takes any action that could exfiltrate the URL, we mark it as "used"
(this is a client designation, not something shared with the server). This ensures that
a new URL is generated the next time the panel opens. These actions include:
- Clicking on any of the API that we use to send via email or SMS
- Copying the URL to the clipboard -- if we can't reliably detect this, then we can use
text-box focus as a proxy for that action.
Comment 1•11 years ago
|
||
tagged for 34 so we revisit after we've gotten some feedback
Target Milestone: --- → mozilla34
Updated•11 years ago
|
Target Milestone: mozilla34 → mozilla35
Comment 2•11 years ago
|
||
Here is a proposal on how to handle this on the client:
- Generate an URL and store it somewhere locally;
- When the user selects it, consider it used;
- Do not generate new URLs if the user doesn't ask explicitely, or if the user isn't marked as used.
Not having something like this means the server is storing a bunch of useless URLs, so fixing it would be helpful!
Updated•11 years ago
|
Priority: -- → P2
Comment 3•11 years ago
|
||
This starts to fill the production database and we should avoid it.
Increasing the importance of this bug.
We shouldn't create urls if they're not used, or at least we should delete old urls in case they haven't been copied / accessed.
Severity: normal → critical
Priority: P2 → P1
| Reporter | ||
Comment 4•11 years ago
|
||
(In reply to Alexis Metaireau (:alexis) from comment #3)
> This starts to fill the production database and we should avoid it.
>
> Increasing the importance of this bug.
Do you have stats of how many links are being created across different versions, versus how many calls are being made? (or at least the first).
Average length of time to expiry might be useful as well, i.e. were there a bunch created a couple of weeks ago, that will expire soon?
We should really try to make sure we have appropriate documented stats before we change anything so that we can check before and after.
| Reporter | ||
Updated•11 years ago
|
Flags: needinfo?(alexis+bugs)
Comment 5•11 years ago
|
||
That's correct, we should have these stats, but I don't think that will fix up the problem we're facing.
Currently, each time a panel is opened, a new call-url is generated, and that's not what was planned. As a result, we are sending a lot more data to the server than we should have, and most of this data is useless.
I'm not sure what's the best way to know when a new call-url should be created on the server side, to be honnest.
A few possibilities:
- We could consider the call-urls to be valid after then panel is open for a certain amount of time.
- We could update the pannel to act a bit differently: have a placeholder with a text ("put your mouse over here to generate a link") or a button that transforms to a call-url, or something like that.
Also, note that we re-sized the database so that it can handle more call-urls, so it will take some time before we fill everything up again.
Flags: needinfo?(alexis+bugs)
| Reporter | ||
Comment 6•11 years ago
|
||
(In reply to Alexis Metaireau (:alexis) from comment #5)
> That's correct, we should have these stats, but I don't think that will fix
> up the problem we're facing.
The assumption is that this is the only problem. I'd like to see stats so that we know this isn't the only problem we have.
> Currently, each time a panel is opened, a new call-url is generated, and
> that's not what was planned.
That was a change of design that was planned.
> A few possibilities:
>
> - We could consider the call-urls to be valid after then panel is open for a
> certain amount of time.
> - We could update the pannel to act a bit differently: have a placeholder
> with a text ("put your mouse over here to generate a link") or a button that
> transforms to a call-url, or something like that.
I think comment 0's ideas are already sufficient here. That still may have a background of one-url per user that opens the panel (even if they don't use it), but that's a better static cost.
I'd like to see this done in time for Firefox 34 if we can. Though I think it might be okay if this slips to Firefox 35 since we're doing a soft-start in 34.
| Reporter | ||
Comment 8•11 years ago
|
||
Some of us discussed this in a meeting today, I'll try summarising:
- Firstly, we're concerned that we don't have any numbers of how many urls are being generated per user. This could mean that the fundamental problem is not actually this bug, but more just the fact users are looking at Loop.
- Secondly, the flow suggested in comment 0, doesn't take into account manual copying/reading, which may be a possibility, and we'll certainly have to add in tracking for that possibility to see if it occurs or not.
Additionally, this new flow will need UX and maybe product sign-off.
Comment 9•11 years ago
|
||
Completely true, we need to have information about how many call urls we have per user.
The current script (attached) allows this, I will get ops to run it for us and report back any finding they have.
Comment 10•11 years ago
|
||
Bob, Dean, can you run this script on current production and report any findings here?
Flags: needinfo?(dwilson)
Flags: needinfo?(bobm)
Comment 11•11 years ago
|
||
(In reply to Mark Banner (:standard8) from comment #8)
> - Secondly, the flow suggested in comment 0, doesn't take into account
> manual copying/reading, which may be a possibility...
By "manual copying," you mean without the assistance of the system clipboard?
In both cases, I think you'll find that users aren't going to have a lot of success doing that. We're using URLs that include (and distinguish between) "I" and "l" and "0" and "O", and renders them in a sans-serif font that makes perceiving the difference nearly impossible.
If we're just talking about marking a URL as "used" for the purposes of generating (or not generating) a new one, I think we can ignore these uses as being pretty fringe.
Comment 12•11 years ago
|
||
(In reply to Alexis Metaireau (:alexis) from comment #10)
> Bob, Dean, can you run this script on current production and report any
> findings here?
average is 1.34957020057
Flags: needinfo?(bobm)
Comment 13•11 years ago
|
||
Bob is it still the case?
Comment 14•11 years ago
|
||
It's now 1.42617835441
Comment 15•11 years ago
|
||
Decreasing the importance of this bug, since it's not what is causing the huge number of request to the call-url endpoint. We're working on ways to throttle the clients on the server side as well, so we cannot be ddos-ed easily.
Severity: critical → normal
Flags: needinfo?(dwilson)
Comment 16•11 years ago
|
||
looking at latest comments - looking like this could be closed and that we are looking at similar issues for Rooms.
backlog: --- → -
Flags: needinfo?(standard8)
Target Milestone: mozilla35 → ---
| Reporter | ||
Comment 17•11 years ago
|
||
(In reply to sescalante from comment #16)
> looking at latest comments - looking like this could be closed and that we
> are looking at similar issues for Rooms.
Rooms don't have an issue - they are created manually, rather than every time the window is opened.
I think its reasonable to close this as wontfix for now - afaik we're not planning on fixing this.
Status: NEW → RESOLVED
Closed: 11 years ago
Flags: needinfo?(standard8)
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•