add core state lock deadlock detection config option #17020
Conversation
|
Testing here might also be too clever by half: I didn't want to punt on adding a test but also wanted to avoid mocking away too much from go-deadlock's logging/exit. There's perhaps a better approach in using go-deadlock's config options to not exit instead. |
troyready
force-pushed
the
detect_deadlocks
branch
from
9d50713
to
677fcbe
Compare
This was previously available via a build flag but will now be available as a configuration option.
Previous implementation was unnecessary
Can use deadlock global options to avoid testing shenanigans
troyready
force-pushed
the
detect_deadlocks
branch
from
9afec08
to
618aaf4
Compare
|
Ah, sorry for the obtuseness @ncabatoff -- I had drawn some incorrect conclusions due to errant test results. I've simplified the implementation as discussed. |
|
I'm pretty sure the failing race condition test is expected due to the deadlock induced in the new test. I'm torn about best way to fix it:
|
| @@ -3497,3 +3498,51 @@ func TestExpiration_listIrrevocableLeases_includeAll(t *testing.T) { | |||
| t.Errorf("bad lease count. expected %d, got %d", expectedNumLeases, numLeases) | |||
| } | |||
| } | |||
|
|
|||
| func InduceDeadlock(t *testing.T, vaultcore *Core, expected uint32) { | |||
There was a problem hiding this comment.
It's a little weird to have this in expiration_test.go, and have it depend on vaultcore.expiration, since there's nothing specifically about expiration in the test itself.
|
Closing in favor of #18604 . |
Previously, a build flag could be specified to enable the go-deadlock library, but this change makes it a regular service config option.
It's possible that this is still too a bit too conservative of an approach: because go-deadlock has a disable option that is about as efficient as possible we could just use that instead of this conditional assignment.