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

Simplify transaction starting constraints to match reality #319

Merged
merged 3 commits into from
Mar 4, 2020

Conversation

inexorabletash
Copy link
Member

@inexorabletash inexorabletash commented Feb 27, 2020

All implementations have a strict ordering of transactions with
overlapping scopes; read-only transactions can run in parallel but
block a later read/write transaction from starting, and the read/write
transactions similarly block later read/write and read-only
transactions.

Tighten up the constraints definition to something precise, and move
the wordy implications into a non-normative aside.

Also, define "overlapping scopes" as a term.

Closes #253

The following tasks have been completed:

  • Confirmed there are no ReSpec/BikeShed errors or warnings.
  • Verify existing WPT coverage, or add it

Preview | Diff

Copy link
Collaborator

@asutherland asutherland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a really fantastic cleanup!

Two thoughts:

  1. The more straightforward constraints seem to increase the importance of "created" and the ordering it creates. At a surface reading, this might suggest there's a global clock across agents and agent clusters and that supersedes any IPC/spec tasks. We only get into "queue a task" in step 2 of the lifetime when starting the transaction. I'm not sure this demands the full task treatment, but maybe some normative text about the creation order being that as perceived by the underlying implementation task queue?

  2. I think it would be helpful for the spec to make it clear that implementations are not required to run non-overlapping read/write transactions in parallel and that indeed, some implementations do not. For example, we landed a fix in web-platform-tests/wpt@c786945#diff-9896d580c9b272443f82b9268b2fc226 where a commit() test case was depending on implementations to do this. It would seem to make sense for this to live around the "These constraints imply the following" section, if not directly in that section. Phrasing gets tricky with a negation, so maybe just an extra info box like "Note that implementations are not required to run non-overlapping read/write transactions in parallel."

@inexorabletash
Copy link
Member Author

Re: 1 - agreed. Web Locks in theory has that level of detail. I'd like to get that here at some point. A later PR, though. :)

Re: 2 - that's easy to add.... and great to document; I'd missed that WPT PR.

@inexorabletash
Copy link
Member Author

I added normative text:

Implementations may impose additional constraints. For example, implementations are not required to run non-overlapping read/write transactions in parallel, or may impose limits on the number of running transactions.

The reason being, up in the lifecycle section it says:

When an implementation is able to enforce the constraints for the transaction’s scope and mode, defined below, the implementation must queue a task to start the transaction asynchronously.

