Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reissued tickets cause extra uploads #1340

Open
artwyman opened this issue Dec 7, 2023 · 0 comments
Open

Reissued tickets cause extra uploads #1340

artwyman opened this issue Dec 7, 2023 · 0 comments
Assignees
Labels

Comments

@artwyman
Copy link
Member

artwyman commented Dec 7, 2023

Splitting this out of #1271 because it turned out to be less trivial than expected.

Behavior: Without any changes, feed reissuance happens every 60s without triggering any upload. However if you claim a frog (or add another PCD, it will triggers an immediate upload to E2EE storage (correct). Sometime within the next 60s, feeds are polled to reissue tickets, which triggers another upload even if tickets are unchanged. Every upload is currently a potential conflict (though the specific detailed sequence of events can vary). This extra upload can be delayed if the tab is closed or becomes invisible, in which case it happens when the tab is reopened.

Explanation: When tickets feeds get reissued (every 60s) they run actions to delete all PCDs in a folder, then replace the ticket PCDs in a folder. Even if the tickets are identical, this has the effect of moving the ticket PCDs to the end of the insertion-order array in PCDCollection. If there are no intervening changes, this is a nop since the tickets are removed and reinserted in the same order. However if there's another new PCD (e.g. a Frog) now feed issuance triggers a change to the content hash, which triggers an upload.

Complication: My initial thought was that order doesn't matter, so I could fix this by sorting PCDs in a fixed order so that reissuance doesn't change the hash. This was naive. Order is actually a user-visible feature in the following ways:

  1. It's the default display order of PCDs in folders. Users might mostly not care, but the identity PCD being first on the homescreen is pretty obvious, and frogs appearing in the order they were claimed is pretty important.
  2. It's presented as one of the sort order options (the clock icon). It's not quite sorting by a date (there is no date metadata in PCDCollection), but it's close enough to work for users.
  3. When fetching the user's identity, we depend on order in 2 places. The home screen expects the user's identity to be the first PCD. The login code expects the user's identity to be the first PCD of type SemaphoreIdentityPCD.

Note that 3 is incidental and we can/should probably be fixed, but 1 and 2 are user visible features we shouldn't change lightly.

Options: Ways I can see to address this:

  1. Do nothing. Since this isn't simple, we can simply live with it. Other fixes in fix sync #1271 should make extra uploads no longer a data loss risk (just inefficient). When we turn off ticket reissuance, the issue will also go away until we turn it back on for something.
  2. Loosen the requirements of PCD order to only be preserved within the same folder (not globally). Then fix the spurious changes by doing a stable sort of PCDs by folder path. This would be simple, but slightly fragile. We'd need to tweak the identity-fetching code to look only in the root folder.
  3. Change the behavior of feed actions to detect these unnecessary changes and skip the action when the PCDs are identical. This would be cleanest if we created explicit "ReplaceFolderContents" action which could contain this logic, but that has the downside of changing the server and protocol. Without that the client could have some special logic across the 2 Delete and Replace actions in the same folder to detect that the only things deleted are the same things being replaced.

Having dumped all this context, I'm going to seek some discussions before deciding how to proceed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant