[bisq-network/bisq] [POST_CODE_FREEZE] Persist changes to ProtectedStoragePayload objects implementing PersistablePayload (#3640)

[Florian Reimair]

freimair commented on this pull request.



> @@ -418,9 +418,8 @@ public boolean addProtectedStorageEntry(ProtectedStorageEntry protectedStorageEn
 
         // Persist ProtectedStorageEntrys carrying PersistablePayload payloads and signal listeners on changes
         if (protectedStoragePayload instanceof PersistablePayload) {
-            ProtectedStorageEntry previous = protectedDataStoreService.putIfAbsent(hashOfPayload, protectedStorageEntry);
-            if (previous == null)
-                protectedDataStoreListeners.forEach(e -> e.onAdded(protectedStorageEntry));
+            protectedDataStoreService.put(hashOfPayload, protectedStorageEntry);

> I'm not sure I understand the full concern.

The storage stuff is most probably broken and we had corrupted databases in the past (which are very hard to recover from especially because the max message size is nearly reached and seednodes/clients have a hard time sending/receiving the whole 10MB of data). If we increase the number of writes (by persisting stuff everytime something we already have gets updated) we up the chance of hitting that race condition. Nothing more nothing less.

> Are you suggesting we hold the release until FileManager.java:75 is looked into?

no, there is no need to hold the release. The broken code is shipped already, so there is nothing we can do about that. If you mean holding the PR by "hold the release" until this is fixed: yes.

> aren't threadsafe regardless of the Storage data structures. Is that right?

yes, correct. I believe the storage stuff needs a closer look and fast and with high priority.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/bisq-network/bisq/pull/3640#discussion_r350372475
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20191125/09b0171d/attachment-0001.html>