-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Persistence redesign #4589
Merged
Merged
Persistence redesign #4589
Conversation
This file contains 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
chimp1984
force-pushed
the
persistence-redesign
branch
from
October 2, 2020 21:49
6f80144
to
207d098
Compare
Merged
Remove UserThread.execute on shutdown Rename threadname
…nding data to disk Use new readAllPersisted method instead of setupPersistedDataHosts. Run read tasks in a thread in parallel. Read is only done at startup. Remove unused restart method
We will not need threading support anymore once we use the new persistenceManager as serialisation is done on the userThread.
…e instead We will not need threading support anymore once we use the new persistenceManager as serialisation is done on the userThread.
We will not need threading support anymore once we use the new persistenceManager as serialisation is done on the userThread.
Use AtomicReference instead of array, add final
Remove clone from SequenceNumberMap.
…and fields used only for that.
Add comment Use decrementAndGet instead of getAndDecrement Tested cases when there is an exception at write to disk, but as we call the result handler in the finally clause we get always called onWriteCompleted, so it cannot happen that we get stuck.
…did not miss any state update. Priority.HIGH stores are all those which contain private data. Others can be rebuilt from network data or are not critical like navigationPath.
…which did not have priority as a param. Make initializePersistenceManager in StorageService abstract to enforce in concrete class to define priority. Change priorities for future renaming to a different meaning. instead of priority we want to describe the category: private data, public data,.... will come in next commit
… intention of the usage. Rename: LOW to NETWORK MID to PRIVATE_LOW_PRIO HIGH to PRIVATE Change delay of MID/PRIVATE_LOW_PRIO from 30 min to 2 hours (we had different datastores before using it, now its only real low prio stores) Add comment to each enum
chimp1984
force-pushed
the
persistence-redesign
branch
from
October 3, 2020 17:48
fbaef26
to
6693a03
Compare
Rebased again latest master merge. |
sqrrm
requested changes
Oct 4, 2020
common/src/main/java/bisq/common/persistence/PersistenceManager.java
Outdated
Show resolved
Hide resolved
common/src/main/java/bisq/common/persistence/PersistenceManager.java
Outdated
Show resolved
Hide resolved
common/src/main/java/bisq/common/persistence/PersistenceManager.java
Outdated
Show resolved
Hide resolved
common/src/main/java/bisq/common/proto/persistable/PersistableList.java
Outdated
Show resolved
Hide resolved
common/src/main/java/bisq/common/proto/persistable/PersistableList.java
Outdated
Show resolved
Hide resolved
common/src/main/java/bisq/common/proto/persistable/PersistableList.java
Outdated
Show resolved
Hide resolved
Review needed from @ManfredKarrer to be able to merge this |
…r.java Co-authored-by: sqrrm <[email protected]>
Remove ObservableList methods from PersistableList Let DisputeList and TradableList extend PersistableListAsObservable Fix param names
ManfredKarrer
previously approved these changes
Oct 4, 2020
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK
This avoids frequent write at dao sync and better performance.
sqrrm
reviewed
Oct 4, 2020
common/src/main/java/bisq/common/persistence/PersistenceManager.java
Outdated
Show resolved
Hide resolved
sqrrm
reviewed
Oct 4, 2020
common/src/main/java/bisq/common/proto/persistable/PersistableListAsObservable.java
Outdated
Show resolved
Hide resolved
…hods. We do not set the collection as list but we fill the list created via the createList method.
sqrrm
approved these changes
Oct 4, 2020
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Based on #4586
Replaces #4527
Attempt for a more clean commit history (still a lot). As merge was getting pretty difficult I decided to re-apply all changes again and compare with old PR code and with base of #4586. Hope no bugs entered by the challenging merge process.
Update: As #4586 is now merged to master this PR got rebased on master.