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

[Announcement] Offline Storage upgrades in v2.3 will be temporarily unavailable #1248

Closed
joeyparrish opened this issue Jan 24, 2018 · 7 comments
Assignees
Labels
status: archived Archived and locked; will not be updated type: announcement An announcement from the team; generally pinned to the top type: bug Something isn't working correctly
Milestone

Comments

@joeyparrish
Copy link
Member

TL;DR: If you use offline storage, you may have to delete existing offline content to upgrade to current v2.3 releases. Future releases will fix this. You may want to delay upgrade to v2.3 if deleting content is not acceptable.

In #1230, a user notified us that they were unable to store new content offline. We tracked this to an issue caused by upgrading an application from v2.2 (or earlier) to v2.3.x.

While working on a fix, we have discovered many complicating factors, including:

  • IndexedDB schema updates must happen in a transaction
  • Transactions have a maximum size which is easy to surpass with media content
  • Limitations differ across browsers

Because of these complexities, we have decided on a short-term fix and a long-term fix.

The short-term fix is to offer developers a new error code to detect upgrade issues and a new method to delete the existing database. This will allow applications to recover from this issue, albeit at the expense of any content already stored by end-users. When this is done, we will close #1230. The changes will appear in our next release, v2.3.2.

The long-term fix is to change the way we deal with the storage databases. We will continue to read old data from the old tables, and new data will be stored into new tables. We will avoid database upgrades completely and we will not be hampered by the various complexities of IndexedDB upgrades. When this is done, we will close this issue.

The long-term fix will be made available in a future v2.3.x release. (As soon as we can complete the work.) If deleting a user's existing content is unacceptable in your application, we recommend you avoid updating to v2.3.x until the long-term fix is in.

@joeyparrish joeyparrish added the type: bug Something isn't working correctly label Jan 24, 2018
@joeyparrish joeyparrish added this to the v2.4.0 milestone Jan 24, 2018
@joeyparrish joeyparrish added the type: announcement An announcement from the team; generally pinned to the top label Jan 24, 2018
@vaage
Copy link
Contributor

vaage commented Jan 26, 2018

UPDATE

We have removed the upgrade code using this list of CLs:

With these CLs, if your storage needs to upgrade, the storage operation will fail with the error UNSUPPORTED_UPGRADE_REQUEST. If you catch this you can delete the database backing storage by using shaka.offline.Storage.deleteAll. In the error, data[0] will have a list of original content uris so that you can re-download all the content again.

@joeyparrish
Copy link
Member Author

Fixes to date cherry-picked for v2.3.2

joeyparrish pushed a commit that referenced this issue Jan 30, 2018
This removed all of DBUpgrade (lib and test) as it was not accomplishing
its requirements.

Now instead, if DBEngine detects that there needs to be an upgrade, it will
abort the upgrade and message the app with |UNSUPPORTED_UPGRADE_REQUEST|.

Issue #1230
Issue #1248

Change-Id: If65bc013b0482c9b0c6e71e644e9132584984414
joeyparrish pushed a commit that referenced this issue Jan 30, 2018
Expose a function through Storage to allow developers to delete the
database that backs storage.

Issue #1230
Issue #1248

Change-Id: Icb20b6c43966a299d8903b2d14f33666f672bd15
joeyparrish pushed a commit that referenced this issue Jan 30, 2018
Create tests to show that DBEngine should not open unless it has
the correct version.

Issue #1230
Issue #1248

Change-Id: I410e1c5d35a3a7b9e91afdd247f06ffac8d5a86d
joeyparrish pushed a commit that referenced this issue Jan 30, 2018
When DBEngine aborts because of an upgrade, include a list of the
original uris for all downloaded content.

This will allow apps to re-download the content if they choose to.

Closes #1230
Issue #1248

Change-Id: Ic2029d935dd8da60c0d502cd0cdbb08b38bbbe76
shaka-bot pushed a commit that referenced this issue Feb 1, 2018
Issue #1230
Issue #1248

Change-Id: Ia3942d8f6fc1b6c5e0166f86aa6620625f75d0db
joeyparrish added a commit that referenced this issue Feb 1, 2018
Issue #1230
Issue #1248