... which seems reasonable as the starting point for a more rigorous algorithm (per #291).

@asutherland
Copy link
Collaborator

Looks great, thanks! Your continued evolution of the spec is greatly appreciated!

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Feb 29, 2020
In service of w3c/IndexedDB#253 move some
transaction scheduling tests from Blink to WPT.

This involved converting them from js-test.js to testharness.js, but
the overall logic of each test was retained.

This also adds one new test which verifies the change described
in w3c/IndexedDB#319 - all browsers implicitly
block R-O transactions behind overlapping R/W transactions.

Change-Id: I596aaa75b79bf3bf3e17a2553abb4e11329d59ab
All implementations have a strict ordering of transactions with
overlapping scopes; read-only transactions can run in parallel but
block a later read/write transaction from starting, and the read/write
transactions similarly block later read/write and read-only
transactions.

Tighten up the constraints definition to something precise, and move
the wordy implications into a non-normative aside.

Also, define "overlapping scopes" as a term.

Closes #253
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Mar 3, 2020
In service of w3c/IndexedDB#253 move some
transaction scheduling tests from Blink to WPT.

This involved converting them from js-test.js to testharness.js, but
the overall logic of each test was retained.

This also adds one new test which verifies the change described
in w3c/IndexedDB#319 - all browsers implicitly
block R-O transactions behind overlapping R/W transactions.

Change-Id: I596aaa75b79bf3bf3e17a2553abb4e11329d59ab
Bug: 921193
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Mar 3, 2020
In service of w3c/IndexedDB#253 move some
transaction scheduling tests from Blink to WPT.

This involved converting them from js-test.js to testharness.js, but
the overall logic of each test was retained.

This also adds one new test which verifies the change described
in w3c/IndexedDB#319 - all browsers implicitly
block R-O transactions behind overlapping R/W transactions.

Change-Id: I596aaa75b79bf3bf3e17a2553abb4e11329d59ab
Bug: 921193
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2081237
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Commit-Queue: Joshua Bell <jsbell@chromium.org>
Auto-Submit: Joshua Bell <jsbell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#746553}
blueboxd pushed a commit to blueboxd/chromium-legacy that referenced this pull request Mar 3, 2020
In service of w3c/IndexedDB#253 move some
transaction scheduling tests from Blink to WPT.

This involved converting them from js-test.js to testharness.js, but
the overall logic of each test was retained.

This also adds one new test which verifies the change described
in w3c/IndexedDB#319 - all browsers implicitly
block R-O transactions behind overlapping R/W transactions.

Change-Id: I596aaa75b79bf3bf3e17a2553abb4e11329d59ab
Bug: 921193
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2081237
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Commit-Queue: Joshua Bell <jsbell@chromium.org>
Auto-Submit: Joshua Bell <jsbell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#746553}
stephenmcgruer pushed a commit to web-platform-tests/wpt that referenced this pull request Mar 4, 2020
In service of w3c/IndexedDB#253 move some
transaction scheduling tests from Blink to WPT.

This involved converting them from js-test.js to testharness.js, but
the overall logic of each test was retained.

This also adds one new test which verifies the change described
in w3c/IndexedDB#319 - all browsers implicitly
block R-O transactions behind overlapping R/W transactions.

Change-Id: I596aaa75b79bf3bf3e17a2553abb4e11329d59ab
Bug: 921193
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2081237
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Commit-Queue: Joshua Bell <jsbell@chromium.org>
Auto-Submit: Joshua Bell <jsbell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#746553}

Co-authored-by: Joshua Bell <inexorabletash@gmail.com>
@inexorabletash
Copy link
Member Author

Tests are in. Merging.

@inexorabletash inexorabletash merged commit ae36fd6 into master Mar 4, 2020
xeonchen pushed a commit to xeonchen/gecko that referenced this pull request Mar 7, 2020
…cheduling tests, a=testonly

Automatic update from web-platform-tests
WPT: Upstream and add some transaction scheduling tests (#22027)

In service of w3c/IndexedDB#253 move some
transaction scheduling tests from Blink to WPT.

This involved converting them from js-test.js to testharness.js, but
the overall logic of each test was retained.

This also adds one new test which verifies the change described
in w3c/IndexedDB#319 - all browsers implicitly
block R-O transactions behind overlapping R/W transactions.

Change-Id: I596aaa75b79bf3bf3e17a2553abb4e11329d59ab
Bug: 921193
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2081237
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Commit-Queue: Joshua Bell <jsbell@chromium.org>
Auto-Submit: Joshua Bell <jsbell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#746553}

Co-authored-by: Joshua Bell <inexorabletash@gmail.com>

--

wpt-commits: 5e1160e543827227c05b10c7660795436a76b307
wpt-pr: 22027
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Mar 7, 2020
…cheduling tests, a=testonly

Automatic update from web-platform-tests
WPT: Upstream and add some transaction scheduling tests (#22027)

In service of w3c/IndexedDB#253 move some
transaction scheduling tests from Blink to WPT.

This involved converting them from js-test.js to testharness.js, but
the overall logic of each test was retained.

This also adds one new test which verifies the change described
in w3c/IndexedDB#319 - all browsers implicitly
block R-O transactions behind overlapping R/W transactions.

Change-Id: I596aaa75b79bf3bf3e17a2553abb4e11329d59ab
Bug: 921193
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2081237
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Commit-Queue: Joshua Bell <jsbell@chromium.org>
Auto-Submit: Joshua Bell <jsbell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#746553}

Co-authored-by: Joshua Bell <inexorabletash@gmail.com>

--

wpt-commits: 5e1160e543827227c05b10c7660795436a76b307
wpt-pr: 22027
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Mar 9, 2020
…cheduling tests, a=testonly

Automatic update from web-platform-tests
WPT: Upstream and add some transaction scheduling tests (#22027)

In service of w3c/IndexedDB#253 move some
transaction scheduling tests from Blink to WPT.

This involved converting them from js-test.js to testharness.js, but
the overall logic of each test was retained.

This also adds one new test which verifies the change described
in w3c/IndexedDB#319 - all browsers implicitly
block R-O transactions behind overlapping R/W transactions.

Change-Id: I596aaa75b79bf3bf3e17a2553abb4e11329d59ab
Bug: 921193
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2081237
Reviewed-by: Daniel Murphy <dmurphchromium.org>
Commit-Queue: Joshua Bell <jsbellchromium.org>
Auto-Submit: Joshua Bell <jsbellchromium.org>
Cr-Commit-Position: refs/heads/master{#746553}

Co-authored-by: Joshua Bell <inexorabletashgmail.com>

--

wpt-commits: 5e1160e543827227c05b10c7660795436a76b307
wpt-pr: 22027

UltraBlame original commit: 6b1929e476680a2aafc78037914b756199333f09
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Mar 9, 2020
…cheduling tests, a=testonly

Automatic update from web-platform-tests
WPT: Upstream and add some transaction scheduling tests (#22027)

In service of w3c/IndexedDB#253 move some
transaction scheduling tests from Blink to WPT.

This involved converting them from js-test.js to testharness.js, but
the overall logic of each test was retained.

This also adds one new test which verifies the change described
in w3c/IndexedDB#319 - all browsers implicitly
block R-O transactions behind overlapping R/W transactions.

Change-Id: I596aaa75b79bf3bf3e17a2553abb4e11329d59ab
Bug: 921193
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2081237
Reviewed-by: Daniel Murphy <dmurphchromium.org>
Commit-Queue: Joshua Bell <jsbellchromium.org>
Auto-Submit: Joshua Bell <jsbellchromium.org>
Cr-Commit-Position: refs/heads/master{#746553}

Co-authored-by: Joshua Bell <inexorabletashgmail.com>

--

wpt-commits: 5e1160e543827227c05b10c7660795436a76b307
wpt-pr: 22027

UltraBlame original commit: 6b1929e476680a2aafc78037914b756199333f09
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Mar 10, 2020
…cheduling tests, a=testonly

Automatic update from web-platform-tests
WPT: Upstream and add some transaction scheduling tests (#22027)

In service of w3c/IndexedDB#253 move some
transaction scheduling tests from Blink to WPT.

This involved converting them from js-test.js to testharness.js, but
the overall logic of each test was retained.

This also adds one new test which verifies the change described
in w3c/IndexedDB#319 - all browsers implicitly
block R-O transactions behind overlapping R/W transactions.

Change-Id: I596aaa75b79bf3bf3e17a2553abb4e11329d59ab
Bug: 921193
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2081237
Reviewed-by: Daniel Murphy <dmurphchromium.org>
Commit-Queue: Joshua Bell <jsbellchromium.org>
Auto-Submit: Joshua Bell <jsbellchromium.org>
Cr-Commit-Position: refs/heads/master{#746553}

Co-authored-by: Joshua Bell <inexorabletashgmail.com>

--

wpt-commits: 5e1160e543827227c05b10c7660795436a76b307
wpt-pr: 22027

UltraBlame original commit: 6b1929e476680a2aafc78037914b756199333f09
@inexorabletash inexorabletash deleted the txorder branch March 27, 2020 18:53
inexorabletash added a commit that referenced this pull request Feb 1, 2021
All implementations have a strict ordering of transactions with
overlapping scopes; read-only transactions can run in parallel but
block a later read/write transaction from starting, and the read/write
transactions similarly block later read/write and read-only
transactions.

Tighten up the constraints definition to something precise, and move
the wordy implications into a non-normative aside.

Also, define "overlapping scopes" as a term.

Closes #253
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 this pull request may close these issues.

Disallow starting readwrite transactions while readonly transactions are running?
2 participants