Refactor storage #367
Open
Refactor storage #367
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } | ||
| defer lock.Unlock() | ||
|
|
||
| fi, err := os.Stat(historyPath) |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
| return "", 0, storage.ErrorNotFound | ||
| } | ||
|
|
||
| fd, err := os.Open(historyPath) |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
| defer lock.Unlock() | ||
|
|
||
| currentGen := int64(0) | ||
| fi, err := os.Stat(historyPath) |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
| return currentGen, storage.ErrorWrongGeneration | ||
| } | ||
|
|
||
| hist, err := os.OpenFile(historyPath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0600) |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
nemunaire
force-pushed
the
f/refactor-storage
branch
from
48f9109 to
7fda066
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We know that reMarkable uses Google Storage as the backend for its sync service. Many features we observe are inherited directly from the capabilities provided by Google Storage.
In particular, routes like
/sync/v3/files/abcdef123456are effectively aliases for Google Storage routes. The logic behind these routes should remain independent from other routes, to allow using an object storage (e.g., S3) instead of a local filesystem.However, the routes
/sync/v3/rootand/sync/v4/rootare not tied to object storage. The "root" object, along with its generation number, is used to help merging concurrent modifications, something not handled by Google Storage itself.Currently, the generation number is returned by the
LoadBlobfunction, even though it's only meaningful for the root. Individual files don't have a generation number.The logic handling the root object and its history should be separated from the file storage logic. The current root index is closer to user state than to a file/blob.
What This PR Changes
I’ve extracted the root-specific logic from
LoadBloband introduced two new functions:GetRootandUpdateRoot. These operate on the user storage layer, not the blob storage layer.Some types from the
storagepackage were moved into themodelspackage to allow them to be shared across different storage implementations.The last commit introduces a new storage backend abstraction for the root index: the
RootStorerinterface, responsible for managing the root index, its generation number, and the cached tree.All abstractions are now cleanly separated to make it easier to implement alternative storage backends. For example: a database backend for user storage, or a S3-compatible blob storage.
All go interface could have a different implementation (users in database, root indexes in filesystem, blobs in S3, ...).
All filesystem-related calls are now isolated in the
fspackage. The top-levelstoragepackage now only contains interfaces and logic common to all storage implementations.Overall, this pull request should not change any user-facing behavior.
Please test it with your usual workflow and report any unexpected issues or regressions.