Change-Id: Ia3942d8f6fc1b6c5e0166f86aa6620625f75d0db
shaka-bot pushed a commit that referenced this issue Feb 26, 2018
This change introduces the storage cell which looks very similar to
IStorageEngine. There are two differences:
  1. A storage cell can be listed as "fixed key space" which means
     that it does not support new keys. This will be true when we
     mount our old indexed db stores.
  2. All commands are designed to be batched commands. This is to
     create consistency with what each command expects as input and
     returns. This should allow each implementation of storage cells
     to better optimize their operations.

Issue: #1248
Change-Id: Ic464fd7587aed058e346331f7b692f9c4e279f28
shaka-bot pushed a commit that referenced this issue Feb 26, 2018
Storage mechanism are groups of storage cells. The storage mechanism
is responsible for managing resources shared between all its cells.

Issue: #1248
Change-Id: I57b3321c837776c3e85bd73afa3fd4d97d9b2f50
shaka-bot pushed a commit that referenced this issue Mar 6, 2018
The storage api was using SegmentDB when it should have been using
SegmentDataDB. Since nothing was implementing the api yet, this
oversight was not caught.

Issue: #1248
Change-Id: I0e3e7521458d37e9e21f9a19cf6f7e6f1f2aca88
shaka-bot pushed a commit that referenced this issue Mar 6, 2018
Created a memory storage system that holds all data in main memory. This
system will be used in tests.

Needed to update the eslint rules to allow us to have an instance-level
method that does not use |this|.

Issue: #1248
Change-Id: If1ebbec62bfdae57ee06df9fdef9955b30579d52
shaka-bot pushed a commit that referenced this issue Mar 6, 2018
Created base classes that will be used when working with indexeddb.
These classes are used to manage connections and transaction is a
more promise-friendly way.

Issue: #1248
Change-Id: Iafd001072bee8572f41af7069532f3a1ccd39fbc
shaka-bot pushed a commit that referenced this issue Mar 6, 2018
Since the database schemes for idb versions 2 and 3 used the same
data types (shaka offline version 2) this storage cell can be used
for both. The only difference is that idb version 2 has a fixed
key space.

Issue: #1248

Change-Id: Iac5cb1d57f93fdb81729b59d67d6736e29c9968c
shaka-bot pushed a commit that referenced this issue Mar 7, 2018
Storage Muxer is responsible for finding the correct cell for a
given operations. For example, when writing new content, you need
a cell that supports add-operations, the muxer will find one.

Issue: #1248
Change-Id: Ifd9fd32f9487c0b9bc011ae8b004eec4ee75f7c4
shaka-bot pushed a commit that referenced this issue Mar 14, 2018
Fixed an issue where an update call the IndexedDB was not replacing
the correct value because the key was getting changed to a string.

IndexedDB will count a missing key as a success, so check that the
value is undefined.

Issue: #1248
Change-Id: Icdd6ae263374c655170c6470df85c656b8bd3ba2
@joeyparrish
Copy link
Member Author

I used the phrase "when we resolve" in a commit, and referred to this bug, which means I accidentally closed it. Whoops!

@joeyparrish joeyparrish reopened this Mar 15, 2018
joeyparrish added a commit that referenced this issue Mar 21, 2018
This updates the existing diagrams and adds new ones for cast and
offline.  The offline diagrams will need to be updated again after
we resolve #1248.

Closes #1197

Change-Id: I6b6b1fac732b4997c579f58c7f12f0f84f202380
shaka-bot pushed a commit that referenced this issue Mar 22, 2018
Issue: #1248
Change-Id: I2522ce02e8a9bdb8c6bea9f77adf91393b0265b6
shaka-bot pushed a commit that referenced this issue Mar 22, 2018
Made a copy of the storage cell tests for indexeddb but made them
work for the memory storage cell so show that we can expect the
same behavior from them.

This is to give us an extra level of confidence when we use the
memory storage cells in place of the indexeddb storage cells in
our tests.

Issue: #1248
Change-Id: I96eaf57a9d808b4c7e07f97ac2713318a889fc24
joeyparrish pushed a commit that referenced this issue May 10, 2018
Created a static method that will handle destroying a destroyable
object when you are done with it.

Issue #1248

Change-Id: I4df0569f5f5e00002600702cf24caf1ed2da7c5b
joeyparrish pushed a commit that referenced this issue May 10, 2018
Changed the loops used to check for similar tracks to be simpler. Yes,
there are some double checks, but over all it is easier to read. Since
this code is ran very rarely, the minor redundant checks should not be
a problem to the user.

