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

Move state storage to IndexedDB/LevelDB #2044

Closed
andrevmatos opened this issue Aug 4, 2020 · 2 comments · Fixed by #2110
Closed

Move state storage to IndexedDB/LevelDB #2044

andrevmatos opened this issue Aug 4, 2020 · 2 comments · Fixed by #2110
Assignees
Labels
20 enhancement New feature or request optimization ⚡ Optimizations for the implementation or protocol Ready 🎬 Issues which are ready to be pulled into the iteration refactor sdk 🖥

Comments

@andrevmatos
Copy link
Contributor

andrevmatos commented Aug 4, 2020

Description

Fixes #1933 #1831
The above issues seems to be caused by client losing state, as we've feared for long, or client crashing before persisting state after a critical state change took place (like new BalanceProof created). This is a tricky problem, due to the ephemeral nature of in-browser storage keeping.

Current RaidenState is stored by default like this:

  • An optional localStorage instance is received by Raiden.create
  • The static factory then registers a debounced callback to state$ observable, which by default serialize and store the state after 1s without a new state being generated, and at most every 5s if new states keep going through.
    • This is done because the serialize part can be quite CPU intensive, and both the serialize and state saving on LocalStorage currently are synchronous operations which can take time and block other more important events processing.
    • Upon shutdown (for any reason), the state is also persisted, but of course this won't protect from browser suddenly crashing or other catastrophic event like this.
    • Unfortuntately, this is quite a big time window for state to be lost, and can lead to the mentioned bugs.
    • LocalStorage is also quite limited in size, often being capped at 5MB in browsers, which may not be enough for longer-lived clients.

The proposed solution is to move the state to the IndexedDB or even directly a LevelDB:

  • Available on all modern browsers
  • Have a way higher size cap, often going up to 50% of user's available disk space
  • Quite easily extendable to use external/dedicated keystore/database systems, like Mongo, Redis or even Postgres, for even stricter threat models.
  • put operations are asynchronous
  • IndexedDB/LevelDB is able to receive the state as is, without requiring serializing them ourselves
    • the only reason to serialize the state currently would be BigNumber, but IndexedDB is able to encode them as {"_hex": "0x1234"} objects which, as long as we're able to decode (and we already are), should allow us to persist the state directly, without an expensive encoding step
    • Our immutable state policy also allows us to do this right away, without worrying about locking or concurrent changes to the storing state
  • We could keep certain parts of the state which are prone to growing indefinitely, like sent & received transfers, on a separate object store, or key prefix, for higher efficiency on storing of the general state, although requiring async access (shouldn't be a problem for the async epics, may require some thinking for the reducers)
  • We should still have functions to convert from and to the current JSON state schema, in order to keep supporting downloading and uploading the state as is, since these operations are not performance-sensible.

Acceptance criteria

Tasks

  • [ ]
@andrevmatos andrevmatos added enhancement New feature or request sdk 🖥 refactor optimization ⚡ Optimizations for the implementation or protocol labels Aug 4, 2020
@andrevmatos andrevmatos added this to the Product Backlog milestone Aug 4, 2020
@andrevmatos andrevmatos changed the title Move state storage to IndexedDB Move state storage to IndexedDB/LevelDB Aug 4, 2020
@christianbrb christianbrb added the Ready 🎬 Issues which are ready to be pulled into the iteration label Aug 6, 2020
@christianbrb christianbrb removed this from the Product Backlog milestone Aug 6, 2020
@christianbrb christianbrb added the 20 label Aug 6, 2020
@andrevmatos andrevmatos self-assigned this Aug 6, 2020
@andrevmatos
Copy link
Contributor Author

Currently investigating RxDB as an interface for both IndexedDB (browser) and LevelDB (Node) which would allow us all the Rx goodies right from the DB.

@andrevmatos
Copy link
Contributor Author

An insightful article about serialization costs of deep or wide state updates.
We shouldn't be using workers/postMessage, but the considerations are still valid.
By splitting up transfers, the remaining state should be small enough for serialization steps to not be relevant, and even though RxDB may not use IndexedDB's native StructuredSerializer, we may still keep in mind this while designing this feature, specially if we want to keep open the future possibility of slower (e.g. over-the-network) database backends/storage/backups/replication.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
20 enhancement New feature or request optimization ⚡ Optimizations for the implementation or protocol Ready 🎬 Issues which are ready to be pulled into the iteration refactor sdk 🖥
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants