- In case of reorganized block the Bech32UtxoSet recovered to its previous state #48
Conversation
| @@ -24,7 +24,67 @@ public class IndexBuilderService | |||
| private List<FilterModel> Index { get; } | |||
| private AsyncLock IndexLock { get; } | |||
|
|
|||
| private Dictionary<OutPoint, Script> Bech32UtxoSet { get; } | |||
| private Dictionary<OutPoint, Script> Bech32UtxoSet { get; } //Change to observable concurrent dictionary? https://www.nuget.org/packages/ParallelExtensionsExtras/ | |||
There was a problem hiding this comment.
While concurrent collections don't throw exceptions, not all threading issues can be ignored. And they can work in a quite misleading way: https://stackoverflow.com/a/1966462/2061103
I want it to throw an exception if something is wrong. As you see, right above this line there is an IndexLock for the Index list. This is, because the Index may encounter threading issues, but as the code is today Bech32UtxoSet won't encounter threading issues, since it's an "internal database" that's being touched in a proper way.
Also performance is quite an issue here, so that's another con against concurrent collections.
Generally I use concurrent collections against lock, whenever
- I'm lazy.
- I'm showing data to the user (that doesn't have to be so accurate.)
Discussion on this topic can be found here: stratisproject/StratisBitcoinFullNode#617
If you agree, please remove the todo comment here.
There was a problem hiding this comment.
Sure I will remove. P.S.: I would use the observable part of the dictionary to subcribe the add/remove/clear event to ensure that all changes of the dictionary get noticed.
There was a problem hiding this comment.
Another way is to implement my own observable dictionary without concurrency: something like this
https://stackoverflow.com/questions/5663395/net-observabledictionary
There was a problem hiding this comment.
I think it's better as it is now, since it's simpler, if it gets more complex we can change. Btw, I created my ObservableConcurrentDictionary if you ever need such: https://github.com/nopara73/HiddenWallet/blob/cc8c948ac7a1b9173610145e60c303d71a8c0a66/HiddenWallet/Helpers/ConcurrentObservableDictionary.cs
|
|
||
|
|
||
|
|
||
| internal static class ActionHistoryHelper |
There was a problem hiding this comment.
Could you make it non-static? While it'll be a singleton for now, later or during tests it could cause problems.
There was a problem hiding this comment.
This is a very special class only for this special purpose. I do not want anybody to create more instances or make it visible from outside.
Of course I can change it to non static.
There was a problem hiding this comment.
Please make it. My concern is if IndexBuilderService becomes used in different places it'll break. In fact it probably will while testing. Tests are usually running concurrently, and back then my static global classes were the cause of my unreliable tests, so I'd like to avoid it this time.
I do not want anybody to create more instances or make it visible from outside.
I guess you can make it private, too. If you don't want to write tests for it. I think tests are not needed.
|
I merged it, because I need it. I'll make the changes myself. |
No description provided.