Issue #1248

Change-Id: I6589dda233e94defb5729934ee859b78e3af193f
joeyparrish pushed a commit that referenced this issue May 10, 2018
To make it easier to understand, we collect all the streams within a
period using a map. This ensures that we will have a set of streams.

We then download each stream in the map. Since the map is indexed by
stream id, we can easily connect the variants and streams.

Issue #1248

Change-Id: I65ae01733b5d9dbf8b6d73c37f802320fffa03d1
joeyparrish pushed a commit that referenced this issue May 10, 2018
Group downloads by stream id so that download manager does not
need to know about the stream types.

Issue: #1248
Change-Id: Id61df4e67a419ee085dfd96a863bb56f01a4b732
joeyparrish pushed a commit that referenced this issue May 10, 2018
Went through the download code and tried to make it easier to read
and understand.

Issue #1248

Change-Id: I74316d3337248f0d16f2fb6cc1734e1e57739966
joeyparrish pushed a commit that referenced this issue May 10, 2018
Changed download manager to take |shakaExtern.Request| as input
instead of |shaka.media.SegmentReference| and |shaka.media.InitSegmentReference|
as it really does not need to know about those types.

Issue #1248

Change-Id: I797c437f4339cf670b5eddad14952b0526b72ea5
joeyparrish pushed a commit that referenced this issue May 10, 2018
Because we changed how we collect the streams before downloading we
no longer used the variant id parameter. This change removes the
parameter.

Issue: #1248
Change-Id: Iab4323c7b358ed83812b2ff36e9632022e65f88a
joeyparrish added a commit that referenced this issue May 10, 2018
This makes it easy to dump and restore databases from IndexedDB. This,
in turn, makes it easy for us to test that all database versions can
be read. Before we close the backward compatibility issue that has
plagued v2.3, we will add tests that use this utility to load DB
snapshots from various older schemas and prove that they can still be
read.

This also adds dumps of all database versions to the assets folder,
including a snapshot of a what our broken upgrade in v2.3.0 did to the
v2 database schema.

Issue #1248

Backported to v2.3.x

Change-Id: If7e8995f50abbdee67e3fa93e79f07a49582c5e8
joeyparrish pushed a commit that referenced this issue May 10, 2018
Move the use of storage engine out of download manager and back into
storage. Storage creates and manages the life of storage engine, so
it is easier for it to handle all storage functions.

Now, the callback that is given to download manager is used to pass
the downloaded array buffer back to storage so that it can be saved
into storage engine.

This moves all uses of storage engine into storage which will make it
easier to replace with Storage Muxer.

Issue #1248

Change-Id: I555788478af09d1d79c3db7534431e1d9daf4e20
joeyparrish added a commit that referenced this issue May 10, 2018
Using variant bandwidth to estimate download progress. We use a constant
bandwidth estimate for audio, as most of a variant's bandwidth will be used
by video.

Since we normally don't have bandwidth for text, we use a constant bandwidth
estimate for text so that it can influence progress.

Even through we some times have the actual audio and video bandwidth (DASH)
we choose to use always used the combined amount so that we can have
consistent behaviour between manifest types.

Issue #1248

Backported to v2.3.x

Change-Id: Ibffe95e2c8d141659855b54949a50ae84c51dded
joeyparrish pushed a commit that referenced this issue May 10, 2018
It appears that closure did not notice that a return type
was not consistent. This corrects the return type to match
what the download method in download manager actually
returns.

Issue #1248

Change-Id: I796e31d9e7ad1d812bd53a55f5f1c745ac0b187c
joeyparrish pushed a commit that referenced this issue May 10, 2018
Make storage muxer implement the IDestroyable interface so that
it can be used with |shaka.util.IDestroyable.with|.

Issue #1248

Backported to v2.3.x

Change-Id: I6e8c71b57e939ea30d5c24c75ede9c2de80026db
joeyparrish pushed a commit that referenced this issue May 10, 2018
Changed |findAll| to |forEachCell| as it provides better usability in the
codebase where we would have used |findAll|.

Issue #1248

Backported to v2.3.x

Change-Id: I6ca993ffb85aa0bf15d6ac250ac85a00e966fcbe
joeyparrish pushed a commit that referenced this issue May 10, 2018
Added a toString method to StorageCellPath so that it will be
easier to read when added to a string.

