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

Use "queue a global task" and supply the correct arguments #101

Open
annevk opened this issue Jun 19, 2023 · 5 comments
Open

Use "queue a global task" and supply the correct arguments #101

annevk opened this issue Jun 19, 2023 · 5 comments

Comments

@annevk
Copy link

annevk commented Jun 19, 2023

Or maybe use "queue a storage task" as defined in the Storage Standard.

Just "queue a task" without task source or global is not correct a lot of the time.

ayuishii added a commit that referenced this issue Sep 26, 2023
ayuishii added a commit that referenced this issue Sep 26, 2023
@ayuishii
Copy link
Collaborator

Thanks for reviewing the spec! Closing with #103

@ayuishii
Copy link
Collaborator

ayuishii commented Oct 2, 2023

Re-opening this issue for further advice on queuing.

As we’re polishing the spec, we realized that using “queuing a storage task” might leave us needing to coordinate with other storage APIs that use the same task source. Since “Queue” implies FIFO, we don’t think guaranteeing all async requests in the same order they’re made is right in all scenarios (and certainly not across different storage APIs).

With this,

  1. Would you still recommend using the storage task source? Alternatively we see examples that specify their own task source like ServiceWorker. Or a more generic one like the DOM manipulation task source.

  2. Just "queue a task" without task source or global is not correct a lot of the time.

Curious to learn in which cases it might be correct to just "queue a task".

Thanks!

@ayuishii ayuishii reopened this Oct 2, 2023
@asutherland
Copy link
Collaborator

I think as long as "queue a storage task" is invoked from a general hand-waving "in parallel" that itself doesn't have an associated queue that enforces ordering constraints, then everything may be fine[1].

The problem right now is that, for example, both the algorithm for keys() and persisted() never go "in parallel", so these implicitly must be strictly ordered.

The algorithm for open() has a somewhat more dramatic problem as, since the algorithm never goes parallel and it synchronously returns an already-resolved promise, it wouldn't surprise me if it's a requirement that a handler immediately chained on the promise before the task ends would be required to have the storage bucket already available when the microtask queue is processed and before running any additional tasks.

1: That said, the spec text for storage task source is:

The storage task source is a task source to be used for all tasks related to a storage endpoint. In particular those that relate to a storage endpoint’s quota.

And storage endpoint is a little fuzzy but seems to explicitly correspond to a given bottle:

A storage endpoint is a local or session storage API that uses the infrastructure defined by this standard, most notably storage bottles, to keep track of its storage needs.

So it's not clear the storage task source works as defined since the storage shelf is really where the Storage Buckets API sits.

It could make sense to define a specific task source for shelves if the storage spec also wants to define something like ServiceWorker's job queue mechanism to provide a strict ordering of operations related to buckets. This could be helpful for things like testing bucket limit enforcement since that lack of a queue would mean that one could never have more than one outstanding open() request at a time because there are no ordering constraints. Having an explicit queue that then references a specific task queue could provide expectations about the sequencing which could be a reasonable implementation constraint.

That said, we'd want to be judicious in terms of what would want to use the queue. For example, I don't think we would ever want quota tracking to use the job queue, as I think it would be important to leave browsers a lot of implementation flexibility there. Things that might use the queue might also invoke "in parallel" in a way that relaxes ordering constraints again.

@ayuishii
Copy link
Collaborator

ayuishii commented Oct 3, 2023

Thanks for your comment @asutherland!

I think as long as "queue a storage task" is invoked from a general hand-waving "in parallel" that itself doesn't have an associated queue that enforces ordering constraints, then everything may be fine[1].

Oh interesting, I guess this relaxes the ordering concerns a bit.

The problem right now is that, for example, both the algorithm for keys() and persisted() never go "in parallel", so these implicitly must be strictly ordered.

Thanks for pointing this out 😅, I'll go over them and try to fix these.

And storage endpoint is a little fuzzy but seems to explicitly correspond to a given bottle:

Good point...seems the storage task source may not be the right fit here. Adding a task source for the storage shelf on the storage spec sounds like it could be a good option (thoughts @annevk ?).

In the interim, does it sound reasonable to create our own task source for storage buckets for the spec draft?
Alternatively I can call it the storage shelf task source as well.

@asutherland
Copy link
Collaborator

Thanks for your comment @asutherland!
In the interim, does it sound reasonable to create our own task source for storage buckets for the spec draft? Alternatively I can call it the storage shelf task source as well.

I think there's an advantage to creating our own specific task source than using something more generic like the DOM manipulation task source because we avoid accidentally specifying an ordering that we don't intend or making people think there's some ordering intended.

I think for naming, the issues for confusion we need to deal with are:

  • Is this associated with a specific storage shed/shelf/bucket/bottle granularity?
  • Avoiding confusion with the pieces that might correspond to a job queue operating at the other end of IPC in another process.

So for names:

  • storage shelf task source gets the granularity right but risks confusion that someone might be picturing the shed and shelves living in the "parent" (Gecko), "browser"? (Blink) process.
  • StorageBucketManager task source may be bad form because StorageBucketManager is too much of an implementation detail, but has the advantage of clarity in my mind. Hopefully @annevk has a better suggestion :)

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

No branches or pull requests

3 participants