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

Add queue for server actions #224

Closed
eloquence opened this issue Jan 11, 2019 · 4 comments · Fixed by #374
Closed

Add queue for server actions #224

eloquence opened this issue Jan 11, 2019 · 4 comments · Fixed by #374

Comments

@eloquence
Copy link
Member

eloquence commented Jan 11, 2019

It may be useful to have a persistent queue of certain pending actions on the server, e.g., replies, starring:

  • it can be enforced that no duplicate actions are added to the queue (starring the same source twice);
  • we can ensure that order-sensitive operations are performed on the server in the sequence in which the user performed them (e.g., replies are not sent out of order);
  • retries can be performed even after a crash, and the state of the number of retries for a given action is known;
  • when an operation ultimately fails after repeated retries, that failure can be reported to the user, e.g. to solve a connection problem and then retry the operation;
  • when an action ultimately fails, but the state change is already implemented on the client, we can reset the client to the original state (e.g., a pending reply can be cancelled, a source unstarred).

Such a mechanism may potentially be used to queue up replies offline as well, though that is admittedly of less obvious benefit.

@heartsucker
Copy link
Contributor

If we're going to have logic that prevents duplicates (double starring), then we also need to be able to detect negations of those operations. E.g., I click start, then unstar, then star again, the queue needs to end with the last operation being the star operation.

Also imagine this case. The client can't make a circuit to the journalist interface, and the journalist using the client makes many replies to many source. In the meantime, another journalist deletes one of the sources. If the client keeps retrying failures (404 in this case), that deleted source will block all other actions.

@eloquence
Copy link
Member Author

eloquence commented Jan 18, 2019

If the client keeps retrying failures (404 in this case), that deleted source will block all other actions.

The current behavior appears to be that we're already removing sources on the client upon sync if they no longer exist on the server.

Consistent with that behavior, the correct way to resolve a 404 in the expected API response format seems to me to either remove the source (and associated queue items) on the client immediately, or to do so with some user warning upon the next sync. Leaving aside how to manage the issue of user surprise, are there other technical reasons not to do that?

This doc contains some notes on how different failure modes could be surfaced to the user.

@eloquence
Copy link
Member Author

For the record, the architecture is being discussed in this document drafted by @heartsucker:
https://docs.google.com/document/d/1oiJdP5uvDEBrR6KyxTfR04NG0M0gW-GPoeKF9hJD4nA/edit

There are still some open questions (comments in the doc); once these are resolved, we can create a more formally scoped issue for the next sprint.

@eloquence
Copy link
Member Author

For the 4/17-5/1 sprint, at minimum, we've committed to a WIP skeleton implementation that addresses the core functionality described in the proposal at least for one well-defined use case (e.g., sending replies) and that may ignore some of the considerations in the document, e.g., conflict resolution prior to queue items being consumed.

During the WIP stage, we don't have to immediately add full test coverage, since the goal is to get clarity on the implementation (we'll continue to check in via the weekly client sync meetings). That said, landing a testable minimally viable implementation should be considered a stretch goal for this sprint.

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

Successfully merging a pull request may close this issue.

2 participants