Issue #1248

Change-Id: Iad7c2e633b6327bd39475d29d23426217f042ee3
joeyparrish pushed a commit that referenced this issue May 10, 2018
Added a method to get a cell from storage muxer. Unlike resolve path
this will return a rejected promise if the cell can not be found.

Issue #1248

Backported to v2.3.x

Change-Id: Ia594b7741351d515e564e574321e49ad51073005
joeyparrish pushed a commit that referenced this issue May 10, 2018
Limit the scope of storage and download manager to |store|. This
will make it easier to swap it with storage muxer.

Issue #1248

Backported to v2.3.x

Change-Id: I3ec5de4457cbb534eb0c9b44d518602a1295d709
joeyparrish pushed a commit that referenced this issue May 10, 2018
Create the indexeddb storage mechanism which will allow us to
connect to indexeddb storage cells. As we don't have a V1 Cell
yet, this only connects V2 and V3 cells. However the code does
assume that V1 will be added and only omits connecting the cell
when it sees the V1 object stores indexeddb.

Issue #1248

Backported to v2.3.x

Change-Id: I7395b8a334cd68795aab02ff656a5b49d1f5742b
joeyparrish pushed a commit that referenced this issue May 10, 2018
Need to close database connections when destroying the
indexeddb storage mechanism or else it will get in the
way of deleting the database.

Issue #1248

Backported to v2.3.x

Change-Id: I56466851e8172282c60c3128e5b4113001a10970
joeyparrish pushed a commit that referenced this issue May 10, 2018
While working with integrating storage muxer into the codebase, there
were some painful points in the API. This change addresses those
painful points.

1. Getting cells - many times we know that the cell should be under
                   a specific path. So resolving the path and
                   checking for null got annoying. This adds
                   a call that assume it should find the cell.

2. Get active cell - when getting the active cell, we often need
                     the cell and the path to the cell. So this
                     creates a "Handle" which pairs the cell and
                     path together.

3. Adjusting registry - when testing we need to change what storage
                        cells are actually available. Instead of
                        unregistering and registering cells, this
                        adds methods to add and remove a temporary
                        override.

Issue #1248

Backported to v2.3.x

Change-Id: Ifd45ee5912f53b1da444476b560cf03669a19b11
joeyparrish pushed a commit that referenced this issue May 10, 2018
Created the V1 storage cell for Shaka Player's original offline
data scheme.

Issue #1248

Backported to v2.3.x

Change-Id: Ib564ec0670c56ba1b455f198133b118a21fd0a27
joeyparrish pushed a commit that referenced this issue May 10, 2018
This CL replaces the old storage system with the storage muxer system.
In addition to replacing the old system, this CL removes all the
code that was only used by the old system.

As of this change Storage Muxer support shaka player's V1, V2, and V2
indexeddb schemes and exposes a plug-in system for other offline storage
components.

Issue #1248

Backported to v2.3.x

Change-Id: I1a4914477ad8db29fd0e7ad7de2f149b6497f67e
joeyparrish pushed a commit that referenced this issue May 10, 2018
Tracking progress when removing an asset was missing from the new storage
system. This CL adds that support back.

Issue #1248

Backported to v2.3.x

Change-Id: Iab275cd75af817cfc34ee0888ddeea257b1ead56
joeyparrish pushed a commit that referenced this issue May 10, 2018
Added a new test to verify that the progress callback is called when content
is removed from storage.

Issue #1248

Change-Id: I60f1f1ab744d79214685c0060e56f4e21514948c
@joeyparrish
Copy link
Member Author

All fixes will appear in v2.3.8 and v2.4.0 this week.

@odedhutzler
Copy link

Hey @joeyparrish,

Upon closing this issue, are you considering the pause / resume & renew license features?

We also thought of constantly updating the manifest in the DB, so if the browser / tab is closed accidentally, the user will be able to continue the download from where it ended. Are you considering some kind of anti-failure mechanism?

@joeyparrish
Copy link
Member Author

@odedhutzler, this issue was about backward compatibility for upgrades. Please file feature requests for those new features. Thanks!

@shaka-project shaka-project locked and limited conversation to collaborators Jul 9, 2018
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Apr 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: archived Archived and locked; will not be updated type: announcement An announcement from the team; generally pinned to the top type: bug Something isn't working correctly
Projects
None yet
Development

No branches or pull requests

4 participants