Implement queue persistence

[eloquence]

The queue that landed in #374 is still an in-memory implementation. That means that if there are pending actions (e.g., replies) and the client is closed or crashes, those pending actions will be lost.

Given that the planned pilot usage represents a real-world production deployment, avoiding accidental data loss is very important. Moreover, this in-memory user forces the user to work according to the client's schedule (wait for all tasks to be completed before logging out), which may often be impractical. We should therefore aim, even for the beta, to make the queue persistent across client sessions.

Acceptance Criteria

Given that I am running the client, and have started some operations that have not completed yet
When I restart the client
Then those operations should continue.

User Story

As an journalist, I want to be able to pause my work without data loss, so that my work schedule is not determined by the software I use to do my job.

[eloquence]

One open question for the first implementation is whether this means that the queue must be a per-user queue.

There are strong arguments in favor of a per-user queue:

• Prioritizing another user's jobs could block a user from completing their work.
• Visible client activity not initiated by the logged in user could be confusing and anxiety-provoking.
• Sending replies by another user that were deliberately delayed for later could be harmful.

Moving to per-user queues does limit the FIFO property of the queue to a given user: my reply is not guaranteed to be the next one sent. It means we may end up with duplicate actions in different user queues (e.g., two downloads of the same file), and may have to do additional work to normalize across per-user queues before or after running jobs, to limit UI state indicators to a given user, etc.

Perhaps it is worth considering keeping the first iteration of persistence global. This would at least address the data loss concern, and may be a manageable usability compromise for the beta.

[sssoleileraaa]

Thinking about this more, I think it's more important to first focus on saving draft replies than persisting the queue. The application could crash or the user might need to force quit before the creation of a queue job. Also if the job is created and the application quits while it's sitting on the queue, the draft reply will still be there.

And maybe it doesn't make sense to persist the queue.

There are some background queue jobs that happen regardless of user interaction, e.g. downloading messages, downloading replies, and downloading metadata. I don't think we should worry about persisting these types of jobs because they will already pick back up each client session at regular intervals.

As far as jobs that require user interaction (other than sending a reply), e.g. staring, source deletion, a name change, file download, etc., a journalist might have quit the application early becuase they don't care if the jobs finish. For instance, they might have clicked to download a file by mistake. Automatically redownloading this file the next time the user or any user logs in (could be a month later or so, who knows?) doesn't make sense in that case. Also the file would need to be redownloaded from scratch anyway, so is the queue complexity worth saving a user from having to click "Download" again?

Trying the think of cases... In the "Download All" case where many file download jobs are created you are again not saving much by persisting the queue. The download jobs that finish will show up as finshed, and the ones that don't will need to be redownloaded.

[eloquence]

I think it's more important to first focus on saving draft replies than persisting the queue.

I agree that reply data loss is the biggest concern during the beta. We have to anticipate 1..n replies to 1..n sources not having been sent yet when the client is closed or crashes. Whatever method we use to persist this data needs to be able to maintain:

• the relationship between replies and sources
• the ordering of replies

A couple of other notes:

• Users may reboot the workstation as part of diagnosing a network problem. Imagine a situation where you're lining 25 downloads up individually, but none of them start. It seems to be some Qubes issue, so you reboot. After reboot all those jobs are forgotten. That could be very frustrating.

• Deletion is a sensitive job with security implications. Since the UI currently blocks until it's completed and we have no immediate plans to change that, this should hopefully not be impacted by what we decide to do here.

• Other actions being forgotten (starring, flagging, codename cycling if we support it) could be frustrating and confusing, but not necessarily a dealbreaker for the beta.

A common way to solve the problems you describe would be to give each job an expiry property:

• expires after session
• expires after a certain amount of time
• never expires.

That would allow us to reason through the utility of persistence for different jobs. It does of course add to the complexity, along with the per-user requirement -- beyond what may be possible for beta. I do, however, think we should have an idea of the desired end state, before scoping the first iteration. If we ultimately will want those queue properties, that may influence the architecture of iteration 1.

[sssoleileraaa]

Whatever method we use to persist this data needs to be able to maintain:

• the relationship between replies and sources
• the ordering of replies

If a reply job gets queued but never processed on application exit, then should we give the user a chance to make sure they still want to send the reply next time they log in? Or should we automatically resend it and possibly others in the same order that they were originally queued? I know we've gone back and forth on this so I can't remember where this landed. It would be easier to start off by showing the user that the replies failed to send the next time they log in. This would also give them a chance to resend or discard failed replies individually.

• Users may reboot the workstation as part of diagnosing a network problem. Imagine a situation where you're lining 25 downloads up individually, but none of them start. It seems to be some Qubes issue, so you reboot. After reboot all those jobs are forgotten. That could be very frustrating.

Gotcha. So either way we have to persist which files are "in briefcase" (true or false) and what their status is (pending, sent, or failed). If the client starts up and sees a file that has a status of pending, we know the job never left the queue on exit. If a job succeeds "in briefcase" can be set to false.

• Deletion is a sensitive job with security implications. Since the UI currently blocks until it's completed and we have no immediate plans to change that, this should hopefully not be impacted by what we decide to do here.

This is an interesting case. I'm glad you mentioned this one. Once we move deletion to a queue job, this removes the blocking behavior, so we'll need to add it back somehow. Also a blocking job should be bumped to the front of the queue.

