participation key registry : complete cleanup #3768
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3768 +/- ##
==========================================
+ Coverage 49.75% 49.77% +0.01%
==========================================
Files 392 392
Lines 68745 68757 +12
==========================================
+ Hits 34205 34222 +17
+ Misses 30777 30770 -7
- Partials 3763 3765 +2
Continue to review full report at Codecov.
|
| // Persist participation registry metrics. | ||
| node.accountManager.FlushRegistry(2 * time.Second) | ||
| // Persist participation registry updates to last-used round and voting key changes. | ||
| err = node.accountManager.Registry().Flush(participationRegistryFlushMaxWaitDuration) |
There was a problem hiding this comment.
I'm not sure this duration should be raised. Don't we want to keep the warning if it is taking > 2 seconds?
| err = node.accountManager.Registry().Flush(participationRegistryFlushMaxWaitDuration) | |
| err = node.accountManager.Registry().Flush(2 * time.Second) |
There was a problem hiding this comment.
It would be nice to get a warning about this specific flush timeout being slow, agreed. (As opposed to the REST API ones)
| return makeErasableAccessor(dbfilename, false) | ||
| } | ||
|
|
||
| func makeErasableAccessor(dbfilename string, readOnly bool) (Accessor, error) { |
There was a problem hiding this comment.
I guess this function does not need readOnly argument, since _secure_delete does not make sense for reader
There was a problem hiding this comment.
It depend on the case - see new erasablepair
There was a problem hiding this comment.
I discovered our readOnly argument is not enforced by the driver, and mostly only controls whether _txlock=immediate is set in the URI. So I wanted to ensure that both of the pair of DB conns used the same _secure_delete flag when being opened, just in case.
| } | ||
| } | ||
|
|
||
| // mark updated records as dirty, so they will be flushed by a call to FlushRegistry after each round |
There was a problem hiding this comment.
FlushRegistry func looks removed
There was a problem hiding this comment.
yes, we should fix this in a later PR.
There was a problem hiding this comment.
Do we want to add a TODO here?
There was a problem hiding this comment.
I'll take care of this one.
6 participants
Summary
This PR completes the previous started efforts and ensure all the requests are tunneled directly to the participation registry for optimal performance.
Test Plan
Unit tests added.