[bisq-network/bisq] ConcurrentModificationException FileManager::saveToFile (#3752)

[Julian Knutsen]

I spent the afternoon running some tests and profiling the impact of a few ways to handle this. I wanted to share the options I came up with to get some feedback from others who may have encountered this in the past or have thought about it for more than a few hours.

### Use concurrent data structures
Replace all data structures with their concurrent options. ArrayList -> CopyOnWriteArrayList & HashMap -> ConcurrentHashMap

This will fix the immediate problem by ensuring that during iteration a change in the underlying data structure won't cause an exception. But, there are two issues to consider:

First, `CopyOnWriteArraySet` is only efficient for read-heavy workloads. Since there is typically one write per read (iterating and persist) there would be a costly performance penalty when the COWAL has to copy the array on write/update/remove. This may be a non-issue because the data sets are so small, but something to consider.

Second, the real problem is that the FileManager is running arbitrary methods (toProtoMessage) from non-UserThread without any synchronization or safety from the `Storage` classes. Even if we fix up all these current issues they will just reappear in a different form if it isn't fixed in a more sustainable way. It is really non-obvious which class members need to be thread-safe and which ones don't.

### Create protobuf objects on the UserThread instead of the FileManager writer thread
This seems like the cleanest design moving forward. Current and future `Store` objects don't need to care about thread safety.

The `FileManager` is easier to reason about as well since the work queue `AtomicReference` would just store the immutable protobuf object instead of a class instance that can change.

But, the overhead for building the protobuf message on each save is pretty significant. On average, it takes around 1500ms for `DaoStateStore::toProtoMessage()` to complete. This negates a lot of the batching work that the FileManager hoped to accomplish and can cause the initial dao rebuild to take quite a while.

![on_user_thread_after](https://user-images.githubusercontent.com/8082291/70292605-7b086300-1793-11ea-985a-34fab20a361f.JPG)
![on_user_thread_after2](https://user-images.githubusercontent.com/8082291/70292606-7b086300-1793-11ea-95c0-a61d567c83a8.JPG)




-- 
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/issues/3752#issuecomment-562415187
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20191205/c2cae0c0/attachment-0001.html>