New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 729790 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Feature


Participants' hotlists:
OWP-Storage-DevTools


Sign in to add a comment

[DevTools] [IndexedDB] Allow DevTools to trigger a (force?) compaction

Project Member Reported by dmu...@chromium.org, Jun 5 2017

Issue description

Create the ability for devtools to compact an IndexedDB's leveldb database.

This would be on a per-origin basis.

To force the compaction, we might need to abort all pending transactions to release all snapshots.

Code that triggers a compaction:
https://cs.chromium.org/chromium/src/content/browser/indexed_db/indexed_db_backing_store.cc?type=cs&l=1937
This should be a synchronous operations - not sure how this will play with snapshots and such.

Probably would pair with calling AbortAllTransactions first for every connection to the origin:
https://cs.chromium.org/chromium/src/content/browser/indexed_db/indexed_db_connection.cc?type=cs&l=126

Probably a 2-sided patch, one to wire the ability through CPP to the renderer, creating a new mojo method here:
https://cs.chromium.org/chromium/src/content/common/indexed_db/indexed_db.mojom?q=indexed_db+mojom&l=1
That also has a callback back for when it's done.
 
Cc: cmumford@chromium.org
Labels: -OS-Mac OS-All
+Chris for any further thoughts
Labels: -Type-Bug -Pri-3 Pri-2 Type-Feature
One issue is to know when the compaction is finished. I'm unaware of any API in leveldb that supports this. Sanjay (leveldb owner) has a high bar when adding new leveldb API, so I'm not sure if he'd be in favor of making this change. If not then it might be something (rather hokey IMHO) like listening for the log message emitted in DBImpl::InstallCompactionResults - hopefully we can come up with a better solution.

I believe we can trigger a compaction at any time, but since transactions create shapshots, which in turn hold on to tables, you'd need to ensure no transactions are running in order to get an accurate measurement of disk space.
Yes we'd probably have to abort all transactions. I wonder if we can do anything w/ the custom environment. I'll dig a little.

Comment 5 by dmu...@chromium.org, Jun 21 2017

Owner: kristip...@chromium.org
Status: Assigned (was: Available)
It seems like we force a synchronous compaction when we delete an object store or database. So I think we can do this, I've seen the traces where the compaction happens on the IDB thread.

Here's the idea:
Create a new method in the Factory interface in here:
https://cs.chromium.org/chromium/src/content/common/indexed_db/indexed_db.mojom?l=326 named CompactDatabase or something.

No arguments, and have it return a new enum called Status, with, say: Ok, Error

This will hook into a method here:
https://cs.chromium.org/chromium/src/content/browser/indexed_db/indexed_db_factory.h?q=indexed_db_fact&l=1

Which will be implemented here:
https://cs.chromium.org/chromium/src/content/browser/indexed_db/indexed_db_factory_impl.h
and cc file here:
https://cs.chromium.org/chromium/src/content/browser/indexed_db/indexed_db_dispatcher_host.cc?l=38

Notice the SequenceHelper there - that is to help us thread hop to the IDB thread instead of the IO thread (Which is what the IPC is sent on initially). See GetDatabaseNames example here:
https://cs.chromium.org/chromium/src/content/browser/indexed_db/indexed_db_dispatcher_host.cc?l=165

So there will be a new method on the SequenceHelper, CompactDatabaseOnIDBThread (or just CompactDAtabase).

This can now talk to to IndexedDBFactoryImpl just like database names here:
https://cs.chromium.org/chromium/src/content/browser/indexed_db/indexed_db_factory_impl.cc?l=172

When you grab the IndexedDBBackingStore, make a method on that for compacting, which can then call CompactAll():
https://cs.chromium.org/chromium/src/content/browser/indexed_db/leveldb/leveldb_database.h?l=108
which lives on the db_ variable in IndexedDBBackingStore.

You'll probably need to pass the mojo callback to the sequencehelper, which then will call it back after it calls into the factory impl.

Hopefully that makes sense. It's a lot of piping.


On the Renderer side, you'll need to pipe a call into the https://cs.chromium.org/chromium/src/content/child/indexed_db/webidbfactory_impl.h?dr=C, and that can be in a follow up patch.

Comment 6 by dmu...@chromium.org, Jun 21 2017

