Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(276)

Issue 2691423005: [IndexedDB] Transaction limitting (Closed)

Created:
3 years, 10 months ago by dmurph
Modified:
3 years, 10 months ago
Reviewers:
cmumford, pwnall
CC:
chromium-reviews, jam, jsbell+idb_chromium.org, darin-cc_chromium.org, cmumford
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[IndexedDB] Transaction limiting, minimal patch. Limits the # of active transactions per-database. Prevents transaction trashing. R=cmumford,pwnall BUG=533648 Review-Url: https://codereview.chromium.org/2691423005 Cr-Commit-Position: refs/heads/master@{#451174} Committed: https://chromium.googlesource.com/chromium/src/+/635bab41ab657fb272b5e1539b194e12e0bffa6f

Patch Set 1 #

Total comments: 5

Patch Set 2 : removed code #

Patch Set 3 : more explanation #

Total comments: 5

Patch Set 4 : comment change #

Unified diffs Side-by-side diffs Delta from patch set Stats (+12 lines, -0 lines) Patch
M content/browser/indexed_db/indexed_db_transaction_coordinator.cc View 1 2 3 2 chunks +12 lines, -0 lines 0 comments Download

Messages

Total messages: 27 (15 generated)
dmurph
3 years, 10 months ago (2017-02-16 22:58:29 UTC) #2
dmurph
PTAL, this is the transaction throttling patch.
3 years, 10 months ago (2017-02-16 23:08:03 UTC) #5
pwnall
Did you leave the extra tracing code in by accident? This doesn't seem minimal to ...
3 years, 10 months ago (2017-02-16 23:15:30 UTC) #6
cmumford
https://codereview.chromium.org/2691423005/diff/1/content/browser/indexed_db/indexed_db_transaction_coordinator.cc File content/browser/indexed_db/indexed_db_transaction_coordinator.cc (right): https://codereview.chromium.org/2691423005/diff/1/content/browser/indexed_db/indexed_db_transaction_coordinator.cc#newcode14 content/browser/indexed_db/indexed_db_transaction_coordinator.cc:14: static const size_t kMaxStartedTransactions = 10; Can you add ...
3 years, 10 months ago (2017-02-16 23:16:05 UTC) #7
pwnall
https://codereview.chromium.org/2691423005/diff/1/content/browser/indexed_db/indexed_db_transaction_coordinator.cc File content/browser/indexed_db/indexed_db_transaction_coordinator.cc (right): https://codereview.chromium.org/2691423005/diff/1/content/browser/indexed_db/indexed_db_transaction_coordinator.cc#newcode14 content/browser/indexed_db/indexed_db_transaction_coordinator.cc:14: static const size_t kMaxStartedTransactions = 10; Can you explain ...
3 years, 10 months ago (2017-02-16 23:19:40 UTC) #10
dmurph
https://codereview.chromium.org/2691423005/diff/1/content/browser/indexed_db/indexed_db_transaction_coordinator.cc File content/browser/indexed_db/indexed_db_transaction_coordinator.cc (right): https://codereview.chromium.org/2691423005/diff/1/content/browser/indexed_db/indexed_db_transaction_coordinator.cc#newcode14 content/browser/indexed_db/indexed_db_transaction_coordinator.cc:14: static const size_t kMaxStartedTransactions = 10; On 2017/02/16 23:19:40, ...
3 years, 10 months ago (2017-02-16 23:32:35 UTC) #12
cmumford
lgtm % comment nit. https://codereview.chromium.org/2691423005/diff/40001/content/browser/indexed_db/indexed_db_transaction_coordinator.cc File content/browser/indexed_db/indexed_db_transaction_coordinator.cc (right): https://codereview.chromium.org/2691423005/diff/40001/content/browser/indexed_db/indexed_db_transaction_coordinator.cc#newcode15 content/browser/indexed_db/indexed_db_transaction_coordinator.cc:15: // Limited prevent transaction trashing. ...
3 years, 10 months ago (2017-02-16 23:36:14 UTC) #14
pwnall
https://codereview.chromium.org/2691423005/diff/40001/content/browser/indexed_db/indexed_db_transaction_coordinator.cc File content/browser/indexed_db/indexed_db_transaction_coordinator.cc (right): https://codereview.chromium.org/2691423005/diff/40001/content/browser/indexed_db/indexed_db_transaction_coordinator.cc#newcode15 content/browser/indexed_db/indexed_db_transaction_coordinator.cc:15: // Limited prevent transaction trashing. Ten is chosen to ...
3 years, 10 months ago (2017-02-16 23:38:28 UTC) #15
dmurph
https://codereview.chromium.org/2691423005/diff/40001/content/browser/indexed_db/indexed_db_transaction_coordinator.cc File content/browser/indexed_db/indexed_db_transaction_coordinator.cc (right): https://codereview.chromium.org/2691423005/diff/40001/content/browser/indexed_db/indexed_db_transaction_coordinator.cc#newcode15 content/browser/indexed_db/indexed_db_transaction_coordinator.cc:15: // Limited prevent transaction trashing. Ten is chosen to ...
3 years, 10 months ago (2017-02-16 23:50:27 UTC) #18
pwnall
LGTM, but I think you should create a dependent CL that tests this rate-limiting. The ...
3 years, 10 months ago (2017-02-17 00:11:56 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2691423005/60001
3 years, 10 months ago (2017-02-17 00:16:36 UTC) #24
commit-bot: I haz the power
3 years, 10 months ago (2017-02-17 02:07:09 UTC) #27
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/635bab41ab657fb272b5e1539b19...

Powered by Google App Engine
This is Rietveld 408576698