A common way to solve the problems you describe would be to give each job an expiry property:

• expires after session
• expires after a certain amount of time
• never expires.

That would allow us to reason through the utility of persistence for different jobs. It does of course add to the complexity, along with the per-user requirement -- beyond what may be possible for beta. I do, however, think we should have an idea of the desired end state, before scoping the first iteration. If we ultimately will want those queue properties, that may influence the architecture of iteration 1.

In this implementation, would a "never expires" job be persisted on queue exit or at the beginning of enqueue? When do you see the reply content being persisted? I am proposing that we persist it immediately. If we want to take a step towards queue persistence, we could add the persistence logic to the RunnableQueue instead of the Controller::send_reply method. We would need to add a put method or something that could be used to indicate that a new job is being added that may need to be persisted.

[sssoleileraaa]

What do you think of this order of next steps?

1. Support the most important thing (replies) in the simplest way (store them in a "draft" state, or actually a "pending" state).

   This would require adding a status column to the replies table. When a user clicks on the Send button, the client would create a db record of a reply with its content and pending status. At this point it is persisted. Then simultaneously the client can update the UI and enqueue a SendReplyJob. If the client starts up and sees pending requests (this would only happen if the app crashed or user quit or went offline while SendReplyJob was still in the queue) then it updates them to failed.

   Or if we want to take a step towards queue persistence, we could do the above with the following modification: When a user clicks on the Send button, the client first enqueues a SendReplyJob which would persist the content and status of the reply from the queue thread and then emit a signal or somehow tell the Controller to tell the UI to refresh its reply widget to show the persisted pending content.

   Either way, the content needs to be persisted right away.

2. Support keeping state of a "briefcase" using the same logic.

   Need to add an in_briefcase and status column to the files table. When the user adds a file to the briefcase, set in_briefcase to True. When the user is ready to download, enqueue a bunch of FileDownloadJobs and set the status to pending. If the client starts up and sees any pending downloads it updates them to failed. The failed files will still have in_briefcase set.

3. Next add per-user support in the client.

4. Next add a generic solution of persisting the queue. Will have to thinking about

[redshiftzero]

Support the most important thing (replies) in the simplest way (store them in a "draft" state, or actually a "pending" state).

👍 to storing per-user draft replies in the database and updating the conversation view accordingly

to the question of which of these jobs would we actually want to definitely resume and which would we want to ask the user first (or implicitly by e.g. making the reply show as failed and they need to manually click "retry").

I think only the idempotent actions we'd want to resume:

• File downloads - provided that we add a check to the beginning of the file download job to check the file has not been downloaded by someone else in the interim
• Star, unstar, flag actions - : if Journo A queues the action, it fails, and then Journo B does it the next day, Journo A doing it again causes no issue.

Actions we probably wouldn't want to resume:

• Regarding deletion, the behavior is that the job is sent and the UI doesn't update to display the deleted source until it is confirmed deleted on the server, but the UI isn't blocked, i.e. there's no dialog that comes up "Waiting to delete source X...". If journalist A tries to delete a source and it doesn't complete, I don't think we'd want to resume. We might just want to display somewhere (in the eventual activity log) "Deletion of source X failed".
• Regarding reply sending, I think we'd want to definitely have the users retry these manually instead of firing off whatever was pending, e.g. what if Journo A sent a message that failed, Journo B comes along and responds in Journo A's stead, we don't want Journo A's message to succeed the next time they log on.

Note that the global persistent queue idea is going to be problematic specifically for the replies: reply jobs are associated with a given user (whichever user the API token corresponds to) and we don't want to poison that data (which they will if User B's token ends up sending reply X from User A after User A logged out - we can't reuse User A's token because it's either expired or invalidated). It also complicates auditing: User B will be doing things that User A started on the server, making the journalist interface logs much less useful. I think we should avoid a global persistent queue for these reasons.

[sssoleileraaa]

"in briefcase"

Realizing that we also use this word for the export feature. Sorry for the confusion. I was picturing a way for a user to add files to a collection before they decide to download, but this probably isn't the way users download things.

I agree we wouldn't want to auto-resume sending replies or deletion operations. As far as auto-resuming dowload jobs that a user made from a previous session, I don't think it makes sense but do see value in it when there is no activity log. An activity log would help users find failed replies too.

Note that the global persistent queue idea is going to be problematic specifically for the replies: reply jobs are associated with a given user

Good point, so actually adding per-user pending replies in the client is probably the first step we need to take. I think this means doing step 1 from my list above. Then add per-user pending file downloads. If we decide that it is absolutely necessary to automatically resend these jobs the next time the user logs in, then I understand why we would want to add "queue persistence." Otherwise i think we should just show the file as failed and allow the user to resend.

[eloquence]

I think we should avoid a global persistent queue for these reasons.

Should we therefore close this issue, and defer further discussion of queue persistence until we have a truly compelling use case for it? It seems like the main goals that motivated this issue can be sufficiently satisfied with reply persistence (which we now have) and an activity log (which is a nice post-beta enhancement).

[redshiftzero]

yep that makes sense since replies were the main pain point and they persist in a draft state if sending fails, so closing (if anyone wants to discuss more just comment and we can reopen)