And to make sure it works (or at least doesn't crash) you can use the IndexedDBDispatcherHostTest framework to do a test of that call w/ a real database and such.
Project Member

Comment 7 by bugdroid1@chromium.org, Jul 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7603dc313cc97e1a1674d63c72dcd9b8c4fb755a

commit 7603dc313cc97e1a1674d63c72dcd9b8c4fb755a
Author: kristipark <kristipark@chromium.org>
Date: Thu Jul 06 22:28:22 2017

[IndexedDB] Trigger manual compaction

Bug:  729790 
Change-Id: I41e050311da9a905414902265c5a620036dc63f1
Reviewed-on: https://chromium-review.googlesource.com/551183
Commit-Queue: Kristi Park <kristipark@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#484755}
[modify] https://crrev.com/7603dc313cc97e1a1674d63c72dcd9b8c4fb755a/content/browser/indexed_db/indexed_db_database.cc
[modify] https://crrev.com/7603dc313cc97e1a1674d63c72dcd9b8c4fb755a/content/browser/indexed_db/indexed_db_database.h
[modify] https://crrev.com/7603dc313cc97e1a1674d63c72dcd9b8c4fb755a/content/browser/indexed_db/indexed_db_dispatcher_host.cc
[modify] https://crrev.com/7603dc313cc97e1a1674d63c72dcd9b8c4fb755a/content/browser/indexed_db/indexed_db_dispatcher_host.h
[modify] https://crrev.com/7603dc313cc97e1a1674d63c72dcd9b8c4fb755a/content/browser/indexed_db/indexed_db_dispatcher_host_unittest.cc
[modify] https://crrev.com/7603dc313cc97e1a1674d63c72dcd9b8c4fb755a/content/browser/indexed_db/indexed_db_factory.h
[modify] https://crrev.com/7603dc313cc97e1a1674d63c72dcd9b8c4fb755a/content/browser/indexed_db/indexed_db_factory_impl.cc
[modify] https://crrev.com/7603dc313cc97e1a1674d63c72dcd9b8c4fb755a/content/browser/indexed_db/indexed_db_factory_impl.h
[modify] https://crrev.com/7603dc313cc97e1a1674d63c72dcd9b8c4fb755a/content/browser/indexed_db/mock_indexed_db_factory.h
[modify] https://crrev.com/7603dc313cc97e1a1674d63c72dcd9b8c4fb755a/content/common/indexed_db/indexed_db.mojom

Project Member

Comment 8 by bugdroid1@chromium.org, Jul 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ffa79076cf808c8295f0e499d56d920049b79ecb

commit ffa79076cf808c8295f0e499d56d920049b79ecb
Author: kristipark <kristipark@chromium.org>
Date: Wed Jul 12 01:15:30 2017

[IndexedDB] Trigger abort all transactions for database

Bug:  729790 
Change-Id: Ia96c9d84f52cedecc7d09bae6fd32bde8e898f2b
Reviewed-on: https://chromium-review.googlesource.com/563830
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Kristi Park <kristipark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485759}
[modify] https://crrev.com/ffa79076cf808c8295f0e499d56d920049b79ecb/content/browser/indexed_db/indexed_db_dispatcher_host.cc
[modify] https://crrev.com/ffa79076cf808c8295f0e499d56d920049b79ecb/content/browser/indexed_db/indexed_db_dispatcher_host.h
[modify] https://crrev.com/ffa79076cf808c8295f0e499d56d920049b79ecb/content/browser/indexed_db/indexed_db_dispatcher_host_unittest.cc
[modify] https://crrev.com/ffa79076cf808c8295f0e499d56d920049b79ecb/content/browser/indexed_db/indexed_db_factory.h
[modify] https://crrev.com/ffa79076cf808c8295f0e499d56d920049b79ecb/content/browser/indexed_db/indexed_db_factory_impl.cc
[modify] https://crrev.com/ffa79076cf808c8295f0e499d56d920049b79ecb/content/browser/indexed_db/indexed_db_factory_impl.h
[modify] https://crrev.com/ffa79076cf808c8295f0e499d56d920049b79ecb/content/browser/indexed_db/mock_indexed_db_factory.h
[modify] https://crrev.com/ffa79076cf808c8295f0e499d56d920049b79ecb/content/common/indexed_db/indexed_db.mojom

Owner: eostroukhov@chromium.org
Status: Fixed (was: Assigned)

Sign in to add a comment